mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 23:46:35 +00:00
fix(offline): a parked write holds back the later writes to the same entity
Parking a write the server keeps failing on unblocked its trip, so a later queued edit of the same budget item, visit, tour or driving settings went through while the older one waited. Try again then replayed the older write last and it silently overwrote the newer one: only places and packing items send a token the server could refuse it with. Before the parking change a 5xx stopped the whole flush, so this order was never at risk. The flush now holds back every pending write whose entity has an older write parked as failed or waiting as a conflict, and a write that parks during the pass holds back the rest of that pass too. Other entities and other trips keep syncing. Try again therefore replays the parked write first and the held ones after it, Keep mine does the same for a conflict, and Discard or Keep theirs flush right away so the held writes go out without waiting for the next trigger. Writes queued against an offline create that got parked now wait for it instead of failing as an unresolved temp id.
This commit is contained in:
+1
-1
@@ -35,7 +35,7 @@ The layering is **component → feature hook → store/slice → `repo/` → `ap
|
||||
- **`src/repo/`** — per-entity repositories mediating between the network and the offline cache: online, call REST and upsert into Dexie; offline, read Dexie. Always go through a repo for trip data.
|
||||
- **`src/api/client.ts`** — the single Axios instance, typed from `@trek/shared`. New API surface goes in a per-domain `api/<domain>.ts`, not the god module.
|
||||
- **`src/db/offlineDb.ts`** — the Dexie schema. Database and table names are the on-device contract — renaming one orphans every user's cache.
|
||||
- **`src/sync/`** — `mutationQueue.ts`: offline writes do an optimistic Dexie write (temporary **negative id** for creates), then `flush()` replays REST on reconnect with the mutation's UUID as the **`X-Idempotency-Key`** header (matching the server interceptor) and reconciles the row. The rest of the directory drives reconnection sync, the auth gate that stops background loops before logout swaps the DB, offline preferences, and map pre-download (raster and vector prefetchers).
|
||||
- **`src/sync/`** — `mutationQueue.ts`: offline writes do an optimistic Dexie write (temporary **negative id** for creates), then `flush()` replays REST on reconnect with the mutation's UUID as the **`X-Idempotency-Key`** header (matching the server interceptor) and reconciles the row. Writes to one entity keep their order: a write parked as `failed` or `conflict` holds back the later writes to the same `resource` + entity id until the user retries, resolves or discards it, and only that action deletes a parked row (trip eviction never does). The rest of the directory drives reconnection sync, the auth gate that stops background loops before logout swaps the DB, offline preferences, and map pre-download (raster and vector prefetchers).
|
||||
|
||||
## Rules for new code
|
||||
|
||||
|
||||
@@ -81,6 +81,38 @@ function isRetryableStatus(status: number | undefined): boolean {
|
||||
return status === 401 || status === 408 || status === 425 || status === 429
|
||||
}
|
||||
|
||||
/**
|
||||
* The entity a queued write targets, as `resource:id`. A create is keyed by its
|
||||
* temporary id, which is also what the writes queued against it carry until it
|
||||
* syncs. Undefined when the row names no entity (it is then never held back).
|
||||
*/
|
||||
function entityKey(m: QueuedMutation): string | undefined {
|
||||
if (!m.resource) return undefined
|
||||
const id = m.entityId ?? m.tempEntityId ?? (m.method === 'POST' ? m.tempId : undefined)
|
||||
return id === undefined ? undefined : `${m.resource}:${id}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Entities with a parked write ('failed', or a 'conflict' waiting for the user),
|
||||
* each with the time of the earliest such write. A later write to the same
|
||||
* entity must not overtake it: replayed afterwards by Try again or Keep mine,
|
||||
* the older write would silently overwrite the newer one, and only places and
|
||||
* packing items carry a token the server could refuse it with.
|
||||
*/
|
||||
async function parkedEntities(): Promise<Map<string, number>> {
|
||||
const parked = await offlineDb.mutationQueue.where('status').anyOf(['failed', 'conflict']).toArray()
|
||||
const held = new Map<string, number>()
|
||||
for (const m of parked) holdEntity(held, m)
|
||||
return held
|
||||
}
|
||||
|
||||
function holdEntity(held: Map<string, number>, m: QueuedMutation): void {
|
||||
const key = entityKey(m)
|
||||
if (key === undefined) return
|
||||
const since = held.get(key)
|
||||
if (since === undefined || m.createdAt < since) held.set(key, m.createdAt)
|
||||
}
|
||||
|
||||
/** Pull the server's current entity out of a 409 response body ({ server: {...} }). */
|
||||
function extractConflictServer(err: unknown): unknown {
|
||||
const data = (err as { response?: { data?: unknown } })?.response?.data
|
||||
@@ -177,7 +209,9 @@ export const mutationQueue = {
|
||||
* Stops on the first network error (retried on the next trigger). 4xx answers
|
||||
* are marked failed and skipped; a DELETE answered 404 counts as done. A 5xx
|
||||
* holds back only its own trip, retried with growing gaps, and is parked as
|
||||
* failed once it has failed MAX_SERVER_ERROR_ATTEMPTS times.
|
||||
* failed once it has failed MAX_SERVER_ERROR_ATTEMPTS times. A write parked as
|
||||
* failed or as a conflict holds back the later writes to the same entity, so
|
||||
* they keep their order whatever the user decides about it.
|
||||
*/
|
||||
async flush(): Promise<void> {
|
||||
if (_flushing || isEffectivelyOffline() || !isAuthed()) return
|
||||
@@ -212,6 +246,9 @@ export const mutationQueue = {
|
||||
// Trips whose queue waits behind a write the server failed on. Their later
|
||||
// writes must keep their order, but every other trip goes on syncing.
|
||||
const blockedTrips = new Set<number>()
|
||||
// Entities whose later writes wait behind a parked one, until the user
|
||||
// tries it again or discards it. Grows as writes park during this pass.
|
||||
const heldEntities = await parkedEntities()
|
||||
|
||||
for (const mutation of pending) {
|
||||
// Re-checked every pass, not just on entry: this loop writes server
|
||||
@@ -220,6 +257,9 @@ export const mutationQueue = {
|
||||
// would otherwise seed it with the previous account's rows.
|
||||
if (!isAuthed()) break
|
||||
if (blockedTrips.has(mutation.tripId)) continue
|
||||
const key = entityKey(mutation)
|
||||
const heldSince = key === undefined ? undefined : heldEntities.get(key)
|
||||
if (heldSince !== undefined && heldSince < mutation.createdAt) continue
|
||||
if (mutation.retryAfter !== undefined && mutation.retryAfter > Date.now()) {
|
||||
blockedTrips.add(mutation.tripId)
|
||||
continue
|
||||
@@ -248,6 +288,7 @@ export const mutationQueue = {
|
||||
attempts: mutation.attempts + 1,
|
||||
lastError: 'unresolved temp id (dependent create did not sync)',
|
||||
})
|
||||
holdEntity(heldEntities, mutation)
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -365,6 +406,7 @@ export const mutationQueue = {
|
||||
status: 'conflict', conflictServer: server ?? null, conflictAt: Date.now(),
|
||||
attempts: mutation.attempts + 1, lastError: 'conflict',
|
||||
})
|
||||
holdEntity(heldEntities, mutation)
|
||||
}
|
||||
continue
|
||||
}
|
||||
@@ -380,6 +422,7 @@ export const mutationQueue = {
|
||||
attempts: mutation.attempts + 1,
|
||||
lastError: String(err),
|
||||
})
|
||||
holdEntity(heldEntities, mutation)
|
||||
} else if (httpStatus !== undefined && httpStatus >= 500) {
|
||||
// The server answered and failed. Retried with growing gaps, and
|
||||
// only this trip waits for it. One that never goes through is
|
||||
@@ -388,6 +431,7 @@ export const mutationQueue = {
|
||||
if (attempts >= MAX_SERVER_ERROR_ATTEMPTS) {
|
||||
await dropTempRows(mutation)
|
||||
await offlineDb.mutationQueue.update(mutation.id, { status: 'failed', attempts, lastError: String(err), retryAfter: undefined })
|
||||
holdEntity(heldEntities, mutation)
|
||||
} else {
|
||||
await offlineDb.mutationQueue.update(mutation.id, {
|
||||
status: 'pending',
|
||||
@@ -455,7 +499,12 @@ export const mutationQueue = {
|
||||
.count()
|
||||
},
|
||||
|
||||
/** Put every parked change back in line, from a fresh start, and flush. */
|
||||
/**
|
||||
* Put every parked change back in line, from a fresh start, and flush. The
|
||||
* later writes to the same entities waited behind them, so the replay keeps
|
||||
* the order they were made in, and a place or packing item still sends the
|
||||
* token it was edited against, which a newer server version refuses (409).
|
||||
*/
|
||||
async retryFailed(): Promise<void> {
|
||||
await offlineDb.mutationQueue
|
||||
.where('status')
|
||||
@@ -466,10 +515,12 @@ export const mutationQueue = {
|
||||
|
||||
/**
|
||||
* Drop every parked change. The local copy of an edit that never reached the
|
||||
* server goes with the next trip sync, which reads the server's version.
|
||||
* server goes with the next trip sync, which reads the server's version. The
|
||||
* later writes that waited behind a parked one are sent right away.
|
||||
*/
|
||||
async discardFailed(): Promise<void> {
|
||||
await offlineDb.mutationQueue.where('status').equals('failed').delete()
|
||||
await this.flush()
|
||||
},
|
||||
|
||||
/** Count unresolved sync conflicts (offline edits the server rejected as stale). */
|
||||
@@ -509,6 +560,8 @@ export const mutationQueue = {
|
||||
if (!m || m.status !== 'conflict') return
|
||||
await applyServerEntity(m, m.conflictServer)
|
||||
await offlineDb.mutationQueue.delete(id)
|
||||
// Later writes to the same entity waited behind the conflict.
|
||||
await this.flush()
|
||||
},
|
||||
|
||||
/** Reset internal flushing flag and timestamp counters — useful in tests. */
|
||||
|
||||
@@ -163,6 +163,55 @@ describe('mutationQueue — chained edits to the same entity', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('mutationQueue: a later edit waits behind a conflict on the same entity', () => {
|
||||
/** A compare-and-swap server where someone else already moved place 42 on to 'T1'. */
|
||||
function casServer() {
|
||||
let token = 'T1'
|
||||
const seen: string[] = []
|
||||
let serverPlace = { ...buildPlace({ trip_id: 1, id: 42, name: 'Theirs' }), updated_at: token } as Record<string, unknown>
|
||||
server.use(http.put('/api/trips/1/places/42', async ({ request }) => {
|
||||
const base = request.headers.get('X-Base-Updated-At')
|
||||
const body = await request.json() as { name: string }
|
||||
if (base && base !== token) return HttpResponse.json({ error: 'conflict', server: serverPlace }, { status: 409 })
|
||||
seen.push(body.name)
|
||||
token = `${token}+`
|
||||
serverPlace = { ...serverPlace, ...body, updated_at: token }
|
||||
return HttpResponse.json({ place: serverPlace })
|
||||
}))
|
||||
return seen
|
||||
}
|
||||
|
||||
it('keep-mine sends the conflicting edit first, then the later one on the fresh token', async () => {
|
||||
const seen = casServer()
|
||||
await mutationQueue.enqueue({ id: 'a', tripId: 1, method: 'PUT', url: '/trips/1/places/42', body: { name: 'First' }, resource: 'places', entityId: 42, baseUpdatedAt: 'T0' })
|
||||
await mutationQueue.enqueue({ id: 'b', tripId: 1, method: 'PUT', url: '/trips/1/places/42', body: { name: 'Second' }, resource: 'places', entityId: 42, baseUpdatedAt: 'T0' })
|
||||
|
||||
await mutationQueue.flush()
|
||||
expect((await offlineDb.mutationQueue.get('a'))!.status).toBe('conflict')
|
||||
// Held, not sent: replaying it now and 'a' later would undo it.
|
||||
expect((await offlineDb.mutationQueue.get('b'))!).toMatchObject({ status: 'pending', attempts: 0 })
|
||||
|
||||
await mutationQueue.resolveKeepMine('a')
|
||||
|
||||
expect(seen).toEqual(['First', 'Second'])
|
||||
expect(await offlineDb.mutationQueue.count()).toBe(0)
|
||||
expect((await offlineDb.places.get(42))!.name).toBe('Second')
|
||||
})
|
||||
|
||||
it('keep-theirs lets the later edit go, which the server judges on its own token', async () => {
|
||||
casServer()
|
||||
await mutationQueue.enqueue({ id: 'a', tripId: 1, method: 'PUT', url: '/trips/1/places/42', body: { name: 'First' }, resource: 'places', entityId: 42, baseUpdatedAt: 'T0' })
|
||||
await mutationQueue.enqueue({ id: 'b', tripId: 1, method: 'PUT', url: '/trips/1/places/42', body: { name: 'Second' }, resource: 'places', entityId: 42, baseUpdatedAt: 'T0' })
|
||||
await mutationQueue.flush()
|
||||
|
||||
await mutationQueue.resolveKeepServer('a')
|
||||
|
||||
expect(await offlineDb.mutationQueue.get('a')).toBeUndefined()
|
||||
// Made against the same stale version, so it is a conflict of its own.
|
||||
expect((await offlineDb.mutationQueue.get('b'))!.status).toBe('conflict')
|
||||
})
|
||||
})
|
||||
|
||||
describe('mutationQueue — auto strategies', () => {
|
||||
it('"server" adopts the server version automatically', async () => {
|
||||
setConflictStrategy('server')
|
||||
|
||||
@@ -582,6 +582,7 @@ describe('mutationQueue: parked changes', () => {
|
||||
await mutationQueue.enqueue(makeMutation({ id: parked }));
|
||||
await mutationQueue.enqueue(makeMutation({ id: waiting }));
|
||||
await offlineDb.mutationQueue.update(parked, { status: 'failed' });
|
||||
server.use(http.post('/api/trips/1/places', () => HttpResponse.json({ error: 'down' }, { status: 503 })));
|
||||
|
||||
await mutationQueue.discardFailed();
|
||||
|
||||
@@ -589,3 +590,127 @@ describe('mutationQueue: parked changes', () => {
|
||||
expect(await offlineDb.mutationQueue.get(waiting)).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('mutationQueue: writes to one entity keep their order around a parked one', () => {
|
||||
function editPlace(id: string, name: string, placeId = 5) {
|
||||
return makeMutation({ id, method: 'PUT', url: `/trips/1/places/${placeId}`, body: { name }, entityId: placeId });
|
||||
}
|
||||
|
||||
/** Answers every PUT of place 5 and 6 with `status`, recording the names in arrival order. */
|
||||
function recordPuts(status: number) {
|
||||
const seen: string[] = [];
|
||||
const answer = async (request: Request, id: number) => {
|
||||
const { name } = await request.json() as { name: string };
|
||||
seen.push(name);
|
||||
return status < 300
|
||||
? HttpResponse.json({ place: buildPlace({ trip_id: 1, id, name }) })
|
||||
: HttpResponse.json({ error: 'boom' }, { status });
|
||||
};
|
||||
server.use(
|
||||
http.put('/api/trips/1/places/5', ({ request }) => answer(request, 5)),
|
||||
http.put('/api/trips/1/places/6', ({ request }) => answer(request, 6)),
|
||||
);
|
||||
return seen;
|
||||
}
|
||||
|
||||
it('a write parked in this pass holds back the later write to the same entity', async () => {
|
||||
const first = generateUUID();
|
||||
const second = generateUUID();
|
||||
await mutationQueue.enqueue(editPlace(first, 'A'));
|
||||
await mutationQueue.enqueue(editPlace(second, 'B'));
|
||||
await offlineDb.mutationQueue.update(first, { attempts: 7 });
|
||||
const seen = recordPuts(500);
|
||||
|
||||
await mutationQueue.flush();
|
||||
|
||||
expect(seen).toEqual(['A']);
|
||||
expect(await offlineDb.mutationQueue.get(first)).toMatchObject({ status: 'failed' });
|
||||
expect(await offlineDb.mutationQueue.get(second)).toMatchObject({ status: 'pending', attempts: 0 });
|
||||
});
|
||||
|
||||
it('Try again replays the parked write first, so it cannot overwrite the newer one', async () => {
|
||||
const first = generateUUID();
|
||||
const second = generateUUID();
|
||||
await mutationQueue.enqueue(editPlace(first, 'A'));
|
||||
await mutationQueue.enqueue(editPlace(second, 'B'));
|
||||
await offlineDb.mutationQueue.update(first, { status: 'failed', attempts: 8, lastError: 'boom' });
|
||||
|
||||
const held = recordPuts(200);
|
||||
await mutationQueue.flush();
|
||||
expect(held).toEqual([]);
|
||||
|
||||
await mutationQueue.retryFailed();
|
||||
expect(held).toEqual(['A', 'B']);
|
||||
expect(await offlineDb.mutationQueue.count()).toBe(0);
|
||||
expect((await offlineDb.places.get(5))!.name).toBe('B');
|
||||
});
|
||||
|
||||
it('a terminal 4xx holds back the later write as well', async () => {
|
||||
const first = generateUUID();
|
||||
const second = generateUUID();
|
||||
await mutationQueue.enqueue(editPlace(first, 'A'));
|
||||
await mutationQueue.enqueue(editPlace(second, 'B'));
|
||||
const seen = recordPuts(400);
|
||||
|
||||
await mutationQueue.flush();
|
||||
|
||||
expect(seen).toEqual(['A']);
|
||||
expect(await offlineDb.mutationQueue.get(second)).toMatchObject({ status: 'pending' });
|
||||
});
|
||||
|
||||
it('other entities and older writes are not held back', async () => {
|
||||
const older = generateUUID();
|
||||
const parked = generateUUID();
|
||||
const other = generateUUID();
|
||||
await mutationQueue.enqueue(editPlace(older, 'old'));
|
||||
await mutationQueue.enqueue(editPlace(parked, 'A'));
|
||||
await mutationQueue.enqueue(editPlace(other, 'C', 6));
|
||||
await offlineDb.mutationQueue.update(parked, { status: 'failed', attempts: 8 });
|
||||
const seen = recordPuts(200);
|
||||
|
||||
await mutationQueue.flush();
|
||||
|
||||
expect(seen).toEqual(['old', 'C']);
|
||||
expect(await offlineDb.mutationQueue.get(parked)).toMatchObject({ status: 'failed' });
|
||||
});
|
||||
|
||||
it('Discard sends the writes that waited behind the parked one', async () => {
|
||||
const first = generateUUID();
|
||||
const second = generateUUID();
|
||||
await mutationQueue.enqueue(editPlace(first, 'A'));
|
||||
await mutationQueue.enqueue(editPlace(second, 'B'));
|
||||
await offlineDb.mutationQueue.update(first, { status: 'failed', attempts: 8 });
|
||||
const seen = recordPuts(200);
|
||||
|
||||
await mutationQueue.discardFailed();
|
||||
|
||||
expect(seen).toEqual(['B']);
|
||||
expect(await offlineDb.mutationQueue.count()).toBe(0);
|
||||
});
|
||||
|
||||
it('writes queued against a parked offline create wait for it instead of failing', async () => {
|
||||
const create = generateUUID();
|
||||
const edit = generateUUID();
|
||||
const tempId = nextTempId();
|
||||
await mutationQueue.enqueue(makeMutation({ id: create, tempId }));
|
||||
await mutationQueue.enqueue(makeMutation({
|
||||
id: edit, method: 'PUT', url: '/trips/1/places/{id}', body: { name: 'Renamed' }, entityId: tempId, tempEntityId: tempId,
|
||||
}));
|
||||
await offlineDb.mutationQueue.update(create, { attempts: 7 });
|
||||
server.use(http.post('/api/trips/1/places', () => HttpResponse.json({ error: 'boom' }, { status: 500 })));
|
||||
|
||||
await mutationQueue.flush();
|
||||
expect(await offlineDb.mutationQueue.get(create)).toMatchObject({ status: 'failed' });
|
||||
expect(await offlineDb.mutationQueue.get(edit)).toMatchObject({ status: 'pending', attempts: 0 });
|
||||
|
||||
const sent: string[] = [];
|
||||
server.use(
|
||||
http.post('/api/trips/1/places', () => { sent.push('create'); return HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 321 }) }); }),
|
||||
http.put('/api/trips/1/places/321', () => { sent.push('edit'); return HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 321, name: 'Renamed' }) }); }),
|
||||
);
|
||||
await mutationQueue.retryFailed();
|
||||
|
||||
expect(sent).toEqual(['create', 'edit']);
|
||||
expect(await offlineDb.mutationQueue.count()).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -95,7 +95,7 @@ If a change you made offline collides with a newer change on the server, it is s
|
||||
|
||||
### Stats & cache
|
||||
|
||||
The stats panel shows cached trips, pending changes, conflicts and failed changes. A change the server keeps refusing with a server error is retried with growing gaps (from 30 seconds up to about two hours) and only holds back the other changes of its own trip; after eight attempts it counts as failed. Failed changes come with **Try again** and **Discard**: discarding keeps the server's version, and the next sync puts it back on the device. **Clear cache** removes all offline data from IndexedDB after you confirm it in TREK's dialog (you can re-sync any time while online). Each cached trip entry shows its date range, place/file count and last successful sync.
|
||||
The stats panel shows cached trips, pending changes, conflicts and failed changes. A change the server keeps refusing with a server error is retried with growing gaps (from 30 seconds up to about two hours) and only holds back the other changes of its own trip; after eight attempts it counts as failed. Later changes to the same item (the same place, packing item, visit, tour or driving settings) wait behind a failed change or an open conflict instead of overtaking it, so they still reach the server in the order you made them. Failed changes come with **Try again** and **Discard**: trying again sends them first and then the changes that waited behind them; discarding keeps the server's version, sends the changes that waited, and the next sync puts the server's version back on the device. **Clear cache** removes all offline data from IndexedDB after you confirm it in TREK's dialog (you can re-sync any time while online). Each cached trip entry shows its date range, place/file count and last successful sync.
|
||||
|
||||
## Limitations
|
||||
|
||||
|
||||
Reference in New Issue
Block a user