From 0eda2db66b562307068c248f95472ce33dded9dd Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 13:28:32 -0500 Subject: [PATCH 01/11] Test that mutate() waits for a slow native enqueue A native enqueue that answers after enqueueTimeoutMs must resolve mutate(), not reject it. Native's own rejection must still reject. The warning fires one time, and the handler still runs after the caller has the id. These tests fail on the current code, which rejects at the timeout. Co-Authored-By: Claude Fable 5.2 --- src/__tests__/client.test.ts | 66 +++++++++++++++++++++++++ src/__tests__/registry.test.ts | 89 ++++++++++++++++++++++++++++------ 2 files changed, 139 insertions(+), 16 deletions(-) diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index 6ea7e408..b807d1ba 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -636,6 +636,72 @@ describe('end to end', () => { expect(order).toEqual([`caller ${id}`, `handler ${id}`]); expect(native.ackEvents).toHaveBeenCalledWith(['e1']); }); + + it('waits past enqueueTimeoutMs for a slow native, warns once, and still runs the handler after the caller', async () => { + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const client = createUploadClient(); + const order: string[] = []; + const create = client.define({ + key: 'item.create', + request: ({ n }: { n: number }) => ({ + url: `https://x/${n}`, + data: { n }, + }), + onSuccess: (_data, _vars, meta) => { + order.push(`handler ${meta.id}`); + }, + }); + client.configure({ enqueueTimeoutMs: 30 }); + await flush(); + let resolveEnqueue!: (id: string) => void; + native.enqueue.mockImplementationOnce( + (entry: { id: string }) => + new Promise((resolve) => { + resolveEnqueue = () => resolve(entry.id); + }), + ); + const pending = create.mutate({ n: 1 }).then( + ({ id }) => { + order.push(`caller ${id}`); + return id; + }, + (e: Error) => { + order.push(`rejected ${e.message}`); + }, + ); + await flush(); + const { id } = native.enqueue.mock.calls.at(-1)![0]; + // Native stored the entry and settled it, but its enqueue answer is + // late, as when the iOS queue is busy with another chunked file. + fire('settled', { + eventId: 'e1', + id, + key: 'item.create', + vars: { n: 1 }, + at: 10, + attempts: 1, + kind: 'completed', + response: { status: 200, bodyTruncated: false }, + state: 'completed', + }); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(order).toEqual([]); + expect(native.ackEvents).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]![0]).toMatch( + `native enqueue for "item.create" (id ${id}) has not answered after 30 ms`, + ); + resolveEnqueue(id); + await pending; + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(order).toEqual([`caller ${id}`, `handler ${id}`]); + expect(native.ackEvents).toHaveBeenCalledWith(['e1']); + expect(warn).toHaveBeenCalledTimes(1); + } finally { + warn.mockRestore(); + } + }); }); describe('with a fake native', () => { diff --git a/src/__tests__/registry.test.ts b/src/__tests__/registry.test.ts index a5b41aa5..7ce3756c 100644 --- a/src/__tests__/registry.test.ts +++ b/src/__tests__/registry.test.ts @@ -812,30 +812,81 @@ describe('uuidV4', () => { }); }); -describe('enqueue watchdog', () => { +describe('slow native enqueue', () => { beforeEach(() => jest.useFakeTimers()); afterEach(() => jest.useRealTimers()); - it('rejects and warns when native enqueue never settles', async () => { - const { define, enqueue, warn } = setup({ enqueueTimeoutMs: 10_000 }); - enqueue.mockImplementation(() => new Promise(() => undefined)); + const deferred = () => { + let resolve!: (id: string) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; + }; + + /** Settles to 'pending' while `promise` has not settled. */ + const peek = async (promise: Promise): Promise => + Promise.race([promise, Promise.resolve('pending')]); + + it('warns once at enqueueTimeoutMs, keeps waiting, then resolves with native\'s answer', async () => { + const { define, enqueue, warn, lastEntry } = setup({ + enqueueTimeoutMs: 10_000, + }); + const answer = deferred(); + enqueue.mockImplementation(() => answer.promise); const defined = define({ key: 'item.create', request: jsonPost }); const promise = defined.mutate({ n: 1 }); - // Attach the handler before the clock moves, so the rejection is observed. const outcome = promise.then( - () => 'resolved', + (value) => value, (e: Error) => e.message, ); await jest.advanceTimersByTimeAsync(9_999); expect(warn).not.toHaveBeenCalled(); await jest.advanceTimersByTimeAsync(1); - expect(await outcome).toMatch( - /native enqueue for "item.create" \(id [^)]+\) did not settle within 10000 ms\. Native did not answer; the entry may still be persisted\.$/, + expect(warn).toHaveBeenCalledTimes(1); + const { id } = lastEntry(); + expect(warn.mock.calls[0][0]).toBe( + `mutate: native enqueue for "item.create" (id ${id}) has not answered after 10000 ms. mutate() keeps waiting for the answer, and outcomes for this id wait with it.`, + ); + expect(await peek(outcome)).toBe('pending'); + await jest.advanceTimersByTimeAsync(60_000); + expect(await peek(outcome)).toBe('pending'); + expect(warn).toHaveBeenCalledTimes(1); + answer.resolve(id); + expect(await outcome).toEqual({ id }); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it("rejects with native's own error when native rejects after the warning", async () => { + const { define, enqueue, warn } = setup({ enqueueTimeoutMs: 1_000 }); + const answer = deferred(); + enqueue.mockImplementation(() => answer.promise); + const defined = define({ key: 'item.create', request: jsonPost }); + const outcome = defined.mutate({ n: 1 }).then( + () => 'resolved', + (e: unknown) => e, ); + await jest.advanceTimersByTimeAsync(5_000); expect(warn).toHaveBeenCalledTimes(1); + const storage = Object.assign(new Error('disk full'), { code: 'E_STORAGE' }); + answer.reject(storage); + expect(await outcome).toBe(storage); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it("rejects with native's own error and does not warn when native rejects in time", async () => { + const { define, enqueue, warn } = setup({ enqueueTimeoutMs: 10_000 }); + const invalid = Object.assign(new Error('bad url'), { code: 'E_INVALID' }); + enqueue.mockRejectedValue(invalid); + const defined = define({ key: 'item.create', request: jsonPost }); + await expect(defined.mutate({ n: 1 })).rejects.toBe(invalid); + await jest.advanceTimersByTimeAsync(20_000); + expect(warn).not.toHaveBeenCalled(); }); - it('clears the watchdog when native settles in time', async () => { + it('does not warn when native settles in time', async () => { const { define, warn } = setup({ enqueueTimeoutMs: 10_000 }); const defined = define({ key: 'item.create', request: jsonPost }); await expect(defined.mutate({ n: 1 })).resolves.toEqual({ @@ -845,14 +896,20 @@ describe('enqueue watchdog', () => { expect(warn).not.toHaveBeenCalled(); }); - it('hands delivery the raced promise, so a timed-out id does not block delivery', async () => { + it("hands delivery a promise that settles only with native's answer", async () => { const { define, enqueue, trackMutate } = setup({ enqueueTimeoutMs: 1_000 }); - enqueue.mockImplementation(() => new Promise(() => undefined)); + const answer = deferred(); + enqueue.mockImplementation(() => answer.promise); const defined = define({ key: 'item.create', request: jsonPost }); - const promise = defined.mutate({ n: 1 }).catch(() => 'timed out'); - await jest.advanceTimersByTimeAsync(1_000); - expect(await promise).toBe('timed out'); - const tracked = trackMutate.mock.calls[0]![1] as Promise; - await expect(tracked).rejects.toThrow(/did not settle/); + const promise = defined.mutate({ n: 1 }); + const [trackedId, tracked] = trackMutate.mock.calls[0]! as [ + string, + Promise, + ]; + await jest.advanceTimersByTimeAsync(30_000); + expect(await peek(tracked)).toBe('pending'); + answer.resolve(trackedId); + await expect(tracked).resolves.toBe(trackedId); + await expect(promise).resolves.toEqual({ id: trackedId }); }); }); From afc53370df559735c53746d40b6aac159c28f28e Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 13:28:33 -0500 Subject: [PATCH 02/11] Make mutate() wait for native's answer instead of timing out mutate() rejected when native enqueue had not answered within enqueueTimeoutMs. Native could still store the entry and upload it, so the app recorded a failure for a request that then went through. On iOS a large chunked file can hold the native queue long enough to cause this. Now mutate() warns one time at enqueueTimeoutMs and keeps waiting. It settles only with native's answer. Delivery for the id waits on the same promise, so a handler still never runs before the caller has the id. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 15 +++++++++++ README.md | 10 +++++--- src/registry.ts | 66 +++++++++++++++++++++---------------------------- src/types.ts | 9 ++++--- 4 files changed, 55 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 008df890..9eddd7c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ ## Unreleased +`mutate()` no longer rejects when native is slow to answer. Before, it +rejected after `enqueueTimeoutMs` (default 10 s) even though native could +still store the entry and upload it. On iOS a large chunked file can hold +the native queue while its part tasks are built, so the next `mutate()` +timed out, the app recorded a failure, and the request uploaded anyway. +Now `mutate()` warns one time at `enqueueTimeoutMs` and keeps waiting. It +resolves when native stored the entry and rejects only with native's own +error (`E_INVALID`, `E_RUNNING`, `E_FILE_MISSING`, `E_STORAGE`). An app that +treated the timeout rejection as a failure no longer gets it. The outcomes +of that id wait for the answer too, so a handler still never runs before +the caller has the id. `enqueueTimeoutMs` stays and sets when the warning +fires. If native never answers, which is a native bug, `mutate()` stays +pending and the warning is the only signal. Do not await `mutate()` where a +wait blocks other work. + iOS no longer deletes a chunked upload's file when `enqueue` cannot save the entry, for example on a full disk. The library had already moved the caller's file into its queue, so this deleted the only copy. The file now diff --git a/README.md b/README.md index 2709ebf5..db7716d4 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,9 @@ while the app was dead from being sent twice. `error` with `errorKind: 'expired'` when it is resumed. A same-id `mutate()` on a settled entry reopens it, and it settles once more. 4. **Never before `mutate()` resolves.** Delivery for an id waits for the - caller's promise. + caller's promise. While native has not answered `enqueue`, the id's + outcomes wait too. They stay journaled and unacknowledged, so none is + lost. 5. **Replay starts after `configure()`.** Outcomes journaled by a dead session deliver then. Call every `define()` first. 6. **Unknown key is loud.** An outcome whose key has no definition stays @@ -374,7 +376,9 @@ copy are on disk. Rejects on a malformed descriptor, an unknown descriptor field, a GET with a body, input native cannot send (`E_INVALID`), a missing file (`E_FILE_MISSING`), a storage failure (`E_STORAGE`), a running entry with a different body (`E_RUNNING`), or `vars` over -`maxVarsBytes` (1 MB by default). Only `vars` are capped. `id` defaults to a UUID. For a definition +`maxVarsBytes` (1 MB by default). Only `vars` are capped. `mutate()` has no +timeout: it settles only with native's answer, and warns one time when that +takes longer than `enqueueTimeoutMs`. `id` defaults to a UUID. For a definition whose `request` takes no vars, call `mutate()` with no arguments; native stores `null`. @@ -389,7 +393,7 @@ outcomes. A second call updates the settings and does not replay again. | `maxVarsBytes` | Cap on the JSON length of `vars`. Default 1 MB. `mutate()` rejects above it. JS-side only. | | `retry` | `{ backoff?: { baseMs, maxMs, jitter }, terminalHttp?: { exempt } }`. Each of the two objects is optional, but one you give must be complete. Defaults 1 s, 2 h, 0.2, `[404]`. | | `headers` | `() => Record`, called at `mutate()`. The descriptor merges over it. | -| `enqueueTimeoutMs` | Default 10 s. `mutate()` rejects and warns when native enqueue has not settled by then. Enqueue includes the time to stage a copy of a `file` body and of form `path` parts, so a large file takes longer. A timeout means native did not answer, not that the request failed. A watchdog for a native bug, not a tuning knob. | +| `enqueueTimeoutMs` | Default 10 s. `mutate()` warns one time when native enqueue has not answered by then, and keeps waiting. It does not make `mutate()` reject. Enqueue includes the time to stage a copy of a `file` body and of form `path` parts, and on iOS it waits behind earlier enqueues, so a large file takes longer. | | `android` | Notification text and identity: `notificationId/Title/TitleNoWifi/TitleNoInternet/Channel`. Persisted natively. | ### `pause(scope?): Promise` and `resume(scope?): Promise` diff --git a/src/registry.ts b/src/registry.ts index b35ea1a8..505b88dd 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -13,11 +13,11 @@ import type { export const DEFAULT_LIFETIME_MS = 14 * 24 * 60 * 60 * 1000; /** - * How long mutate() waits for native enqueue() before it rejects. Enqueue - * includes the time to stage a copy of a `file` body and of form `path` - * parts, so a large file takes longer. A timeout means native did not - * answer, not that the request failed. This is a watchdog for a native path - * that never settles, not a tuning knob. + * How long mutate() waits for native enqueue() before it warns. mutate() + * keeps waiting after the warning and settles only with native's answer. + * Enqueue includes the time to stage a copy of a `file` body and of form + * `path` parts, and on iOS it waits behind earlier enqueues, so a large file + * takes longer. */ export const DEFAULT_ENQUEUE_TIMEOUT_MS = 10_000; /** @@ -61,25 +61,19 @@ export type Settings = { retry?: Partial; }; -/** Rejects with `makeError()` when `promise` has not settled after `ms`. */ -export const withTimeout = ( - promise: Promise, +/** + * Calls `onSlow` one time when `promise` has not settled after `ms`. Does not + * change how `promise` settles. + */ +export const warnIfSlow = ( + promise: Promise, ms: number, - makeError: () => Error, -): Promise => - new Promise((resolve, reject) => { - const timer = setTimeout(() => reject(makeError()), ms); - promise.then( - (value) => { - clearTimeout(timer); - resolve(value); - }, - (error: unknown) => { - clearTimeout(timer); - reject(error); - }, - ); - }); + onSlow: () => void, +): void => { + const timer = setTimeout(onSlow, ms); + const clear = () => clearTimeout(timer); + promise.then(clear, clear); +}; /** * The descriptor as it crosses to native. `dataJson` is `JSON.stringify(data)` @@ -573,21 +567,17 @@ export const createRegistry = ({ expiresAt: descriptor.expiresAt ?? now() + settings.lifetimeMs, }, }; - // Watchdog. Turn a native enqueue() that never settles into a rejection - // with a name, and let delivery for this id proceed instead of waiting - // forever. Enqueue includes the time to stage a copy of a `file` body - // and of form `path` parts, so a large file takes longer. A timeout - // means native did not answer, not that the request failed. If native - // did persist the entry, its outcome still reaches the handlers, and a - // same-id retry resumes instead of duplicating. - const pending = withTimeout( - native.enqueue(entry), - settings.enqueueTimeoutMs, - () => { - const message = `mutate: native enqueue for "${key}" (id ${entry.id}) did not settle within ${settings.enqueueTimeoutMs} ms. Native did not answer; the entry may still be persisted.`; - warn(message); - return new Error(message); - }, + // mutate() settles only with native's answer. A late answer is not a + // failure: native may store the entry after any wait, and a rejection + // here would report a failure for a request that then uploads. So a + // slow answer gets one warning, and mutate() keeps waiting. Delivery + // for this id waits on the same promise, so no handler runs before the + // caller has the id. + const pending = native.enqueue(entry); + warnIfSlow(pending, settings.enqueueTimeoutMs, () => + warn( + `mutate: native enqueue for "${key}" (id ${entry.id}) has not answered after ${settings.enqueueTimeoutMs} ms. mutate() keeps waiting for the answer, and outcomes for this id wait with it.`, + ), ); trackMutate(entry.id, pending); // The JS id is the one the caller may have chosen and the one the diff --git a/src/types.ts b/src/types.ts index a8bd7525..becb1f2a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -332,10 +332,11 @@ export type ConfigureOptions = { headers?: () => Record; /** * Default 10 s. How long `mutate()` waits for native enqueue before it - * rejects. Enqueue includes the time to stage a copy of a `file` body and - * of form `path` parts, so a large file takes longer. A timeout means - * native did not answer, not that the request failed. A watchdog for a - * native bug, not a tuning knob. + * warns one time. `mutate()` keeps waiting after the warning: it resolves + * when native stored the entry and rejects only with native's own error. + * Enqueue includes the time to stage a copy of a `file` body and of form + * `path` parts, and on iOS it waits behind earlier enqueues, so a large + * file takes longer. */ enqueueTimeoutMs?: number; android?: Partial; From eeb602b01af7b8deaf2efba90212ccb49b0d4fed Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 13:39:53 -0500 Subject: [PATCH 03/11] Add onWarning and holdNext, and fix docs after review - configure({ onWarning }) receives every library warning instead of console.warn, so an app can send a slow-enqueue warning to its error tracker. The client builds one warn function that reads the sink at each call, so a sink set by configure() is used by later warnings. A throw from the sink falls back to console.warn. - createFakeNative().holdNext('enqueue') keeps the next enqueue pending until released, so a test can model a slow native. - Android also runs enqueue on one thread, so the docs no longer say that only iOS waits behind earlier enqueues. - The CHANGELOG marks the mutate() change as a behavior change and says that only a rejection from native is native's error. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 17 ++- README.md | 8 +- src/__tests__/client.test.ts | 227 ++++++++++++++++++++++++---------- src/__tests__/testing.test.ts | 72 +++++++++++ src/__typetests__/define.ts | 13 ++ src/delivery.ts | 2 +- src/index.ts | 36 +++++- src/registry.ts | 5 +- src/testing.ts | 111 +++++++++++++---- src/types.ts | 12 +- 10 files changed, 401 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9eddd7c2..1753a72b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,14 @@ ## Unreleased -`mutate()` no longer rejects when native is slow to answer. Before, it +Behavior change: `mutate()` no longer rejects when native is slow to answer. Before, it rejected after `enqueueTimeoutMs` (default 10 s) even though native could still store the entry and upload it. On iOS a large chunked file can hold the native queue while its part tasks are built, so the next `mutate()` timed out, the app recorded a failure, and the request uploaded anyway. Now `mutate()` warns one time at `enqueueTimeoutMs` and keeps waiting. It -resolves when native stored the entry and rejects only with native's own -error (`E_INVALID`, `E_RUNNING`, `E_FILE_MISSING`, `E_STORAGE`). An app that +resolves when native stored the entry. After it reaches native, it rejects +only with native's own error (`E_INVALID`, `E_RUNNING`, `E_FILE_MISSING`, +`E_STORAGE`). An app that treated the timeout rejection as a failure no longer gets it. The outcomes of that id wait for the answer too, so a handler still never runs before the caller has the id. `enqueueTimeoutMs` stays and sets when the warning @@ -15,6 +16,16 @@ fires. If native never answers, which is a native bug, `mutate()` stays pending and the warning is the only signal. Do not await `mutate()` where a wait blocks other work. +Added `configure({ onWarning })`. When set, every library warning goes to it +instead of `console.warn`, with the error or event as a second argument. An +app can send the slow-enqueue warning, and the others, to its error tracker. +Warnings raised before `configure()` still go to `console.warn`, and so does +a warning whose sink throws. + +Added `holdNext('enqueue')` to `createFakeNative()`. The next enqueue stays +pending until the returned function is called, so a test can model a native +that is slow to answer. + iOS no longer deletes a chunked upload's file when `enqueue` cannot save the entry, for example on a full disk. The library had already moved the caller's file into its queue, so this deleted the only copy. The file now diff --git a/README.md b/README.md index db7716d4..e92a9055 100644 --- a/README.md +++ b/README.md @@ -393,7 +393,8 @@ outcomes. A second call updates the settings and does not replay again. | `maxVarsBytes` | Cap on the JSON length of `vars`. Default 1 MB. `mutate()` rejects above it. JS-side only. | | `retry` | `{ backoff?: { baseMs, maxMs, jitter }, terminalHttp?: { exempt } }`. Each of the two objects is optional, but one you give must be complete. Defaults 1 s, 2 h, 0.2, `[404]`. | | `headers` | `() => Record`, called at `mutate()`. The descriptor merges over it. | -| `enqueueTimeoutMs` | Default 10 s. `mutate()` warns one time when native enqueue has not answered by then, and keeps waiting. It does not make `mutate()` reject. Enqueue includes the time to stage a copy of a `file` body and of form `path` parts, and on iOS it waits behind earlier enqueues, so a large file takes longer. | +| `enqueueTimeoutMs` | Default 10 s. `mutate()` warns one time when native enqueue has not answered by then, and keeps waiting. It does not make `mutate()` reject. Enqueue includes the time to stage a copy of a `file` body and of form `path` parts, and it waits behind earlier enqueues, so a large file takes longer. | +| `onWarning` | `(message, detail?) => void`. Receives every library warning instead of `console.warn`: a slow enqueue, a slow or rejected handler, a failed ack, and the rest. `detail` is the error or event, when there is one. Warnings raised before `configure()` go to `console.warn`. If the sink throws, the warning and the throw go to `console.warn`. | | `android` | Notification text and identity: `notificationId/Title/TitleNoWifi/TitleNoInternet/Channel`. Persisted natively. | ### `pause(scope?): Promise` and `resume(scope?): Promise` @@ -595,7 +596,10 @@ re-emit) or the header merge of `updateHeaders()`; script those with `native.entries` holds every enqueue as `mutate()` sent it, with `vars` and `data` parsed back from JSON. `failNext()` makes the next native call reject -with a code such as `E_STORAGE`. `seedUnacknowledged()` and `seedRows()` +with a code such as `E_STORAGE`. `holdNext('enqueue')` keeps the next enqueue +pending, as when native is slow to answer, and returns a function that +releases it. The released call then runs as usual, or rejects with a failure +that `failNext()` queued. `reset()` drops held calls. `seedUnacknowledged()` and `seedRows()` model a journal and rows left by a dead session, so `configure()` replays them. diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index b807d1ba..9606758d 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -638,69 +638,170 @@ describe('end to end', () => { }); it('waits past enqueueTimeoutMs for a slow native, warns once, and still runs the handler after the caller', async () => { - const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); - try { - const client = createUploadClient(); - const order: string[] = []; - const create = client.define({ - key: 'item.create', - request: ({ n }: { n: number }) => ({ - url: `https://x/${n}`, - data: { n }, - }), - onSuccess: (_data, _vars, meta) => { - order.push(`handler ${meta.id}`); - }, - }); - client.configure({ enqueueTimeoutMs: 30 }); - await flush(); - let resolveEnqueue!: (id: string) => void; - native.enqueue.mockImplementationOnce( - (entry: { id: string }) => - new Promise((resolve) => { - resolveEnqueue = () => resolve(entry.id); - }), - ); - const pending = create.mutate({ n: 1 }).then( - ({ id }) => { - order.push(`caller ${id}`); - return id; - }, - (e: Error) => { - order.push(`rejected ${e.message}`); - }, - ); - await flush(); - const { id } = native.enqueue.mock.calls.at(-1)![0]; - // Native stored the entry and settled it, but its enqueue answer is - // late, as when the iOS queue is busy with another chunked file. - fire('settled', { - eventId: 'e1', - id, - key: 'item.create', - vars: { n: 1 }, - at: 10, - attempts: 1, - kind: 'completed', - response: { status: 200, bodyTruncated: false }, - state: 'completed', - }); - await new Promise((resolve) => setTimeout(resolve, 100)); - expect(order).toEqual([]); - expect(native.ackEvents).not.toHaveBeenCalled(); - expect(warn).toHaveBeenCalledTimes(1); - expect(warn.mock.calls[0]![0]).toMatch( - `native enqueue for "item.create" (id ${id}) has not answered after 30 ms`, - ); - resolveEnqueue(id); - await pending; - await new Promise((resolve) => setTimeout(resolve, 20)); - expect(order).toEqual([`caller ${id}`, `handler ${id}`]); - expect(native.ackEvents).toHaveBeenCalledWith(['e1']); - expect(warn).toHaveBeenCalledTimes(1); - } finally { - warn.mockRestore(); - } + const fake = createFakeNative(); + const client = createUploadClient({ native: fake }); + const order: string[] = []; + const onWarning = jest.fn(); + const create = client.define({ + key: 'item.create', + request: ({ n }: { n: number }) => ({ + url: `https://x/${n}`, + data: { n }, + }), + onSuccess: (_data, _vars, meta) => { + order.push(`handler ${meta.id}`); + }, + }); + client.configure({ enqueueTimeoutMs: 30, onWarning }); + await flush(); + const release = fake.holdNext('enqueue'); + const pending = create.mutate({ n: 1 }, { id: 'u1' }).then( + ({ id }) => order.push(`caller ${id}`), + (e: Error) => order.push(`rejected ${e.message}`), + ); + // Native stored the entry and settled it, but its enqueue answer is + // late, as when the native queue is busy with another chunked file. + fake.emit.settled({ + eventId: 'e1', + id: 'u1', + key: 'item.create', + vars: { n: 1 }, + at: 10, + attempts: 1, + url: 'https://x/1', + method: 'POST', + kind: 'completed', + response: { status: 200, bodyTruncated: false }, + state: 'completed', + }); + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(order).toEqual([]); + expect(fake.ackedEventIds).toEqual([]); + expect(onWarning).toHaveBeenCalledTimes(1); + expect(onWarning.mock.calls[0]![0]).toMatch( + 'native enqueue for "item.create" (id u1) has not answered after 30 ms', + ); + release(); + await pending; + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(order).toEqual(['caller u1', 'handler u1']); + expect(fake.ackedEventIds).toEqual(['e1']); + expect(onWarning).toHaveBeenCalledTimes(1); + }); +}); + +describe('onWarning', () => { + const setupClient = (onWarning?: (m: string, d?: unknown) => void) => { + const fake = createFakeNative(); + const client = createUploadClient({ native: fake }); + const failing = client.define({ + key: 'k', + request: (_vars: null) => ({ url: 'https://x' }), + onSuccess: () => { + throw new Error('handler failed'); + }, + }); + client.configure({ onWarning }); + return { fake, client, failing }; + }; + + let consoleWarn: jest.SpyInstance; + beforeEach(() => { + consoleWarn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => consoleWarn.mockRestore()); + + it('receives every library warning, with its detail, instead of console.warn', async () => { + const onWarning = jest.fn(); + const { fake, client, failing } = setupClient(onWarning); + await flush(); + const { id } = await failing.mutate(); + fake.emit.settled(fake.buildSettled(id, { kind: 'completed' })); + await flush(); + const subscription = client.addListener('state', () => { + throw new Error('listener failed'); + }); + fake.emit.settled({ + ...fake.buildSettled(id, { kind: 'completed' }), + key: 'nobody', + }); + await flush(); + subscription.remove(); + expect(onWarning.mock.calls).toEqual([ + [ + `delivery: the "k" handler for ${id} rejected. The outcome stays unacknowledged and redelivers at the next launch.`, + expect.objectContaining({ message: 'handler failed' }), + ], + [ + `delivery: no definition for key "nobody" (id ${id}). The outcome stays unacknowledged.`, + undefined, + ], + [ + 'addListener: a state listener threw', + expect.objectContaining({ message: 'listener failed' }), + ], + ]); + expect(consoleWarn).not.toHaveBeenCalled(); + }); + + it('leaves warnings on console.warn before configure() and when not set', async () => { + const fake = createFakeNative(); + const client = createUploadClient({ native: fake }); + const onWarning = jest.fn(); + const request = (_vars: null) => ({ url: 'https://x' }); + client.define({ key: 'dup', request }); + client.define({ key: 'dup', request }); + expect(consoleWarn).toHaveBeenCalledWith( + 'define: "dup" is already defined. The new definition replaces it.', + ); + const failing = client.define({ + key: 'k', + request, + onSuccess: () => { + throw new Error('handler failed'); + }, + }); + // Each configure() replaces the full configuration, so the second call + // removes the sink. + client.configure({ onWarning }); + client.configure({}); + await flush(); + const { id } = await failing.mutate(); + fake.emit.settled(fake.buildSettled(id, { kind: 'completed' })); + await flush(); + expect(onWarning).not.toHaveBeenCalled(); + expect(consoleWarn).toHaveBeenLastCalledWith( + `delivery: the "k" handler for ${id} rejected. The outcome stays unacknowledged and redelivers at the next launch.`, + expect.objectContaining({ message: 'handler failed' }), + ); + }); + + it('falls back to console.warn when the sink throws, and keeps delivering', async () => { + const thrown = new Error('sink failed'); + const onWarning = jest.fn(() => { + throw thrown; + }); + const { fake, failing } = setupClient(onWarning); + await flush(); + const { id } = await failing.mutate(); + fake.emit.settled(fake.buildSettled(id, { kind: 'completed' })); + await flush(); + expect(onWarning).toHaveBeenCalledTimes(1); + expect(consoleWarn.mock.calls).toEqual([ + ['configure: onWarning threw', thrown], + [ + `delivery: the "k" handler for ${id} rejected. The outcome stays unacknowledged and redelivers at the next launch.`, + expect.objectContaining({ message: 'handler failed' }), + ], + ]); + expect(fake.ackedEventIds).toEqual([]); + }); + + it('rejects a value that is not a function', () => { + const client = createUploadClient({ native: createFakeNative() }); + expect(() => + client.configure({ onWarning: 'log' as unknown as () => void }), + ).toThrow('configure: onWarning must be a function when given, got string'); }); }); diff --git a/src/__tests__/testing.test.ts b/src/__tests__/testing.test.ts index 6c78e9e3..d3313174 100644 --- a/src/__tests__/testing.test.ts +++ b/src/__tests__/testing.test.ts @@ -508,3 +508,75 @@ describe('reset', () => { } }); }); + +describe('holdNext', () => { + const peek = (promise: Promise): Promise => + Promise.race([promise, Promise.resolve('pending')]); + + it('holds the next enqueue until released, then stores, emits and resolves', async () => { + const fake = createFakeNative(); + const states = jest.fn(); + fake.onState(states); + const release = fake.holdNext('enqueue'); + const pending = fake.enqueue(entry()); + await flush(); + expect(await peek(pending)).toBe('pending'); + expect(fake.entries).toEqual([]); + expect(states).not.toHaveBeenCalled(); + release(); + await expect(pending).resolves.toBe('a'); + expect(fake.entries).toHaveLength(1); + expect(states).toHaveBeenCalledTimes(1); + // Only the next call is held. + await expect(fake.enqueue(entry({ id: 'b' }))).resolves.toBe('b'); + }); + + it('applies a failNext failure when the held call is released', async () => { + const fake = createFakeNative(); + const release = fake.holdNext('enqueue'); + const pending = fake.enqueue(entry()); + fake.failNext('enqueue', 'E_STORAGE', 'disk full'); + release(); + await expect(pending).rejects.toMatchObject({ + code: 'E_STORAGE', + message: 'disk full', + }); + expect(fake.entries).toEqual([]); + }); + + it('uses holds in order and ignores a second release', async () => { + const fake = createFakeNative(); + const releaseFirst = fake.holdNext('enqueue'); + const releaseSecond = fake.holdNext('enqueue'); + const first = fake.enqueue(entry({ id: 'a' })); + const second = fake.enqueue(entry({ id: 'b' })); + releaseSecond(); + await expect(second).resolves.toBe('b'); + expect(await peek(first)).toBe('pending'); + releaseFirst(); + releaseFirst(); + await expect(first).resolves.toBe('a'); + expect(fake.entries.map((e) => e.id)).toEqual(['b', 'a']); + }); + + it('cancels the hold when released before the call is made', async () => { + const fake = createFakeNative(); + const release = fake.holdNext('enqueue'); + release(); + await expect(fake.enqueue(entry())).resolves.toBe('a'); + }); + + it('drops held calls and unused holds on reset()', async () => { + const fake = createFakeNative(); + const releaseHeld = fake.holdNext('enqueue'); + const releaseUnused = fake.holdNext('enqueue'); + const pending = fake.enqueue(entry()); + fake.reset(); + releaseHeld(); + releaseUnused(); + await flush(); + expect(await peek(pending)).toBe('pending'); + expect(fake.entries).toEqual([]); + await expect(fake.enqueue(entry({ id: 'b' }))).resolves.toBe('b'); + }); +}); diff --git a/src/__typetests__/define.ts b/src/__typetests__/define.ts index dc8b63eb..6fa88d1f 100644 --- a/src/__typetests__/define.ts +++ b/src/__typetests__/define.ts @@ -297,3 +297,16 @@ client.define({ // @ts-expect-error wifiOnly is a boolean request: (_vars: null) => ({ url: 'https://x', wifiOnly: 'yes' }), }); + +// onWarning takes a message and an optional detail. +client.configure({ + onWarning: (_message, _detail) => { + assertEqual(true); + assertEqual(true); + }, +}); +client.configure({ onWarning: (_message: string) => undefined }); +// @ts-expect-error onWarning is a function +client.configure({ onWarning: 'console' }); +// @ts-expect-error the message is a string +client.configure({ onWarning: (_message: number) => undefined }); diff --git a/src/delivery.ts b/src/delivery.ts index 03eb9628..685145d1 100644 --- a/src/delivery.ts +++ b/src/delivery.ts @@ -49,7 +49,7 @@ type DeliveryDeps = { lookup: (key: string) => AnyDefinition | undefined; /** Feeds the client's `state` listeners. */ emitState: (event: StateEvent) => void; - warn?: (message: string, ...rest: unknown[]) => void; + warn?: (message: string, detail?: unknown) => void; handlerWarningMs?: number; }; diff --git a/src/index.ts b/src/index.ts index d68c4d2d..e61406ef 100644 --- a/src/index.ts +++ b/src/index.ts @@ -93,6 +93,26 @@ export const createUploadClient = ({ // registered twice is removed one subscription at a time. const stateListeners = new Set<{ listener: (event: StateEvent) => void }>(); + // Every library warning goes through here. The registry and delivery + // capture this function when the client is built, before configure(), so + // it reads the sink at each call: a sink set later is used from then on. + const warn = (message: string, detail?: unknown): void => { + const sink = settings.onWarning; + if (sink) { + try { + sink(message, detail); + return; + } catch (e) { + console.warn('configure: onWarning threw', e); + } + } + if (detail === undefined) { + console.warn(message); + } else { + console.warn(message, detail); + } + }; + // A listener that throws must not stop the others or fail the delivery // that produced the row. const emitState = (event: StateEvent): void => { @@ -100,7 +120,7 @@ export const createUploadClient = ({ try { listener(event); } catch (e) { - console.warn('addListener: a state listener threw', e); + warn('addListener: a state listener threw', e); } }); }; @@ -109,17 +129,20 @@ export const createUploadClient = ({ native, lookup: (key) => definitions.get(key), emitState, + warn, }); const { define } = createRegistry({ native, definitions, getSettings: () => settings, trackMutate: delivery.trackMutate, + warn, }); /** * One-time setup. Call it at boot, after every define() call. Stores the - * lifetime, the vars cap, the headers provider and the retry defaults, forwards the + * lifetime, the vars cap, the headers provider, the warning sink and the + * retry defaults, forwards the * lifetime, retry and Android notification settings to native, then starts * replaying journaled outcomes. A second call updates the settings and does * not replay again. Each call replaces the full configuration. @@ -147,8 +170,17 @@ export const createUploadClient = ({ ); } settings.maxVarsBytes = maxVarsBytes; + if ( + options.onWarning !== undefined && + typeof options.onWarning !== 'function' + ) { + throw new Error( + `configure: onWarning must be a function when given, got ${typeof options.onWarning}`, + ); + } settings.headers = options.headers; settings.retry = options.retry; + settings.onWarning = options.onWarning; const forwarded: Record = { lifetimeMs, ...options.android, diff --git a/src/registry.ts b/src/registry.ts index 505b88dd..2f9584b6 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -16,8 +16,8 @@ export const DEFAULT_LIFETIME_MS = 14 * 24 * 60 * 60 * 1000; * How long mutate() waits for native enqueue() before it warns. mutate() * keeps waiting after the warning and settles only with native's answer. * Enqueue includes the time to stage a copy of a `file` body and of form - * `path` parts, and on iOS it waits behind earlier enqueues, so a large file - * takes longer. + * `path` parts, and it waits behind earlier enqueues, so a large file takes + * longer. */ export const DEFAULT_ENQUEUE_TIMEOUT_MS = 10_000; /** @@ -59,6 +59,7 @@ export type Settings = { maxVarsBytes: number; headers?: () => Record; retry?: Partial; + onWarning?: (message: string, detail?: unknown) => void; }; /** diff --git a/src/testing.ts b/src/testing.ts index 093c2342..176a61f0 100644 --- a/src/testing.ts +++ b/src/testing.ts @@ -19,7 +19,8 @@ * E_RUNNING, re-emit). A second enqueue() on an id only overwrites its row. * It also does not model the header merge of updateHeaders(). It records the * patch and merges nothing. Script those cases with failNext() and - * seedUnacknowledged(). + * seedUnacknowledged(). holdNext('enqueue') models a native that is slow to + * answer. */ import type { SettledEvent } from './delivery'; import type { Spec } from './NativeRNFileUploader'; @@ -89,6 +90,9 @@ export type FailableMethod = | 'ackEvents' | 'takeDiagnostics'; +/** The methods whose next call can be held open with holdNext(). */ +export type HoldableMethod = 'enqueue'; + export type FakeNativeOptions = { /** * How long settle() waits for the ack before it rejects. Default 2000 ms. @@ -163,8 +167,18 @@ export type FakeNative = Spec & { */ failNext: (method: FailableMethod, code: string, message?: string) => void; /** - * Clears entries, rows, journal, diagnostics, calls, acks, the pause state and queued - * failures. A + * The next call of `method` stays pending, as when native is slow to + * answer. Call the returned function to release it: the call then runs as + * usual. It records the entry, emits its row and resolves, or rejects with + * a failure queued by failNext(). A failure is taken when the call is + * released, not when it is made. Releasing before the call is made cancels + * the hold. A second release does nothing. Holds are used in order. + */ + holdNext: (method: HoldableMethod) => () => void; + /** + * Clears entries, rows, journal, diagnostics, calls, acks, the pause state, queued + * failures and holds. A held call that was not released stays pending + * forever, and its release function does nothing. A * settle() that still waits for its ack resolves now, so it cannot reject * later inside another test. Keeps the subscriptions, because a client subscribes one time, at its first * configure(). New eventIds never repeat old ones, so delivery's dedupe @@ -269,6 +283,10 @@ export const createFakeNative = ( const isPaused = (key: string): boolean => pausedAll || pausedKeys.has(key); const failures = new Map(); + // A hold waits for its call in `holds`, then for its release in `held`. + type Hold = { run?: () => void; done: boolean }; + const holds: Hold[] = []; + const held = new Set(); const ackWaiters = new Map void>(); const state = makeEmitter(); @@ -404,6 +422,31 @@ export const createFakeNative = ( return acked; }; + const enqueueBody = (input: object): Promise => + run('enqueue', () => { + const raw = input as EnqueueEntry; + const vars = JSON.parse(raw.varsJson) as Json; + entries.push({ + id: raw.id, + key: raw.key, + vars, + descriptor: toDescriptor(raw.descriptor), + raw, + }); + const prior = rows.get(raw.id); + setRow({ + id: raw.id, + key: raw.key, + vars, + state: isPaused(raw.key) ? 'paused' : 'queued', + bytesSent: 0, + totalBytes: prior?.totalBytes ?? 0, + attempts: 0, + updatedAt: Date.now(), + }); + return raw.id; + }); + const fake: FakeNative = { entries, calls, @@ -413,30 +456,19 @@ export const createFakeNative = ( calls.configure.push(config); }, - enqueue: (input) => - run('enqueue', () => { - const raw = input as EnqueueEntry; - const vars = JSON.parse(raw.varsJson) as Json; - entries.push({ - id: raw.id, - key: raw.key, - vars, - descriptor: toDescriptor(raw.descriptor), - raw, - }); - const prior = rows.get(raw.id); - setRow({ - id: raw.id, - key: raw.key, - vars, - state: isPaused(raw.key) ? 'paused' : 'queued', - bytesSent: 0, - totalBytes: prior?.totalBytes ?? 0, - attempts: 0, - updatedAt: Date.now(), - }); - return raw.id; - }), + enqueue: (input) => { + const body = () => enqueueBody(input); + const hold = holds.shift(); + if (!hold) { + return body(); + } + held.add(hold); + return new Promise((resolve, reject) => { + hold.run = () => { + body().then(resolve, reject); + }; + }); + }, pause: (input) => run('pause', () => { @@ -602,6 +634,26 @@ export const createFakeNative = ( failures.set(method, queue); }, + holdNext: (method) => { + if (method !== 'enqueue') { + throw new Error(`holdNext: cannot hold ${String(method)}`); + } + const hold: Hold = { done: false }; + holds.push(hold); + return () => { + if (hold.done) { + return; + } + hold.done = true; + const waiting = holds.indexOf(hold); + if (waiting >= 0) { + holds.splice(waiting, 1); + } + held.delete(hold); + hold.run?.(); + }; + }, + reset: () => { entries.length = 0; rows.clear(); @@ -617,6 +669,11 @@ export const createFakeNative = ( calls.setWifiOnly.length = 0; calls.updateHeaders.length = 0; failures.clear(); + [...holds, ...held].forEach((hold) => { + hold.done = true; + }); + holds.length = 0; + held.clear(); // Each waiter clears its own timer and resolves its settle(). ackWaiters.forEach((waiter) => waiter()); ackWaiters.clear(); diff --git a/src/types.ts b/src/types.ts index becb1f2a..83c74d4c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -335,10 +335,18 @@ export type ConfigureOptions = { * warns one time. `mutate()` keeps waiting after the warning: it resolves * when native stored the entry and rejects only with native's own error. * Enqueue includes the time to stage a copy of a `file` body and of form - * `path` parts, and on iOS it waits behind earlier enqueues, so a large - * file takes longer. + * `path` parts, and it waits behind earlier enqueues, so a large file + * takes longer. */ enqueueTimeoutMs?: number; + /** + * Receives every library warning instead of `console.warn`: a slow native + * enqueue, a slow or rejected handler, a failed ack, and the rest. + * `detail` is the error or event the warning is about, when there is one. + * Warnings raised before `configure()` go to `console.warn`. If the sink + * throws, the warning and the throw go to `console.warn`. + */ + onWarning?: (message: string, detail?: unknown) => void; android?: Partial; }; From d24d8b77ba6f38d290e36b00092fc4d90607c99f Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 14:36:02 -0500 Subject: [PATCH 04/11] Add useRequests(): the queue rows as React hooks, cached per client The client keeps one in-memory copy of getRequests(). It reads native when the first hook or listener subscribes, and each state event replaces the row it carries. useRequestsWithProgress() overlays the bytes of progress events. Filtered results keep their array while their rows are the same. Native forgets a row with no event in two cases that this library's JS starts, so the cache drops the row itself: when the ack of a completed or cancelled outcome resolves (delivery's new onAcked), and when cancel() resolves on a settled entry. An error row stays until cancel() or a same-id mutate(), which emits a state event. define() now also returns useRequests() and useRequestsWithProgress() for the definition's key, with typed vars. The fake's settle() resolves after the client has handled the ack, so the hooks are up to date then. Co-Authored-By: Claude Opus 5.5 --- .../__tests__/useRequests.test.tsx | 212 ++++++++ example/RNBGUExample/jest.config.js | 6 + src/__tests__/client.test.ts | 4 + src/__tests__/delivery.test.ts | 94 +++- src/__tests__/requests.test.ts | 471 ++++++++++++++++++ src/__typetests__/requests.ts | 82 +++ src/delivery.ts | 24 +- src/index.ts | 90 +++- src/registry.ts | 25 +- src/requests.ts | 408 +++++++++++++++ src/testing.ts | 41 +- src/types.ts | 57 +++ 12 files changed, 1488 insertions(+), 26 deletions(-) create mode 100644 example/RNBGUExample/__tests__/useRequests.test.tsx create mode 100644 src/__tests__/requests.test.ts create mode 100644 src/__typetests__/requests.ts create mode 100644 src/requests.ts diff --git a/example/RNBGUExample/__tests__/useRequests.test.tsx b/example/RNBGUExample/__tests__/useRequests.test.tsx new file mode 100644 index 00000000..138a28e9 --- /dev/null +++ b/example/RNBGUExample/__tests__/useRequests.test.tsx @@ -0,0 +1,212 @@ +/** + * @format + */ + +import 'react-native'; +import React from 'react'; + +// Note: import explicitly to use the types shipped with jest. +import {beforeEach, expect, it, jest} from '@jest/globals'; + +// Note: test renderer must be required after react-native. +import renderer, {act, type ReactTestRenderer} from 'react-test-renderer'; + +import { + createUploadClient, + type RequestRow, + type RequestsFilter, + type UploadClient, +} from 'react-native-background-upload'; +import {createFakeNative, type FakeNative} from '../../../src/testing'; + +// The package root builds a default client over the TurboModule at load. +jest.mock('../../../src/NativeRNFileUploader', () => ({ + __esModule: true, + default: jest + .requireActual( + '../../../src/testing', + ) + .createFakeNative(), +})); + +const row = (overrides: Partial = {}): RequestRow => ({ + id: 'a', + key: 'k', + vars: null, + state: 'queued', + bytesSent: 0, + totalBytes: 100, + attempts: 0, + updatedAt: 1, + ...overrides, +}); + +let native: FakeNative; +let client: UploadClient; + +beforeEach(() => { + native = createFakeNative(); + client = createUploadClient({native}); + client.configure({}); +}); + +type Seen = {renders: number; results: Array}; + +const Rows = ({ + seen, + filter, + progress = false, +}: { + seen: Seen; + filter?: RequestsFilter; + progress?: boolean; +}) => { + const rows = progress + ? client.useRequestsWithProgress(filter) + : client.useRequests(filter); + seen.renders += 1; + seen.results.push(rows); + return null; +}; + +const newSeen = (): Seen => ({renders: 0, results: []}); +const last = (seen: Seen) => seen.results[seen.results.length - 1]!; + +const mount = (element: React.ReactElement): ReactTestRenderer => { + let tree!: ReactTestRenderer; + act(() => { + tree = renderer.create(element); + }); + return tree; +}; + +it('reads native once for two mounted hooks', () => { + native.seedRows([row()]); + const getRequests = jest.spyOn(native, 'getRequests'); + const one = newSeen(); + const two = newSeen(); + mount( + <> + + + , + ); + expect(getRequests).toHaveBeenCalledTimes(1); + expect(last(one)).toEqual([row()]); + expect(last(two)).toEqual([row()]); +}); + +it('re-renders on a state event, and only the progress hook on progress', () => { + native.seedRows([row()]); + const plain = newSeen(); + const withProgress = newSeen(); + mount( + <> + + + , + ); + const plainRenders = plain.renders; + const progressRenders = withProgress.renders; + + act(() => native.emit.progress({id: 'a', bytesSent: 40, totalBytes: 100})); + expect(plain.renders).toBe(plainRenders); + expect(withProgress.renders).toBe(progressRenders + 1); + expect(last(withProgress)[0]!.bytesSent).toBe(40); + expect(last(plain)[0]!.bytesSent).toBe(0); + + act(() => native.emit.state(row({state: 'running', updatedAt: 2}))); + expect(plain.renders).toBe(plainRenders + 1); + expect(last(plain)[0]!.state).toBe('running'); + expect(last(withProgress)[0]!.state).toBe('running'); +}); + +it('returns the same filtered array across renders while its rows hold', () => { + native.seedRows([row({id: 'a'}), row({id: 'b'})]); + const seen = newSeen(); + // A new filter object each render, as an inline literal is. + const tree = mount(); + act(() => tree.update()); + expect(seen.renders).toBe(2); + expect(seen.results[1]).toBe(seen.results[0]); + + // A change to another row does not re-render this one. + act(() => native.emit.state(row({id: 'b', state: 'running'}))); + expect(seen.renders).toBe(2); + + act(() => native.emit.state(row({id: 'a', state: 'running'}))); + expect(seen.renders).toBe(3); + expect(last(seen)).toEqual([row({id: 'a', state: 'running'})]); +}); + +it('drops a completed row once its outcome is acknowledged', async () => { + const ping = client.define({ + key: 'ping', + request: () => ({url: 'https://api.test/ping'}), + }); + const seen = newSeen(); + mount(); + let id = ''; + await act(async () => { + ({id} = await ping.mutate()); + }); + expect(last(seen)).toEqual([expect.objectContaining({id, state: 'queued'})]); + await act(async () => { + await native.settle(id, {kind: 'completed'}); + }); + expect(last(seen)).toEqual([]); +}); + +it('drops a settled row when cancel() resolves, and hides cancelled rows', async () => { + native.seedRows([ + row({id: 'e', state: 'error'}), + row({id: 'c', state: 'cancelled'}), + ]); + const visible = newSeen(); + const all = newSeen(); + mount( + <> + + + , + ); + expect(last(visible).map(r => r.id)).toEqual(['e']); + expect(last(all).map(r => r.id)).toEqual(['e', 'c']); + await act(async () => { + await client.cancel('e'); + }); + expect(last(visible)).toEqual([]); + expect(last(all).map(r => r.id)).toEqual(['c']); +}); + +it('re-checks native when a hook mounts again', () => { + native.seedRows([row({id: 'a'})]); + const first = newSeen(); + const tree = mount(); + act(() => tree.unmount()); + // A change with no event while nothing is mounted. + native.seedRows([row({id: 'b'})]); + const second = newSeen(); + mount(); + expect(last(second).map(r => r.id)).toEqual(['a', 'b']); +}); + +it('types the vars of a definition', () => { + type Vars = {captureId: string}; + const upload = client.define({ + key: 'capture', + request: (_vars: Vars) => ({url: 'https://api.test/c'}), + }); + native.seedRows([ + row({id: 'x', key: 'capture', vars: {captureId: 'c1'}}), + row({id: 'y', key: 'other'}), + ]); + const ids: string[][] = []; + const Captures = () => { + const rows = upload.useRequests(); + ids.push(rows.map(r => r.vars.captureId)); + return null; + }; + mount(); + expect(ids[ids.length - 1]).toEqual(['c1']); +}); diff --git a/example/RNBGUExample/jest.config.js b/example/RNBGUExample/jest.config.js index 8eb675e9..3be97f22 100644 --- a/example/RNBGUExample/jest.config.js +++ b/example/RNBGUExample/jest.config.js @@ -1,3 +1,9 @@ module.exports = { preset: 'react-native', + // The library source in ../../src imports react. Without this it resolves + // the repo root's copy, and the test renderer's copy here is another React, + // so hooks fail. + moduleNameMapper: { + '^react$': '/node_modules/react', + }, }; diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index 9606758d..549f9e4d 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -107,17 +107,21 @@ describe('client shape', () => { expect(Object.keys(client).sort()).toEqual( [ 'addListener', + 'addRequestsListener', 'android', 'cancel', 'chunkPlan', 'configure', 'define', + 'getCachedRequests', 'getRequests', 'pause', 'resume', 'setWifiOnly', 'takeDiagnostics', 'updateHeaders', + 'useRequests', + 'useRequestsWithProgress', ].sort(), ); expect(Object.keys(client.android)).toEqual(['addNotificationListener']); diff --git a/src/__tests__/delivery.test.ts b/src/__tests__/delivery.test.ts index ef82730f..4995c12c 100644 --- a/src/__tests__/delivery.test.ts +++ b/src/__tests__/delivery.test.ts @@ -56,15 +56,17 @@ const setup = ( }; const stateEvents: StateEvent[] = []; const warn = jest.fn(); + const onAcked = jest.fn(); const delivery = createDelivery({ native, lookup: (key) => definitions[key], emitState: (e) => stateEvents.push(e), + onAcked, warn, handlerWarningMs, }); const emit = (event: SettledEvent) => handlers.forEach((h) => h(event)); - return { ...delivery, native, emit, stateEvents, warn, handlers }; + return { ...delivery, native, emit, stateEvents, warn, handlers, onAcked }; }; describe('replay', () => { @@ -802,3 +804,93 @@ describe('slow handler warning', () => { expect(warn).not.toHaveBeenCalled(); }); }); + +describe('onAcked', () => { + it('reports each outcome after its ack resolves', async () => { + const ackGate = deferred(); + const { start, emit, native, onAcked } = setup({ + k: { key: 'k', request: jest.fn() }, + }); + native.ackEvents.mockReturnValueOnce(ackGate.promise); + start(); + await flush(); + emit(completed()); + await flush(); + expect(native.ackEvents).toHaveBeenCalledWith(['e1']); + expect(onAcked).not.toHaveBeenCalled(); + ackGate.resolve(); + await flush(); + expect(onAcked).toHaveBeenCalledTimes(1); + expect(onAcked).toHaveBeenCalledWith( + expect.objectContaining({ eventId: 'e1', id: 'u1', kind: 'completed' }), + ); + }); + + it('reports a cancelled outcome, which has no handler', async () => { + const { start, emit, onAcked } = setup({}); + start(); + await flush(); + emit({ + ...completed(), + kind: 'cancelled', + cancelReason: 'user', + state: 'cancelled', + } as SettledEvent); + await flush(); + expect(onAcked).toHaveBeenCalledWith( + expect.objectContaining({ id: 'u1', kind: 'cancelled' }), + ); + }); + + it('does not report an outcome whose ack failed', async () => { + const { start, emit, native, onAcked } = setup({ + k: { key: 'k', request: jest.fn() }, + }); + native.ackEvents.mockRejectedValueOnce(new Error('E_STORAGE')); + start(); + await flush(); + emit(completed()); + await flush(); + expect(onAcked).not.toHaveBeenCalled(); + }); + + it('does not report an outcome that was not acked', async () => { + const { start, emit, onAcked } = setup({ + k: { + key: 'k', + request: jest.fn(), + onSuccess: async () => { + throw new Error('no'); + }, + }, + }); + start(); + await flush(); + emit(completed()); + emit(completed({ eventId: 'e2', key: 'unknown' })); + await flush(); + expect(onAcked).not.toHaveBeenCalled(); + }); + + it('warns when onAcked throws, and keeps delivering', async () => { + const onSuccess = jest.fn(); + const { start, emit, native, onAcked, warn } = setup({ + k: { key: 'k', request: jest.fn(), onSuccess }, + }); + onAcked.mockImplementationOnce(() => { + throw new Error('boom'); + }); + start(); + await flush(); + emit(completed()); + emit(completed({ eventId: 'e2' })); + await flush(); + expect(native.ackEvents).toHaveBeenCalledTimes(2); + expect(onSuccess).toHaveBeenCalledTimes(2); + expect(onAcked).toHaveBeenCalledTimes(2); + expect(warn).toHaveBeenCalledWith( + expect.stringMatching(/after the ack of e1/), + expect.any(Error), + ); + }); +}); diff --git a/src/__tests__/requests.test.ts b/src/__tests__/requests.test.ts new file mode 100644 index 00000000..5aeed202 --- /dev/null +++ b/src/__tests__/requests.test.ts @@ -0,0 +1,471 @@ +// The row cache behind useRequests(), tested through the client with the +// fake native. React is not rendered here (the root has no renderer); the +// example app's Jest renders the hooks with react-test-renderer. +jest.mock('react-native', () => { + const { createFakeNative: create } = jest.requireActual('../testing'); + const fake = create(); + return { + TurboModuleRegistry: { getEnforcing: () => fake, get: () => fake }, + }; +}); + +import { createUploadClient } from '../index'; +import { createRequestCache, createSelector } from '../requests'; +import { createFakeNative, type FakeNative } from '../testing'; +import type { RequestRow } from '../types'; + +const row = (overrides: Partial = {}): RequestRow => ({ + id: 'a', + key: 'k', + vars: { n: 1 }, + state: 'queued', + bytesSent: 0, + totalBytes: 100, + attempts: 0, + updatedAt: 1, + ...overrides, +}); + +const flush = async (): Promise => { + for (let i = 0; i < 3; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } +}; + +const setup = (native: FakeNative = createFakeNative()) => { + const warnings: string[] = []; + const client = createUploadClient({ native }); + const ping = client.define({ + key: 'ping', + request: (_vars: { n: number }) => ({ url: 'https://api.test/ping' }), + }); + client.configure({ onWarning: (message) => warnings.push(message) }); + const getRequests = jest.spyOn(native, 'getRequests'); + return { native, client, ping, warnings, getRequests }; +}; + +/** Subscribes the way a mounted hook does, and counts the notifications. */ +const listen = (client: ReturnType['client']) => { + const calls: Array = []; + const subscription = client.addRequestsListener((rows) => calls.push(rows)); + return { calls, subscription }; +}; + +describe('first subscribe', () => { + it('reads native once, however many listeners and reads follow', () => { + const { native, client, getRequests } = setup(); + native.seedRows([row({ id: 'a' }), row({ id: 'b' })]); + listen(client); + listen(client); + client.getCachedRequests(); + client.getCachedRequests({ key: 'k' }); + expect(getRequests).toHaveBeenCalledTimes(1); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); + }); + + it('does not read native again at subscribe right after a render read', () => { + const { native, client, getRequests } = setup(); + native.seedRows([row()]); + // A render reads before its effect subscribes, in the same task. + client.getCachedRequests(); + listen(client); + expect(getRequests).toHaveBeenCalledTimes(1); + }); +}); + +describe('state events', () => { + it('patch one row and keep the identity of the others', async () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'a' }), row({ id: 'b' })]); + const { calls } = listen(client); + const before = client.getCachedRequests(); + native.emit.state(row({ id: 'a', state: 'running', updatedAt: 2 })); + const after = client.getCachedRequests(); + expect(after).not.toBe(before); + expect(after[0]).toEqual(row({ id: 'a', state: 'running', updatedAt: 2 })); + expect(after[1]).toBe(before[1]); + expect(calls).toHaveLength(1); + }); + + it('add a row that mutate() created', async () => { + const { client, ping } = setup(); + listen(client); + const { id } = await ping.mutate({ n: 1 }); + expect(client.getCachedRequests()).toEqual([ + expect.objectContaining({ id, key: 'ping', state: 'queued' }), + ]); + }); + + it('do not notify for a row that did not change', () => { + const { native, client } = setup(); + native.seedRows([row()]); + const { calls } = listen(client); + const before = client.getCachedRequests(); + native.emit.state(row()); + expect(client.getCachedRequests()).toBe(before); + expect(calls).toHaveLength(0); + }); + + it('drop the unhandled-key reason from the row', () => { + const { native, client } = setup(); + native.seedRows([row()]); + listen(client); + native.emit.state({ ...row({ state: 'completed' }), reason: 'unhandled-key' }); + expect(client.getCachedRequests()[0]).toEqual(row({ state: 'completed' })); + }); +}); + +describe('progress', () => { + // The progress view is only reachable through the hook, so these tests + // build the cache over a client's events directly. + const progressSetup = () => { + const native = createFakeNative(); + const client = createUploadClient({ native }); + native.seedRows([row({ id: 'a' }), row({ id: 'b' })]); + const cache = createRequestCache({ + readNative: () => native.getRequests() as RequestRow[], + addListener: client.addListener, + warn: jest.fn(), + }); + return { native, cache }; + }; + + it('notifies only the progress view, and overlays the bytes', () => { + const { native, cache } = progressSetup(); + const rowCalls = jest.fn(); + const progressCalls = jest.fn(); + cache.subscribe(rowCalls); + cache.subscribeWithProgress(progressCalls); + const rows = cache.rows(); + const before = cache.rowsWithProgress(); + native.emit.progress({ id: 'a', bytesSent: 40, totalBytes: 100 }); + const after = cache.rowsWithProgress(); + expect(progressCalls).toHaveBeenCalledTimes(1); + expect(rowCalls).not.toHaveBeenCalled(); + expect(cache.rows()).toBe(rows); + expect(after).not.toBe(before); + expect(after[0]).toEqual(row({ id: 'a', bytesSent: 40 })); + expect(after[1]).toBe(before[1]); + // Reading again without a new event returns the same array. + expect(cache.rowsWithProgress()).toBe(after); + }); + + it('notifies both views on a state event', () => { + const { native, cache } = progressSetup(); + const rowCalls = jest.fn(); + const progressCalls = jest.fn(); + cache.subscribe(rowCalls); + cache.subscribeWithProgress(progressCalls); + native.emit.state(row({ id: 'a', state: 'running', updatedAt: 2 })); + expect(rowCalls).toHaveBeenCalledTimes(1); + expect(progressCalls).toHaveBeenCalledTimes(1); + }); + + it('gives way to the bytes of the next state event', () => { + const { native, cache } = progressSetup(); + cache.subscribeWithProgress(() => {}); + native.emit.progress({ id: 'a', bytesSent: 40, totalBytes: 100 }); + native.emit.state(row({ state: 'running', bytesSent: 10, updatedAt: 2 })); + expect(cache.rowsWithProgress()[0]!.bytesSent).toBe(10); + }); + + it('ignores progress for a row it does not hold', () => { + const { native, cache } = progressSetup(); + const progressCalls = jest.fn(); + cache.subscribeWithProgress(progressCalls); + native.emit.progress({ id: 'zzz', bytesSent: 1, totalBytes: 2 }); + expect(progressCalls).not.toHaveBeenCalled(); + }); +}); + +describe('rows native forgets with no event', () => { + it('drops a completed row when its ack resolves', async () => { + const { native, client, ping } = setup(); + const { calls } = listen(client); + const { id } = await ping.mutate({ n: 1 }); + // settle() resolves after the client handled the ack, so the cache is + // already up to date here. + await native.settle(id, { kind: 'completed' }); + expect(native.getRequests()).toEqual([]); + expect(client.getCachedRequests({ includeCancelled: true })).toEqual([]); + expect(calls[calls.length - 1]).toEqual([]); + }); + + it('keeps a completed row whose ack failed', async () => { + const native = createFakeNative({ ackTimeoutMs: 50 }); + const { client, ping, warnings } = setup(native); + listen(client); + const { id } = await ping.mutate({ n: 1 }); + native.failNext('ackEvents', 'E_STORAGE'); + await expect(native.settle(id, { kind: 'completed' })).rejects.toThrow( + /not acknowledged/, + ); + expect(client.getCachedRequests()).toEqual([ + expect.objectContaining({ id, state: 'completed' }), + ]); + expect(warnings).toEqual([expect.stringMatching(/ackEvents failed/)]); + }); + + it('keeps a completed row whose key has no definition', async () => { + const native = createFakeNative({ ackTimeoutMs: 50 }); + const { client } = setup(native); + native.seedRows([row({ id: 'x', key: 'gone', state: 'running' })]); + listen(client); + await expect(native.settle('x', { kind: 'completed' })).rejects.toThrow( + /not acknowledged/, + ); + expect(client.getCachedRequests()).toEqual([ + expect.objectContaining({ id: 'x', state: 'completed' }), + ]); + }); + + it('drops a cancelled live row when its ack resolves', async () => { + const { native, client, ping } = setup(); + listen(client); + const { id } = await ping.mutate({ n: 1 }); + await client.cancel(id); + await flush(); + expect(native.getRequests()).toEqual([]); + expect(client.getCachedRequests({ includeCancelled: true })).toEqual([]); + }); + + it('drops a settled row when cancel() resolves', async () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'e', state: 'error' })]); + const { calls } = listen(client); + const states = jest.fn(); + client.addListener('state', states); + await client.cancel('e'); + expect(states).not.toHaveBeenCalled(); + expect(client.getCachedRequests()).toEqual([]); + expect(calls).toEqual([[]]); + }); + + it('keeps the row when cancel() rejects', async () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'e', state: 'error' })]); + listen(client); + native.failNext('cancel', 'E_STORAGE'); + await expect(client.cancel('e')).rejects.toThrow('E_STORAGE'); + expect(client.getCachedRequests()).toHaveLength(1); + }); + + it('keeps an error row after its ack, until cancel()', async () => { + const { native, client, ping } = setup(); + listen(client); + const { id } = await ping.mutate({ n: 1 }); + await native.settle(id, { kind: 'error', error: { errorKind: 'http' } }); + expect(client.getCachedRequests()).toEqual([ + expect.objectContaining({ id, state: 'error' }), + ]); + await client.cancel(id); + expect(client.getCachedRequests()).toEqual([]); + }); + + it('replaces an error row on a same-id mutate()', async () => { + const { native, client, ping } = setup(); + listen(client); + const { id } = await ping.mutate({ n: 1 }); + await native.settle(id, { kind: 'error', error: { errorKind: 'http' } }); + await ping.mutate({ n: 2 }, { id }); + expect(client.getCachedRequests()).toEqual([ + expect.objectContaining({ id, state: 'queued', vars: { n: 2 } }), + ]); + }); +}); + +describe('the ack of an outcome whose row moved on', () => { + const stub = (nativeRows: RequestRow[]) => { + const listeners: Record void> = {}; + const readNative = jest.fn(() => nativeRows.map((r) => ({ ...r }))); + const cache = createRequestCache({ + readNative, + addListener: ((event: string, listener: (e: never) => void) => { + listeners[event] = listener; + return { remove: () => {} }; + }) as never, + warn: jest.fn(), + }); + return { cache, readNative, listeners }; + }; + + it('asks native when the row is not in the outcome state, and keeps it', async () => { + // A cancelled entry reopened by a same-id mutate(): the old outcome's + // ack does not forget it. + const reopened = row({ id: 'a', state: 'queued', updatedAt: 5 }); + const { cache, readNative } = stub([reopened]); + cache.subscribe(() => {}); + await Promise.resolve(); + readNative.mockClear(); + cache.acked({ id: 'a', kind: 'cancelled' }); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows()).toEqual([reopened]); + }); + + it('drops the row when native no longer has it', async () => { + const nativeRows = [row({ id: 'a', state: 'running' })]; + const { cache } = stub(nativeRows); + cache.subscribe(() => {}); + nativeRows.length = 0; + cache.acked({ id: 'a', kind: 'completed' }); + expect(cache.rows()).toEqual([]); + }); + + it('does nothing for an error outcome', () => { + const { cache, readNative } = stub([row({ id: 'a', state: 'error' })]); + cache.subscribe(() => {}); + readNative.mockClear(); + cache.acked({ id: 'a', kind: 'error' }); + expect(readNative).not.toHaveBeenCalled(); + expect(cache.rows()).toHaveLength(1); + }); +}); + +describe('unsubscribe and subscribe again', () => { + it('keeps the rows, then checks them against native', async () => { + const { native, client, getRequests } = setup(); + native.seedRows([row({ id: 'a' })]); + const first = listen(client); + const kept = client.getCachedRequests(); + first.subscription.remove(); + await flush(); + // A change with no event while nobody listens. + native.seedRows([row({ id: 'b' })]); + getRequests.mockClear(); + const second = listen(client); + expect(getRequests).toHaveBeenCalledTimes(1); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); + // Row 'a' did not change, so it keeps its identity. + expect(client.getCachedRequests()[0]).toBe(kept[0]); + expect(second.calls).toHaveLength(1); + }); + + it('keeps the same array when native has not changed', async () => { + const { native, client } = setup(); + native.seedRows([row()]); + const first = listen(client); + const kept = client.getCachedRequests(); + first.subscription.remove(); + await flush(); + const second = listen(client); + expect(client.getCachedRequests()).toBe(kept); + expect(second.calls).toHaveLength(0); + }); + + it('stops listening to native when the last listener leaves', () => { + const { native, client } = setup(); + const before = native.listenerCount('state'); + const one = listen(client); + const two = listen(client); + expect(native.listenerCount('state')).toBe(before + 1); + one.subscription.remove(); + expect(native.listenerCount('state')).toBe(before + 1); + two.subscription.remove(); + expect(native.listenerCount('state')).toBe(before); + }); + + it('reads native on every getCachedRequests() while nobody listens', async () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'a' })]); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a']); + await flush(); + native.seedRows([row({ id: 'b' })]); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); + }); + + it('warns and keeps the rows when the native read throws', async () => { + const { native, client, warnings } = setup(); + native.seedRows([row()]); + const first = listen(client); + first.subscription.remove(); + await flush(); + native.failNext('getRequests', 'E_BOOM'); + listen(client); + expect(client.getCachedRequests()).toHaveLength(1); + expect(warnings).toEqual([expect.stringMatching(/getRequests/)]); + }); +}); + +describe('two clients', () => { + it('keep separate caches', async () => { + const one = setup(); + const two = setup(); + one.native.seedRows([row({ id: 'a' })]); + two.native.seedRows([row({ id: 'b' })]); + listen(one.client); + listen(two.client); + one.native.emit.state(row({ id: 'c' })); + expect(one.client.getCachedRequests().map((r) => r.id)).toEqual([ + 'a', + 'c', + ]); + expect(two.client.getCachedRequests().map((r) => r.id)).toEqual(['b']); + }); +}); + +describe('filters', () => { + it('leave out cancelled rows unless includeCancelled is set', () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'a' }), row({ id: 'c', state: 'cancelled' })]); + listen(client); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a']); + expect( + client.getCachedRequests({ includeCancelled: true }).map((r) => r.id), + ).toEqual(['a', 'c']); + }); + + it('match key and id', () => { + const { native, client } = setup(); + native.seedRows([ + row({ id: 'a', key: 'k1' }), + row({ id: 'b', key: 'k2' }), + row({ id: 'c', key: 'k2' }), + ]); + listen(client); + expect(client.getCachedRequests({ key: 'k2' }).map((r) => r.id)).toEqual([ + 'b', + 'c', + ]); + expect( + client.getCachedRequests({ key: 'k2', id: 'c' }).map((r) => r.id), + ).toEqual(['c']); + }); + + it('return the same array while nothing changed', () => { + const { native, client } = setup(); + native.seedRows([row()]); + listen(client); + expect(client.getCachedRequests()).toBe(client.getCachedRequests()); + expect(client.getCachedRequests({ includeCancelled: true })).toBe( + client.getCachedRequests({ includeCancelled: true }), + ); + }); +}); + +describe('createSelector', () => { + it('returns the same array for the same rows', () => { + const rows = [row({ id: 'a' }), row({ id: 'b' })]; + const select = createSelector(() => rows, { id: 'a' }); + expect(select()).toBe(select()); + }); + + it('keeps its array when only rows it leaves out change', () => { + let rows = [row({ id: 'a' }), row({ id: 'b' })]; + const select = createSelector(() => rows, { id: 'a' }); + const first = select(); + rows = [rows[0]!, row({ id: 'b', state: 'running' })]; + expect(select()).toBe(first); + rows = [row({ id: 'a', state: 'running' }), rows[1]!]; + expect(select()).not.toBe(first); + expect(select()).toEqual([row({ id: 'a', state: 'running' })]); + }); + + it('keeps its array when a hidden cancelled row changes', () => { + let rows = [row({ id: 'a' }), row({ id: 'c', state: 'cancelled' })]; + const select = createSelector(() => rows, {}); + const first = select(); + expect(first.map((r) => r.id)).toEqual(['a']); + rows = [rows[0]!]; + expect(select()).toBe(first); + }); +}); diff --git a/src/__typetests__/requests.ts b/src/__typetests__/requests.ts new file mode 100644 index 00000000..5f5b6658 --- /dev/null +++ b/src/__typetests__/requests.ts @@ -0,0 +1,82 @@ +// Type-level tests for the row hooks. tsc compiles this file with the library +// (yarn typecheck); nothing here runs. +import type { + DefinedRequestRow, + RequestRow, + RequestsFilter, + UploadClient, +} from '../types'; + +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B + ? 1 + : 2 + ? true + : false; +const expectType = (_actual: Expected): void => {}; +const assertEqual = ( + _proof: Equal extends true ? true : never, +) => {}; + +declare const client: UploadClient; + +// The client hooks take the getRequests() filter plus includeCancelled. +const rows = client.useRequests(); +assertEqual(true); +client.useRequests({ key: 'k', id: 'i', includeCancelled: true }); +client.useRequestsWithProgress({ key: 'k' }); +// @ts-expect-error unknown filter field +client.useRequests({ keys: ['k'] }); +// @ts-expect-error includeCancelled is a boolean +client.useRequests({ includeCancelled: 'yes' }); + +// The array is shared with every other reader, so it is read-only. +// @ts-expect-error push does not exist on a readonly array +rows.push(rows[0]!); + +// Outside React. +expectType(client.getCachedRequests({ key: 'k' })); +const subscription = client.addRequestsListener((current) => { + expectType(current); +}); +subscription.remove(); +const filter: RequestsFilter = { includeCancelled: false }; +client.getCachedRequests(filter); + +// A definition's hooks type the vars, and take no key. +type CaptureVars = { captureId: string; index: number }; +const capture = client.define({ + key: 'capture.upload', + request: ({ captureId }: CaptureVars) => ({ + url: `https://api/captures/${captureId}`, + }), +}); +const captureRows = capture.useRequests(); +assertEqual[]>( + true, +); +const first = captureRows[0]; +if (first) { + assertEqual(true); + assertEqual(true); +} +capture.useRequests({ id: 'x', includeCancelled: true }); +capture.useRequestsWithProgress({ id: 'x' }); +// @ts-expect-error the key is the definition's +capture.useRequests({ key: 'other' }); + +// A definition with a response parser types its rows the same way. +const parsed = client.define({ + key: 'note.add', + request: (_vars: { noteId: string }) => ({ url: 'https://api/notes' }), + response: (raw) => raw as { id: string }, +}); +expectType[]>( + parsed.useRequestsWithProgress(), +); + +// A definition without vars has rows whose vars are null. +const ping = client.define({ + key: 'ping', + request: () => ({ url: 'https://api/ping' }), +}); +expectType[]>(ping.useRequests()); diff --git a/src/delivery.ts b/src/delivery.ts index 685145d1..8c5e6c89 100644 --- a/src/delivery.ts +++ b/src/delivery.ts @@ -49,6 +49,12 @@ type DeliveryDeps = { lookup: (key: string) => AnyDefinition | undefined; /** Feeds the client's `state` listeners. */ emitState: (event: StateEvent) => void; + /** + * Called after the ack of an outcome resolved, so the client's row cache + * can drop a row that native forgot with no event. Not called when the ack + * failed. A throw is a warning and changes nothing in delivery. + */ + onAcked?: (event: SettledEvent) => void; warn?: (message: string, detail?: unknown) => void; handlerWarningMs?: number; }; @@ -76,6 +82,7 @@ export const createDelivery = ({ native, lookup, emitState, + onAcked, warn = console.warn, handlerWarningMs = HANDLER_WARNING_MS, }: DeliveryDeps): Delivery => { @@ -105,11 +112,22 @@ export const createDelivery = ({ }); }; - const ack = async (eventId: string): Promise => { + const ack = async (event: SettledEvent): Promise => { + const { eventId } = event; try { await native.ackEvents([eventId]); } catch (e) { warn(`delivery: ackEvents failed for ${eventId}`, e); + return; + } + // Display only: the ack has already landed, and a throw here is caught. + try { + onAcked?.(event); + } catch (e) { + warn( + `delivery: updating the request rows after the ack of ${eventId} threw`, + e, + ); } }; @@ -247,7 +265,7 @@ export const createDelivery = ({ // A cancelled outcome has no handler, so it acks whether or not the key // is still defined. if (event.kind === 'cancelled') { - await ack(event.eventId); + await ack(event); return; } const definition = lookup(event.key); @@ -259,7 +277,7 @@ export const createDelivery = ({ return; } if (await runHandler(event, definition)) { - await ack(event.eventId); + await ack(event); } }; diff --git a/src/index.ts b/src/index.ts index e61406ef..3e541c3c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,6 +8,12 @@ import NativeRNFileUploader from './NativeRNFileUploader'; import type { Spec } from './NativeRNFileUploader'; import { chunkPlan } from './chunkPlan'; import { createDelivery } from './delivery'; +import { + createRequestCache, + createSelector, + matches, + useRequestRows, +} from './requests'; import { createRegistry, DEFAULT_ENQUEUE_TIMEOUT_MS, @@ -19,11 +25,17 @@ import { import type { AddListener, ConfigureOptions, + Define, + Defined, + DefinedRequestsFilter, + Definition, Diagnostic, PauseScope, RequestRow, + RequestsFilter, StateEvent, UploadClient, + Vars, } from './types'; export * from './types'; @@ -129,9 +141,11 @@ export const createUploadClient = ({ native, lookup: (key) => definitions.get(key), emitState, + // Native forgets a completed or cancelled row at its ack, with no event. + onAcked: (event) => requests.acked(event), warn, }); - const { define } = createRegistry({ + const registry = createRegistry({ native, definitions, getSettings: () => settings, @@ -213,7 +227,16 @@ export const createUploadClient = ({ * it after the ack. On a settled entry: forgets it now: row, bytes, and its * unacknowledged outcomes. */ - const cancel = (id: string): Promise => native.cancel(id); + const cancel = (id: string): Promise => + native.cancel(id).then(() => { + // Native forgets a settled entry with no event. Display only: the + // cancel has landed, so a throw here must not reject it. + try { + requests.cancelled(id); + } catch (e) { + warn(`cancel: updating the request rows after cancel(${id}) threw`, e); + } + }); /** * Persisted natively. Applies to queued and future entries whose @@ -301,6 +324,65 @@ export const createUploadClient = ({ } }) as AddListener; + const requests = createRequestCache({ + readNative: () => getRequests(), + addListener, + warn, + }); + const visibleRows = createSelector(requests.read, {}); + + const getCachedRequests = (filter?: RequestsFilter): readonly RequestRow[] => { + if (filter?.key === undefined && filter?.id === undefined) { + return filter?.includeCancelled === true + ? requests.read() + : visibleRows(); + } + return requests.read().filter((row) => matches(row, filter)); + }; + + const addRequestsListener = ( + listener: (rows: readonly RequestRow[]) => void, + ): { remove: () => void } => { + const remove = requests.subscribe(() => { + try { + listener(visibleRows()); + } catch (e) { + warn('addRequestsListener: a listener threw', e); + } + }); + return { remove }; + }; + + const useRequests = (filter?: RequestsFilter): readonly RequestRow[] => + useRequestRows(requests.subscribe, requests.rows, filter); + + const useRequestsWithProgress = ( + filter?: RequestsFilter, + ): readonly RequestRow[] => + useRequestRows( + requests.subscribeWithProgress, + requests.rowsWithProgress, + filter, + ); + + // define() from the registry, plus the definition's own row hooks. The + // rows of a key are the rows its mutate() enqueued, so their vars are + // typed as the definition's vars. + const define = (( + definition: Definition, + ): Defined => { + const defined = registry.define(definition); + const { key } = defined; + type Rows = ReturnType['useRequests']>; + return { + ...defined, + useRequests: (filter?: DefinedRequestsFilter) => + useRequests({ ...filter, key }) as Rows, + useRequestsWithProgress: (filter?: DefinedRequestsFilter) => + useRequestsWithProgress({ ...filter, key }) as Rows, + }; + }) as Define; + const android = { /** * Fires when the Android upload notification is pressed. It never fires @@ -319,6 +401,10 @@ export const createUploadClient = ({ setWifiOnly, updateHeaders, getRequests, + useRequests, + useRequestsWithProgress, + addRequestsListener, + getCachedRequests, takeDiagnostics, addListener, chunkPlan, diff --git a/src/registry.ts b/src/registry.ts index 2f9584b6..4a598cdb 100644 --- a/src/registry.ts +++ b/src/registry.ts @@ -1,6 +1,5 @@ import type { Spec } from './NativeRNFileUploader'; import type { - Define, Defined, Definition, FormPart, @@ -112,8 +111,19 @@ type RegistryDeps = { now?: () => number; }; +/** + * What the registry's define() builds: the key and mutate(). The client adds + * the row hooks to make a full `Defined`. + */ +export type RegistryDefined = Pick< + Defined, + 'key' | 'mutate' +>; + export type Registry = { - define: Define; + define: ( + definition: Definition, + ) => RegistryDefined; }; declare const __DEV__: boolean | undefined; @@ -513,10 +523,11 @@ export const createRegistry = ({ now = Date.now, }: RegistryDeps): Registry => { // The overloads on Define keep the with-parser and without-parser shapes - // apart for callers. One implementation serves both. - const define = (( + // apart for callers. The client applies them; one implementation serves + // both. + const define = ( definition: Definition, - ): Defined => { + ): RegistryDefined => { const { key } = definition; if (typeof key !== 'string' || key.length === 0) { throw new Error('define: key must be a non-empty string'); @@ -587,8 +598,8 @@ export const createRegistry = ({ return { id: entry.id }; }; - return { key, mutate } as Defined; - }) as Define; + return { key, mutate } as RegistryDefined; + }; return { define }; }; diff --git a/src/requests.ts b/src/requests.ts new file mode 100644 index 00000000..c89f1794 --- /dev/null +++ b/src/requests.ts @@ -0,0 +1,408 @@ +/** + * The client's in-memory copy of the native queue rows, for display. It is + * read from native when the first listener arrives and patched by each + * `state` event, which carries the full row. A second view overlays the bytes + * of `progress` events. The library closes the gaps where native changes a + * row with no event, because its own JS makes those calls: the ack of a + * completed or cancelled outcome, and cancel() on a settled entry. + * + * Nothing here sends, acknowledges, or cancels anything. A failure here can + * only make a row show late or stay longer. + */ +import { useMemo, useSyncExternalStore } from 'react'; +import type { + AddListener, + Json, + ProgressEvent, + RequestRow, + RequestsFilter, + RequestState, + StateEvent, +} from './types'; + +type Listener = () => void; + +const LIVE_STATES: readonly RequestState[] = [ + 'queued', + 'running', + 'awaiting-auth', + 'paused', +]; + +/** Deep equality for JSON values, as `vars` and rows hold them. */ +export const jsonEqual = (a: unknown, b: unknown): boolean => { + if (a === b) { + return true; + } + if ( + typeof a !== 'object' || + typeof b !== 'object' || + a === null || + b === null + ) { + return false; + } + if (Array.isArray(a) !== Array.isArray(b)) { + return false; + } + if (Array.isArray(a)) { + const other = b as unknown[]; + return ( + a.length === other.length && a.every((v, i) => jsonEqual(v, other[i])) + ); + } + const left = a as Record; + const right = b as Record; + const leftKeys = Object.keys(left).filter((k) => left[k] !== undefined); + const rightKeys = Object.keys(right).filter((k) => right[k] !== undefined); + return ( + leftKeys.length === rightKeys.length && + leftKeys.every((k) => jsonEqual(left[k], right[k])) + ); +}; + +const sameItems = (a: readonly T[], b: readonly T[]): boolean => + a.length === b.length && a.every((item, i) => item === b[i]); + +export const matches = (row: RequestRow, filter: RequestsFilter): boolean => + (filter.includeCancelled === true || row.state !== 'cancelled') && + (filter.key === undefined || row.key === filter.key) && + (filter.id === undefined || row.id === filter.id); + +/** + * A read of `source()` narrowed by `filter`. It returns the same array while + * the rows it keeps are the same objects, so a row it leaves out can change + * without a new result. `useSyncExternalStore` then re-renders only when the + * filtered rows change. + */ +export const createSelector = ( + source: () => readonly RequestRow[], + filter: RequestsFilter, +): (() => readonly RequestRow[]) => { + let lastSource: readonly RequestRow[] | undefined; + let lastResult: readonly RequestRow[] | undefined; + return () => { + const rows = source(); + if (rows === lastSource && lastResult) { + return lastResult; + } + const next = rows.filter((row) => matches(row, filter)); + lastSource = rows; + if (!lastResult || !sameItems(next, lastResult)) { + lastResult = next; + } + return lastResult; + }; +}; + +type RequestCacheDeps = { + /** The client's getRequests() without a filter. May throw. */ + readNative: () => RequestRow[]; + /** The client's addListener(), so unhandled-key rows arrive too. */ + addListener: AddListener; + warn: (message: string, detail?: unknown) => void; +}; + +export type RequestCache = { + /** Every cached row, cancelled ones included. Stable until a row changes. */ + rows: () => readonly RequestRow[]; + /** `rows()` with the bytes of the progress events since each row's last state. */ + rowsWithProgress: () => readonly RequestRow[]; + /** Called when a row appears, changes, or goes. Returns the unsubscribe. */ + subscribe: (listener: Listener) => () => void; + /** Also called on every progress event for a cached row. */ + subscribeWithProgress: (listener: Listener) => () => void; + /** + * For reads outside React: with no listener, reads native first, so the + * rows are never out of date. + */ + read: () => readonly RequestRow[]; + /** The ack of an outcome of `id` resolved. */ + acked: (outcome: { id: string; kind: string }) => void; + /** cancel(id) resolved. */ + cancelled: (id: string) => void; +}; + +export const createRequestCache = ({ + readNative, + addListener, + warn, +}: RequestCacheDeps): RequestCache => { + // Undefined until the first read. + let rows: readonly RequestRow[] | undefined; + let withProgress: readonly RequestRow[] | undefined; + // The bytes of the last progress event of each row, and the overlaid row + // built from them, so an unchanged row keeps its identity. + const progress = new Map(); + const overlaid = new Map< + string, + { base: RequestRow; event: ProgressEvent; row: RequestRow } + >(); + const rowListeners = new Set(); + const progressListeners = new Set(); + let subscriptions: Array<{ remove: () => void }> = []; + // True from a native read until the next microtask. Native events arrive + // as new tasks, so none can be missed between a read and a subscribe in + // the same task, and the subscribe does not need to read again. + let fresh = false; + + const notify = (listeners: Set): void => { + listeners.forEach((listener) => listener()); + }; + + /** True when the row had progress to forget. */ + const forgetProgress = (id: string): boolean => { + overlaid.delete(id); + return progress.delete(id); + }; + + const commit = (next: readonly RequestRow[]): void => { + if (next === rows) { + return; + } + rows = next; + withProgress = undefined; + notify(rowListeners); + notify(progressListeners); + }; + + /** Native's rows, or undefined when the read throws. */ + const readSafely = (): RequestRow[] | undefined => { + try { + const current = readNative(); + fresh = true; + void Promise.resolve().then(() => { + fresh = false; + }); + return current; + } catch (e) { + warn( + 'requests: getRequests failed, so the cached rows were not checked against native', + e, + ); + return undefined; + } + }; + + /** + * Native's rows, with the cached object kept for each row that did not + * change, and the cached array kept when no row changed. + */ + const reconcile = ( + cachedRows: readonly RequestRow[], + current: RequestRow[], + ): readonly RequestRow[] => { + const cached = new Map(cachedRows.map((row) => [row.id, row])); + const next = current.map((row) => { + const old = cached.get(row.id); + return old && jsonEqual(old, row) ? old : row; + }); + return sameItems(next, cachedRows) ? cachedRows : next; + }; + + /** Checks every row against native. Keeps the rows when the read fails. */ + const refresh = (): void => { + const current = readSafely(); + if (!rows) { + // The first read: nobody has seen rows yet, so nobody is notified. + rows = current ?? []; + return; + } + if (current) { + commit(reconcile(rows, current)); + } + }; + + const ensureRows = (): readonly RequestRow[] => { + if (!rows) { + refresh(); + } + return rows ?? []; + }; + + const remove = (id: string): void => { + forgetProgress(id); + if (rows?.some((row) => row.id === id)) { + commit(rows.filter((row) => row.id !== id)); + } + }; + + // A state event carries the full row, so it replaces the cached one. Its + // bytes are the row's own, so the progress seen before it is out of date. + const onState = (event: StateEvent): void => { + // The cached row is a plain row; the unhandled-key reason is the event's. + const row: StateEvent = { ...event }; + delete row.reason; + const hadProgress = forgetProgress(row.id); + if (!rows) { + return; + } + const index = rows.findIndex((cached) => cached.id === row.id); + if (index < 0) { + commit([...rows, row]); + return; + } + if (jsonEqual(rows[index], row)) { + // The row is the same, but the progress view showed newer bytes. + if (hadProgress) { + withProgress = undefined; + notify(progressListeners); + } + return; + } + commit(rows.map((cached, i) => (i === index ? row : cached))); + }; + + const onProgress = (event: ProgressEvent): void => { + if (!rows?.some((row) => row.id === event.id)) { + return; + } + progress.set(event.id, event); + withProgress = undefined; + notify(progressListeners); + }; + + const listening = (): boolean => + rowListeners.size + progressListeners.size > 0; + + const start = (): void => { + subscriptions = [ + addListener('state', onState), + addListener('progress', onProgress), + ]; + // Rows kept from an earlier listener can have missed events. + if (!fresh || !rows) { + refresh(); + } + }; + + // The rows stay, so a component that replaces the last listener in one + // commit does not render twice. The next start() checks them against native. + const stop = (): void => { + subscriptions.forEach((subscription) => subscription.remove()); + subscriptions = []; + // Whatever changes from here on is checked at the next start(). + fresh = false; + progress.clear(); + overlaid.clear(); + withProgress = undefined; + }; + + const subscriber = + (listeners: Set) => + (listener: Listener): (() => void) => { + const first = !listening(); + // A wrapper, so the same function subscribed twice is two entries. + const entry = () => listener(); + listeners.add(entry); + if (first) { + start(); + } + return () => { + if (!listeners.delete(entry)) { + return; + } + if (!listening()) { + stop(); + } + }; + }; + + const rowsWithProgress = (): readonly RequestRow[] => { + if (withProgress) { + return withProgress; + } + const base = ensureRows(); + const next = base.map((row) => { + const event = progress.get(row.id); + if (!event) { + return row; + } + const memo = overlaid.get(row.id); + if (memo && memo.base === row && memo.event === event) { + return memo.row; + } + const merged = { + ...row, + bytesSent: event.bytesSent, + totalBytes: event.totalBytes, + }; + overlaid.set(row.id, { base: row, event, row: merged }); + return merged; + }); + withProgress = sameItems(next, base) ? base : next; + return withProgress; + }; + + const acked = ({ id, kind }: { id: string; kind: string }): void => { + // Native forgets a row at the ack of its completed or cancelled outcome. + // An error row stays until cancel() or a same-id mutate(). + if (kind !== 'completed' && kind !== 'cancelled') { + return; + } + const row = rows?.find((cached) => cached.id === id); + if (!row) { + return; + } + if (row.state === kind) { + remove(id); + return; + } + // The row moved on, for example a cancelled entry that a same-id + // mutate() reopened, whose old ack does not forget it. Ask native. + const current = readSafely(); + if (!current) { + return; + } + const native = current.find((candidate) => candidate.id === id); + if (!native) { + remove(id); + } else if (!jsonEqual(native, row)) { + onState(native); + } + }; + + const cancelled = (id: string): void => { + // A settled entry is forgotten at once, with no event. A live one + // settles 'cancelled' with an event, and its ack forgets it. + const row = rows?.find((cached) => cached.id === id); + if (row && !LIVE_STATES.includes(row.state)) { + remove(id); + } + }; + + return { + rows: ensureRows, + rowsWithProgress, + subscribe: subscriber(rowListeners), + subscribeWithProgress: subscriber(progressListeners), + read: () => { + if (!listening()) { + refresh(); + } + return rows ?? []; + }, + acked, + cancelled, + }; +}; + +/** + * The hook behind `useRequests()` and `useRequestsWithProgress()`. The + * filter's fields, not the object, key the selector, so an inline filter + * object does not make a new result each render. + */ +export const useRequestRows = ( + subscribe: (listener: Listener) => () => void, + source: () => readonly RequestRow[], + filter: RequestsFilter | undefined, +): readonly RequestRow[] => { + const key = filter?.key; + const id = filter?.id; + const includeCancelled = filter?.includeCancelled === true; + const getSnapshot = useMemo( + () => createSelector(source, { key, id, includeCancelled }), + [source, key, id, includeCancelled], + ); + return useSyncExternalStore(subscribe, getSnapshot); +}; diff --git a/src/testing.ts b/src/testing.ts index 176a61f0..0502de5c 100644 --- a/src/testing.ts +++ b/src/testing.ts @@ -142,7 +142,8 @@ export type FakeNative = Spec & { /** * Journals the outcome of entry `id`, moves its row to the outcome state, * and emits it on onSettled when a listener exists. Resolves with the event - * after delivery acks it. Rejects after `ackTimeoutMs` when no ack comes: + * after delivery acks it and the client has handled the ack, so the row + * hooks no longer show a forgotten row. Rejects after `ackTimeoutMs` when no ack comes: * the handler rejected, the key has no definition, or configure() was not * called. To test those cases, create the fake with a short * `ackTimeoutMs` and expect settle() to reject. @@ -152,7 +153,11 @@ export type FakeNative = Spec & { outcome: FakeOutcome, overrides?: SettledOverrides, ) => Promise; - /** Adds or replaces rows, for example legacy rows that native imported. */ + /** + * Adds or replaces rows, for example legacy rows that native imported. It + * emits no event, so a mounted row hook sees the rows only after it mounts + * again. + */ seedRows: (rows: RequestRow[]) => void; /** * Adds outcomes to the journal. getUnacknowledgedEvents() returns them, so @@ -557,8 +562,8 @@ export const createFakeNative = ( }), ), - ackEvents: (ids) => - run('ackEvents', () => { + ackEvents: (ids) => { + const acked = run('ackEvents', () => { ids.forEach((eventId) => { ackedEventIds.push(eventId); const index = journal.findIndex((e) => e.eventId === eventId); @@ -578,15 +583,25 @@ export const createFakeNative = ( rows.delete(event.id); } }); - // Waiters resolve after the journal and rows are up to date. - ids.forEach((eventId) => { - const waiter = ackWaiters.get(eventId); - if (waiter) { - ackWaiters.delete(eventId); - waiter(); - } - }); - }), + }); + // Waiters resolve after the journal and rows are up to date, and one + // promise turn after the caller's own continuation on this promise. + // So when settle() resolves, the client has handled the ack too, for + // example dropped the row from its row cache. + void acked + .then(() => undefined) + .then(() => { + ids.forEach((eventId) => { + const waiter = ackWaiters.get(eventId); + if (waiter) { + ackWaiters.delete(eventId); + waiter(); + } + }); + }) + .catch(() => undefined); + return acked; + }, takeDiagnostics: () => run('takeDiagnostics', () => diff --git a/src/types.ts b/src/types.ts index 83c74d4c..115f59ec 100644 --- a/src/types.ts +++ b/src/types.ts @@ -199,6 +199,31 @@ export type RequestRow = { nextAttemptAt?: number; }; +/** + * Which cached rows `useRequests()`, `useRequestsWithProgress()` and + * `getCachedRequests()` return. `key` and `id` match as in `getRequests()`. + * Cancelled rows are left out unless `includeCancelled` is true: such a row + * has no work left, and native forgets it at its ack. + */ +export type RequestsFilter = { + key?: string; + id?: string; + includeCancelled?: boolean; +}; + +/** A definition's rows: the filter has no `key`, because it is the definition's. */ +export type DefinedRequestsFilter = Omit; + +/** + * A row of one definition, with `vars` typed as that definition's vars. The + * type is a promise the library cannot check: `vars` is what native stored + * from `JSON.stringify`, so a `Date` reads back as a string, and an entry + * enqueued under an older vars type keeps its old shape. + */ +export type DefinedRequestRow = Omit & { + vars: V; +}; + /** * A `state` event. `reason: 'unhandled-key'` reports an outcome whose key has * no definition. The row carries the entry's real state, and the library keeps @@ -286,6 +311,14 @@ export type Defined = { mutate: [V] extends [null] ? (vars?: null, options?: { id?: string }) => Promise<{ id: string }> : (vars: V, options?: { id?: string }) => Promise<{ id: string }>; + /** The client's `useRequests()` for this definition's key, with typed vars. */ + useRequests: ( + filter?: DefinedRequestsFilter, + ) => readonly DefinedRequestRow[]; + /** The client's `useRequestsWithProgress()` for this definition's key. */ + useRequestsWithProgress: ( + filter?: DefinedRequestsFilter, + ) => readonly DefinedRequestRow[]; }; /** @@ -386,6 +419,30 @@ export type UploadClient = { setWifiOnly: (enabled: boolean) => Promise; updateHeaders: (patch: Record) => Promise; getRequests: (filter?: { key?: string; id?: string }) => RequestRow[]; + /** + * React hook. The cached rows: read from native when the first listener + * arrives, then patched by events. Re-renders when a row appears, changes + * state, or goes, not on progress. The same array returns while nothing it + * holds changed. + */ + useRequests: (filter?: RequestsFilter) => readonly RequestRow[]; + /** + * React hook. `useRequests()` with the bytes of each progress event, so it + * also re-renders on progress. Use it only where bytes are shown. + */ + useRequestsWithProgress: (filter?: RequestsFilter) => readonly RequestRow[]; + /** + * Calls `listener` with `getCachedRequests()` each time a row appears, + * changes state, or goes. Keeps the cache up to date while subscribed. + */ + addRequestsListener: (listener: (rows: readonly RequestRow[]) => void) => { + remove: () => void; + }; + /** + * The rows the hooks see, for code outside React. With no listener and no + * mounted hook, it reads native first. + */ + getCachedRequests: (filter?: RequestsFilter) => readonly RequestRow[]; takeDiagnostics: () => Promise; addListener: AddListener; chunkPlan: ( From 07e43b5c14d9fa85d905ba2374feb80750dc6305 Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 14:36:15 -0500 Subject: [PATCH 05/11] Document the row hooks, their limits, and how the fake drives them Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 17 ++++++++++++++ README.md | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1753a72b..7a1ace73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ ## Unreleased +Added `useRequests(filter?)` and `useRequestsWithProgress(filter?)`, React +hooks that return the queue rows for display, and +`definition.useRequests()` and `definition.useRequestsWithProgress()`, which +return one definition's rows with typed `vars`. The client keeps one copy of +the rows in memory, read from native when the first hook mounts and updated +by each `state` event. It also removes a row when native forgets it with no +event: after the ack of a `completed` or `cancelled` outcome, and after +`cancel()` on a settled entry. The filter takes `includeCancelled`; cancelled +rows are left out by default. `addRequestsListener()` and +`getCachedRequests()` give the same rows outside React. An app that keeps its +own copy of `getRequests()` for display, and wraps `cancel()` to update it, +can remove that code. Delivery, acks and cancel are unchanged. `define()` +now returns the two hooks with `key` and `mutate`. + +`createFakeNative().settle()` now resolves one promise turn later, after the +client has handled the ack, so the row hooks are up to date when it resolves. + Behavior change: `mutate()` no longer rejects when native is slow to answer. Before, it rejected after `enqueueTimeoutMs` (default 10 s) even though native could still store the entry and upload it. On iOS a large chunked file can hold diff --git a/README.md b/README.md index e92a9055..dd1c1848 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ carries the last one. `createUploadClient()` returns a client. The default export is one client; an app needs one. -### `define(definition): { key, mutate }` +### `define(definition): { key, mutate, useRequests, useRequestsWithProgress }` ```ts type Definition = @@ -465,6 +465,62 @@ uploads .forEach((row) => uploads.cancel(row.id)); ``` +### `useRequests(filter?)` and `useRequestsWithProgress(filter?)` +React hooks that return the queue rows, for display. The client keeps one +copy of the rows in memory. It reads native when the first hook mounts, and +each `state` event updates the row it carries. Every component shares that +copy, so the app does not keep its own. + +```tsx +const CaptureBadge = ({ captureId }: { captureId: string }) => { + // Typed vars: the rows of this definition only. + const rows = captureUpload.useRequests(); + const pending = rows.filter( + (row) => row.vars.captureId === captureId && row.state !== 'completed', + ); + return pending.length > 0 ? : null; +}; +``` + +- `filter` is `{ key?, id?, includeCancelled? }`. `key` and `id` match as in + `getRequests()`. Cancelled rows are left out unless `includeCancelled` is + `true`. A cancelled row has no work left, and native forgets it at its ack. +- `useRequests()` re-renders when a row appears, changes state, or goes. A + row's bytes are those of its last `state` event. +- `useRequestsWithProgress()` also re-renders on each `progress` event of a + row it returns, with that event's bytes. Use it only where bytes are shown. +- The result is the same array while the rows it holds are the same, even + when the filter is a new object each render. The array and its rows are + shared, so do not change them. +- `definition.useRequests(filter?)` and `definition.useRequestsWithProgress(filter?)` + return the rows of that definition's key, with `vars` typed as its vars. + The type is not checked at run time: `vars` is what native stored, so a + `Date` reads back as a string, and a row enqueued under an older vars type + keeps its old shape. + +Native emits no event when it forgets a row, so the library removes the row +itself when its own call made native forget it: when the ack of a +`completed` or `cancelled` outcome resolves, and when `cancel(id)` resolves +on a settled entry. An `error` row stays until `cancel()` or a same-id +`mutate()`, as in native. A same-id `mutate()` emits a `state` event, so it +needs nothing extra. + +Limits. While no hook or listener is mounted, the copy is not updated. The +first one to mount checks it against native, so it can show the old rows +for one render. When the native read fails, the library keeps the rows it +has and sends a warning to `onWarning`. Any change that native makes with no +event and that this library's JS did not start is missed until the next +check. A review of the native enqueue, pause, resume, header, cancel and +ack paths found no such change. + +### `addRequestsListener(listener)` and `getCachedRequests(filter?)` +The same rows outside React, for code such as a store or a logger. +`addRequestsListener` calls `listener` with `getCachedRequests()` each time a +row appears, changes state, or goes, and returns `{ remove }`. It keeps the +copy up to date as a mounted hook does. `getCachedRequests(filter?)` returns +the rows the hooks see. While nothing listens, it reads native first. Without +`key` or `id` it returns the same array while nothing changed. + ### `takeDiagnostics(): Promise` The library's persisted diagnostic records, oldest first. Each call returns the records since the last call and clears them, so send them somewhere @@ -594,6 +650,14 @@ The fake does not model the same-id rules (resume, replace, `E_RUNNING`, re-emit) or the header merge of `updateHeaders()`; script those with `failNext()` and `seedUnacknowledged()`. +The fake drives the row hooks as native does. `settle()` resolves after the +client has handled the ack, so a completed row is already gone from +`useRequests()`. `seedRows()` and `reset()` change rows with no event, as +leftovers from a dead session would. Call them before the first hook mounts +or listener subscribes, or unmount and mount again: the next mount checks the +rows against the fake. The hooks need a React renderer, for example +`react-test-renderer`, and its updates wrapped in `act()`. + `native.entries` holds every enqueue as `mutate()` sent it, with `vars` and `data` parsed back from JSON. `failNext()` makes the next native call reject with a code such as `E_STORAGE`. `holdNext('enqueue')` keeps the next enqueue From 130726f5ab16d832150106e09dd0940bf262a318 Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 14:50:56 -0500 Subject: [PATCH 06/11] Fix the row cache after review: always check at subscribe, index by id Correctness: - The first subscribe always checks native. Native emits from its own thread, so an event can go to no listener between a render's read and the subscribe. reconcile() keeps the same array when nothing changed. - The ack of an error outcome reads that row from native when the cache does not show it as an error: iOS can settle a live row during an ack with no event. - An unhandled-key state row is read from native, because JS builds it from the journaled outcome and a same-id mutate() may have reopened the entry. - addRequestsListener() is not called while it registers, and not called when its rows did not change (only a hidden cancelled row did). Performance: - An id index per rows array, shared by all selectors. An id-filtered hook looks its row up instead of walking every row. - Hooks without an id share one selector per source, key and includeCancelled. - Row equality compares scalar fields first and allocates nothing. vars is always compared. - getCachedRequests() with nothing listening takes native's rows as they are; the next subscribe reconciles them. - Native progress is listened to only while a progress hook is mounted, and a dropped row's progress is forgotten. Docs: take the hooks out of the client before use, because the hooks lint rule skips lowercase X.useY(); completed rows stay until their ack. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- README.md | 51 ++- .../__tests__/useRequests.test.tsx | 39 ++- src/__tests__/requests.test.ts | 205 ++++++++++- src/index.ts | 17 +- src/requests.ts | 327 ++++++++++++++---- src/types.ts | 10 +- 7 files changed, 537 insertions(+), 115 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a1ace73..b67bf7df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ by each `state` event. It also removes a row when native forgets it with no event: after the ack of a `completed` or `cancelled` outcome, and after `cancel()` on a settled entry. The filter takes `includeCancelled`; cancelled rows are left out by default. `addRequestsListener()` and -`getCachedRequests()` give the same rows outside React. An app that keeps its +`getCachedRequests()` give the same rows outside React. Completed rows are +included until their outcome is acknowledged. An app that keeps its own copy of `getRequests()` for display, and wraps `cancel()` to update it, can remove that code. Delivery, acks and cancel are unchanged. `define()` now returns the two hooks with `key` and `mutate`. diff --git a/README.md b/README.md index dd1c1848..9fad9758 100644 --- a/README.md +++ b/README.md @@ -472,9 +472,16 @@ each `state` event updates the row it carries. Every component shares that copy, so the app does not keep its own. ```tsx +// Take the hooks out of the client and the definition at module level. The +// hooks lint rule checks `X.useY()` only when `X` starts with a capital +// letter, so `uploads.useRequests()` inside a condition is not flagged. +// `useRequests()` called as a plain name is checked. +const { useRequests } = uploads; +const { useRequests: useCaptureRows } = captureUpload; + const CaptureBadge = ({ captureId }: { captureId: string }) => { // Typed vars: the rows of this definition only. - const rows = captureUpload.useRequests(); + const rows = useCaptureRows(); const pending = rows.filter( (row) => row.vars.captureId === captureId && row.state !== 'completed', ); @@ -485,13 +492,18 @@ const CaptureBadge = ({ captureId }: { captureId: string }) => { - `filter` is `{ key?, id?, includeCancelled? }`. `key` and `id` match as in `getRequests()`. Cancelled rows are left out unless `includeCancelled` is `true`. A cancelled row has no work left, and native forgets it at its ack. +- Completed rows are included. A completed row stays until its outcome is + acknowledged. When its handler rejects or its key has no definition, the + outcome is not acknowledged, so the row stays for the whole session. Do not + treat a completed row as work still to do. - `useRequests()` re-renders when a row appears, changes state, or goes. A row's bytes are those of its last `state` event. - `useRequestsWithProgress()` also re-renders on each `progress` event of a row it returns, with that event's bytes. Use it only where bytes are shown. - The result is the same array while the rows it holds are the same, even - when the filter is a new object each render. The array and its rows are - shared, so do not change them. + when the filter is a new object each render. A component filtered by `id` + re-renders only when that row changes. The array and its rows are shared, + so do not change them. - `definition.useRequests(filter?)` and `definition.useRequestsWithProgress(filter?)` return the rows of that definition's key, with `vars` typed as its vars. The type is not checked at run time: `vars` is what native stored, so a @@ -503,23 +515,30 @@ itself when its own call made native forget it: when the ack of a `completed` or `cancelled` outcome resolves, and when `cancel(id)` resolves on a settled entry. An `error` row stays until `cancel()` or a same-id `mutate()`, as in native. A same-id `mutate()` emits a `state` event, so it -needs nothing extra. +needs nothing extra. iOS can settle a live row during an ack with no event; +after any ack whose row the copy does not show in the outcome's state, the +library reads that row from native. -Limits. While no hook or listener is mounted, the copy is not updated. The -first one to mount checks it against native, so it can show the old rows -for one render. When the native read fails, the library keeps the rows it -has and sends a warning to `onWarning`. Any change that native makes with no -event and that this library's JS did not start is missed until the next -check. A review of the native enqueue, pause, resume, header, cancel and -ack paths found no such change. +Limits. While no hook or listener is mounted, the copy is not updated. Each +time the first hook or listener subscribes, the library checks the copy +against native, so a mount can show old rows for one render. When the +native read fails, the library keeps the rows it has and sends a warning to +`onWarning`. Any other change that native makes with no event is missed +until the next check. ### `addRequestsListener(listener)` and `getCachedRequests(filter?)` The same rows outside React, for code such as a store or a logger. -`addRequestsListener` calls `listener` with `getCachedRequests()` each time a -row appears, changes state, or goes, and returns `{ remove }`. It keeps the -copy up to date as a mounted hook does. `getCachedRequests(filter?)` returns -the rows the hooks see. While nothing listens, it reads native first. Without -`key` or `id` it returns the same array while nothing changed. +`addRequestsListener` returns `{ remove }` and calls `listener` with +`getCachedRequests()` each time those rows change. There is no call when it +registers, so read `getCachedRequests()` after it returns. It keeps the copy +up to date as a mounted hook does. + +`getCachedRequests(filter?)` returns the rows the hooks see. While nothing +listens, it reads native on each call and returns a new array. While a hook +or listener is mounted and the filter has no `key` or `id`, it returns the +same array until a row changes. A result filtered by `key` or `id` is a new +array on each call, so do not use it as a `useSyncExternalStore` snapshot; +use the hooks. ### `takeDiagnostics(): Promise` The library's persisted diagnostic records, oldest first. Each call returns diff --git a/example/RNBGUExample/__tests__/useRequests.test.tsx b/example/RNBGUExample/__tests__/useRequests.test.tsx index 138a28e9..b7690c46 100644 --- a/example/RNBGUExample/__tests__/useRequests.test.tsx +++ b/example/RNBGUExample/__tests__/useRequests.test.tsx @@ -80,7 +80,7 @@ const mount = (element: React.ReactElement): ReactTestRenderer => { return tree; }; -it('reads native once for two mounted hooks', () => { +it('reads native at the first render and at the first subscribe, not per hook', () => { native.seedRows([row()]); const getRequests = jest.spyOn(native, 'getRequests'); const one = newSeen(); @@ -91,9 +91,44 @@ it('reads native once for two mounted hooks', () => { , ); - expect(getRequests).toHaveBeenCalledTimes(1); + // One read for the render, one check when the first hook subscribes. + expect(getRequests).toHaveBeenCalledTimes(2); expect(last(one)).toEqual([row()]); expect(last(two)).toEqual([row()]); + // The check found nothing new, so no hook rendered again. + expect(one.renders).toBe(1); +}); + +it('works under StrictMode, which subscribes twice', () => { + native.seedRows([row()]); + const seen = newSeen(); + const tree = mount( + + + , + ); + expect(native.listenerCount('state')).toBe(1); + expect(native.listenerCount('progress')).toBe(1); + act(() => native.emit.progress({id: 'a', bytesSent: 7, totalBytes: 100})); + expect(last(seen)[0]!.bytesSent).toBe(7); + act(() => native.emit.state(row({state: 'running', updatedAt: 2}))); + expect(last(seen)[0]!.state).toBe('running'); + act(() => tree.unmount()); + expect(native.listenerCount('state')).toBe(0); + expect(native.listenerCount('progress')).toBe(0); +}); + +it('follows a filter whose value changes', () => { + native.seedRows([row({id: 'a'}), row({id: 'b', key: 'other'})]); + const seen = newSeen(); + const tree = mount(); + expect(last(seen).map(r => r.id)).toEqual(['a']); + act(() => tree.update()); + expect(last(seen).map(r => r.id)).toEqual(['b']); + act(() => tree.update()); + expect(last(seen).map(r => r.id)).toEqual(['a']); + act(() => native.emit.state(row({id: 'b', key: 'other', state: 'running'}))); + expect(last(seen).map(r => r.id)).toEqual(['a']); }); it('re-renders on a state event, and only the progress hook on progress', () => { diff --git a/src/__tests__/requests.test.ts b/src/__tests__/requests.test.ts index 5aeed202..46d075db 100644 --- a/src/__tests__/requests.test.ts +++ b/src/__tests__/requests.test.ts @@ -10,7 +10,13 @@ jest.mock('react-native', () => { }); import { createUploadClient } from '../index'; -import { createRequestCache, createSelector } from '../requests'; +import { + createRequestCache, + createSelector, + jsonEqual, + rowEqual, + sharedSelector, +} from '../requests'; import { createFakeNative, type FakeNative } from '../testing'; import type { RequestRow } from '../types'; @@ -63,13 +69,16 @@ describe('first subscribe', () => { expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); }); - it('does not read native again at subscribe right after a render read', () => { - const { native, client, getRequests } = setup(); - native.seedRows([row()]); - // A render reads before its effect subscribes, in the same task. - client.getCachedRequests(); + it('checks native at subscribe, so a row emitted after a render read is there', () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'a' })]); + // A render reads before its effect subscribes. + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a']); + // Native queues a row and emits it while nothing listens: the event is lost. + native.seedRows([row({ id: 'b' })]); + native.emit.state(row({ id: 'b' })); listen(client); - expect(getRequests).toHaveBeenCalledTimes(1); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); }); }); @@ -106,12 +115,32 @@ describe('state events', () => { expect(calls).toHaveLength(0); }); - it('drop the unhandled-key reason from the row', () => { + it('take an unhandled-key row from native, not from the outcome', () => { const { native, client } = setup(); - native.seedRows([row()]); + // A same-id mutate() reopened the entry after the outcome was journaled. + native.seedRows([row({ state: 'queued', updatedAt: 5 })]); listen(client); - native.emit.state({ ...row({ state: 'completed' }), reason: 'unhandled-key' }); - expect(client.getCachedRequests()[0]).toEqual(row({ state: 'completed' })); + native.emit.state({ + ...row({ state: 'completed', updatedAt: 3 }), + reason: 'unhandled-key', + }); + expect(client.getCachedRequests()).toEqual([ + row({ state: 'queued', updatedAt: 5 }), + ]); + }); + + it('show the native row of an unhandled-key outcome, without the reason', () => { + const { native, client } = setup(); + native.seedRows([row({ state: 'running' })]); + listen(client); + native.seedRows([row({ state: 'completed', updatedAt: 2 })]); + native.emit.state({ + ...row({ state: 'completed', updatedAt: 2 }), + reason: 'unhandled-key', + }); + expect(client.getCachedRequests()).toEqual([ + row({ state: 'completed', updatedAt: 2 }), + ]); }); }); @@ -311,7 +340,7 @@ describe('the ack of an outcome whose row moved on', () => { expect(cache.rows()).toEqual([]); }); - it('does nothing for an error outcome', () => { + it('does nothing for an error outcome on an error row', () => { const { cache, readNative } = stub([row({ id: 'a', state: 'error' })]); cache.subscribe(() => {}); readNative.mockClear(); @@ -319,6 +348,18 @@ describe('the ack of an outcome whose row moved on', () => { expect(readNative).not.toHaveBeenCalled(); expect(cache.rows()).toHaveLength(1); }); + + it('asks native for an error outcome on a row still shown live', () => { + // iOS can settle a live row during the ack with no event. + const nativeRows = [row({ id: 'a', state: 'running' })]; + const { cache, readNative } = stub(nativeRows); + cache.subscribe(() => {}); + nativeRows[0] = row({ id: 'a', state: 'error', updatedAt: 2 }); + readNative.mockClear(); + cache.acked({ id: 'a', kind: 'error' }); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows()).toEqual([row({ id: 'a', state: 'error', updatedAt: 2 })]); + }); }); describe('unsubscribe and subscribe again', () => { @@ -337,7 +378,8 @@ describe('unsubscribe and subscribe again', () => { expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); // Row 'a' did not change, so it keeps its identity. expect(client.getCachedRequests()[0]).toBe(kept[0]); - expect(second.calls).toHaveLength(1); + // No call while the listener registers; it reads the rows itself. + expect(second.calls).toHaveLength(0); }); it('keeps the same array when native has not changed', async () => { @@ -364,13 +406,13 @@ describe('unsubscribe and subscribe again', () => { expect(native.listenerCount('state')).toBe(before); }); - it('reads native on every getCachedRequests() while nobody listens', async () => { - const { native, client } = setup(); + it('reads native on every getCachedRequests() while nobody listens', () => { + const { native, client, getRequests } = setup(); native.seedRows([row({ id: 'a' })]); expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a']); - await flush(); native.seedRows([row({ id: 'b' })]); expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); + expect(getRequests).toHaveBeenCalledTimes(2); }); it('warns and keeps the rows when the native read throws', async () => { @@ -469,3 +511,134 @@ describe('createSelector', () => { expect(select()).toBe(first); }); }); + +describe('addRequestsListener', () => { + it('is not called while it registers, even when the check finds a change', async () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'a' })]); + const first = listen(client); + first.subscription.remove(); + native.seedRows([row({ id: 'b' })]); + const listener = jest.fn(); + client.addRequestsListener(listener); + expect(listener).not.toHaveBeenCalled(); + expect(client.getCachedRequests().map((r) => r.id)).toEqual(['a', 'b']); + }); + + it('is not called when only a hidden cancelled row changes', () => { + const { native, client } = setup(); + native.seedRows([row({ id: 'a' }), row({ id: 'c', state: 'cancelled' })]); + const { calls } = listen(client); + native.emit.state(row({ id: 'c', state: 'cancelled', updatedAt: 9 })); + expect(calls).toHaveLength(0); + native.emit.state(row({ id: 'a', state: 'running' })); + expect(calls).toHaveLength(1); + }); + + it('may call the client while the cache drops a row after an ack', async () => { + const { native, client, ping, warnings } = setup(); + const { id: first } = await ping.mutate({ n: 1 }); + const { id: second } = await ping.mutate({ n: 2 }); + await native.settle(second, { kind: 'error', error: { errorKind: 'http' } }); + const seen: string[][] = []; + const cancels: Array> = []; + client.addRequestsListener((rows) => { + seen.push(rows.map((r) => r.id)); + // Reads native and starts a cancel from inside the notification. + client.getRequests(); + if (cancels.length === 0 && !rows.some((r) => r.id === first)) { + cancels.push(client.cancel(second)); + } + }); + await native.settle(first, { kind: 'completed' }); + await Promise.all(cancels); + expect(cancels).toHaveLength(1); + expect(client.getCachedRequests()).toEqual([]); + expect(native.getRequests()).toEqual([]); + // The completed state event, the drop after the ack, then the cancel. + expect(seen.slice(-2)).toEqual([[second], []]); + expect(warnings).toEqual([]); + }); +}); + +describe('selector work', () => { + /** An array that counts reads of its elements. */ + const counted = (rows: RequestRow[]) => { + const reads = { count: 0 }; + const proxy = new Proxy(rows, { + get: (target, prop, receiver) => { + if (typeof prop === 'string' && /^\d+$/.test(prop)) { + reads.count++; + } + return Reflect.get(target, prop, receiver); + }, + }); + return { proxy, reads }; + }; + + it('looks an id up in one index per rows array, without walking the rows each time', () => { + const many = Array.from({ length: 100 }, (_, i) => row({ id: `r${i}` })); + const { proxy, reads } = counted(many); + const selectors = Array.from({ length: 50 }, (_, i) => + createSelector(() => proxy, { id: `r${i}` }), + ); + const results = selectors.map((select) => select()); + // One index build reads each row once; 50 selectors share it. + expect(reads.count).toBe(100); + expect(results[7]).toEqual([many[7]]); + expect(selectors[7]!()).toBe(results[7]); + }); + + it('keeps an id result when a new array holds the same row object', () => { + const rows = [row({ id: 'a' }), row({ id: 'b' })]; + let current = rows; + const select = createSelector(() => current, { id: 'a' }); + const first = select(); + current = [rows[0]!, row({ id: 'b', state: 'running' })]; + expect(select()).toBe(first); + current = [row({ id: 'a', state: 'cancelled' }), current[1]!]; + expect(select()).toEqual([]); + }); + + it('shares one selector per source and filter without an id', () => { + const rows = [row({ id: 'a', key: 'k1' }), row({ id: 'b', key: 'k2' })]; + const source = () => rows; + expect(sharedSelector(source, {})).toBe(sharedSelector(source, {})); + expect(sharedSelector(source, { key: 'k1' })).toBe( + sharedSelector(source, { key: 'k1' }), + ); + expect(sharedSelector(source, { key: 'k1' })).not.toBe( + sharedSelector(source, {}), + ); + expect(sharedSelector(source, { includeCancelled: true })).not.toBe( + sharedSelector(source, {}), + ); + expect(sharedSelector(source, { key: 'k1' })().map((r) => r.id)).toEqual([ + 'a', + ]); + }); +}); + +describe('jsonEqual', () => { + it('compares JSON values deeply, with undefined fields as absent', () => { + expect(jsonEqual({ a: [1, { b: null }] }, { a: [1, { b: null }] })).toBe( + true, + ); + expect(jsonEqual({ a: 1, b: undefined }, { a: 1 })).toBe(true); + expect(jsonEqual({ a: 1 }, { a: 1, b: 2 })).toBe(false); + expect(jsonEqual({ a: 1, c: 2 }, { a: 1, b: 2 })).toBe(false); + expect(jsonEqual([1, 2], [1, 2, 3])).toBe(false); + expect(jsonEqual([], {})).toBe(false); + expect(jsonEqual(null, {})).toBe(false); + expect(jsonEqual('1', 1)).toBe(false); + }); + + it('finds a changed vars field when every scalar field is the same', () => { + expect( + rowEqual(row({ vars: { n: 1 } }), row({ vars: { n: 2 } })), + ).toBe(false); + expect(rowEqual(row({ vars: { n: 1 } }), row({ vars: { n: 1 } }))).toBe( + true, + ); + }); +}); diff --git a/src/index.ts b/src/index.ts index 3e541c3c..d68fe83b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -343,13 +343,28 @@ export const createUploadClient = ({ const addRequestsListener = ( listener: (rows: readonly RequestRow[]) => void, ): { remove: () => void } => { + // No call while it registers: the caller reads getCachedRequests() after + // this returns. After that, a call only when the rows it gets changed, + // not when only a hidden cancelled row did. + let registering = true; + let last: readonly RequestRow[] | undefined; const remove = requests.subscribe(() => { + if (registering) { + return; + } + const rows = visibleRows(); + if (rows === last) { + return; + } + last = rows; try { - listener(visibleRows()); + listener(rows); } catch (e) { warn('addRequestsListener: a listener threw', e); } }); + registering = false; + last = visibleRows(); return { remove }; }; diff --git a/src/requests.ts b/src/requests.ts index c89f1794..4eb97a8f 100644 --- a/src/requests.ts +++ b/src/requests.ts @@ -12,7 +12,6 @@ import { useMemo, useSyncExternalStore } from 'react'; import type { AddListener, - Json, ProgressEvent, RequestRow, RequestsFilter, @@ -29,7 +28,11 @@ const LIVE_STATES: readonly RequestState[] = [ 'paused', ]; -/** Deep equality for JSON values, as `vars` and rows hold them. */ +/** + * Deep equality for JSON values, as `vars` and rows hold them. A field set to + * `undefined` counts as absent. It allocates nothing, because it runs on + * every state event and on every row of a native read. + */ export const jsonEqual = (a: unknown, b: unknown): boolean => { if (a === b) { return true; @@ -42,38 +45,110 @@ export const jsonEqual = (a: unknown, b: unknown): boolean => { ) { return false; } - if (Array.isArray(a) !== Array.isArray(b)) { + const isArray = Array.isArray(a); + if (isArray !== Array.isArray(b)) { return false; } - if (Array.isArray(a)) { - const other = b as unknown[]; - return ( - a.length === other.length && a.every((v, i) => jsonEqual(v, other[i])) - ); + if (isArray) { + const left = a as unknown[]; + const right = b as unknown[]; + if (left.length !== right.length) { + return false; + } + for (let i = 0; i < left.length; i++) { + if (!jsonEqual(left[i], right[i])) { + return false; + } + } + return true; } - const left = a as Record; - const right = b as Record; - const leftKeys = Object.keys(left).filter((k) => left[k] !== undefined); - const rightKeys = Object.keys(right).filter((k) => right[k] !== undefined); - return ( - leftKeys.length === rightKeys.length && - leftKeys.every((k) => jsonEqual(left[k], right[k])) - ); + const left = a as Record; + const right = b as Record; + // Count the defined fields of each side; equal counts plus every left + // field equal means the same set of fields. + let fields = 0; + for (const k in left) { + if (left[k] === undefined) { + continue; + } + fields++; + if (!jsonEqual(left[k], right[k])) { + return false; + } + } + for (const k in right) { + if (right[k] !== undefined) { + fields--; + } + } + return fields === 0; }; -const sameItems = (a: readonly T[], b: readonly T[]): boolean => - a.length === b.length && a.every((item, i) => item === b[i]); +/** + * Row equality. The scalar fields that change on most transitions are + * compared first, so a changed row is found without walking its `vars`. + * `vars` is always compared: a same-id mutate() can replace it. + */ +export const rowEqual = (a: RequestRow, b: RequestRow): boolean => + a === b || + (a.id === b.id && + a.state === b.state && + a.updatedAt === b.updatedAt && + a.attempts === b.attempts && + a.bytesSent === b.bytesSent && + a.totalBytes === b.totalBytes && + a.nextAttemptAt === b.nextAttemptAt && + a.key === b.key && + jsonEqual(a, b)); + +const sameItems = (a: readonly T[], b: readonly T[]): boolean => { + if (a.length !== b.length) { + return false; + } + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) { + return false; + } + } + return true; +}; export const matches = (row: RequestRow, filter: RequestsFilter): boolean => (filter.includeCancelled === true || row.state !== 'cancelled') && (filter.key === undefined || row.key === filter.key) && (filter.id === undefined || row.id === filter.id); +// One id index per rows array. The cache makes a new array for each change, +// so every id-filtered selector of that change shares one build. +const indexes = new WeakMap< + readonly RequestRow[], + ReadonlyMap +>(); + +/** The rows of `rows` by id, built once per array. */ +export const indexById = ( + rows: readonly RequestRow[], +): ReadonlyMap => { + let index = indexes.get(rows); + if (!index) { + const built = new Map(); + for (const row of rows) { + built.set(row.id, row); + } + indexes.set(rows, built); + index = built; + } + return index; +}; + +const EMPTY: readonly RequestRow[] = []; + /** * A read of `source()` narrowed by `filter`. It returns the same array while * the rows it keeps are the same objects, so a row it leaves out can change * without a new result. `useSyncExternalStore` then re-renders only when the - * filtered rows change. + * filtered rows change. With an `id`, it looks the row up in the shared + * index and does not walk the rows. */ export const createSelector = ( source: () => readonly RequestRow[], @@ -81,6 +156,24 @@ export const createSelector = ( ): (() => readonly RequestRow[]) => { let lastSource: readonly RequestRow[] | undefined; let lastResult: readonly RequestRow[] | undefined; + if (filter.id !== undefined) { + const { id } = filter; + let lastRow: RequestRow | undefined; + return () => { + const rows = source(); + if (rows === lastSource && lastResult) { + return lastResult; + } + lastSource = rows; + const row = indexById(rows).get(id); + const kept = row && matches(row, filter) ? row : undefined; + if (!lastResult || kept !== lastRow) { + lastRow = kept; + lastResult = kept ? [kept] : EMPTY; + } + return lastResult; + }; + } return () => { const rows = source(); if (rows === lastSource && lastResult) { @@ -95,6 +188,36 @@ export const createSelector = ( }; }; +// One selector per source and filter without an id, shared by every hook +// that asks for it, so all unfiltered or same-key hooks filter the rows once +// per change. The number of entries is the number of distinct keys used. +const shared = new WeakMap< + () => readonly RequestRow[], + Map readonly RequestRow[]> +>(); + +/** `createSelector()` for a filter without an id, shared per source. */ +export const sharedSelector = ( + source: () => readonly RequestRow[], + filter: Omit, +): (() => readonly RequestRow[]) => { + let bySource = shared.get(source); + if (!bySource) { + bySource = new Map(); + shared.set(source, bySource); + } + const includeCancelled = filter.includeCancelled === true; + const name = `${includeCancelled ? 1 : 0}:${filter.key ?? ''}:${ + filter.key === undefined ? 0 : 1 + }`; + let selector = bySource.get(name); + if (!selector) { + selector = createSelector(source, { key: filter.key, includeCancelled }); + bySource.set(name, selector); + } + return selector; +}; + type RequestCacheDeps = { /** The client's getRequests() without a filter. May throw. */ readNative: () => RequestRow[]; @@ -140,11 +263,9 @@ export const createRequestCache = ({ >(); const rowListeners = new Set(); const progressListeners = new Set(); - let subscriptions: Array<{ remove: () => void }> = []; - // True from a native read until the next microtask. Native events arrive - // as new tasks, so none can be missed between a read and a subscribe in - // the same task, and the subscribe does not need to read again. - let fresh = false; + let stateSubscription: { remove: () => void } | undefined; + // Native progress is listened to only while a progress view is mounted. + let progressSubscription: { remove: () => void } | undefined; const notify = (listeners: Set): void => { listeners.forEach((listener) => listener()); @@ -162,6 +283,15 @@ export const createRequestCache = ({ } rows = next; withProgress = undefined; + // Progress of a row that left is not needed any more. + if (progress.size > 0) { + const index = indexById(next); + progress.forEach((_event, id) => { + if (!index.has(id)) { + forgetProgress(id); + } + }); + } notify(rowListeners); notify(progressListeners); }; @@ -169,12 +299,7 @@ export const createRequestCache = ({ /** Native's rows, or undefined when the read throws. */ const readSafely = (): RequestRow[] | undefined => { try { - const current = readNative(); - fresh = true; - void Promise.resolve().then(() => { - fresh = false; - }); - return current; + return readNative(); } catch (e) { warn( 'requests: getRequests failed, so the cached rows were not checked against native', @@ -192,10 +317,10 @@ export const createRequestCache = ({ cachedRows: readonly RequestRow[], current: RequestRow[], ): readonly RequestRow[] => { - const cached = new Map(cachedRows.map((row) => [row.id, row])); + const cached = indexById(cachedRows); const next = current.map((row) => { const old = cached.get(row.id); - return old && jsonEqual(old, row) ? old : row; + return old && rowEqual(old, row) ? old : row; }); return sameItems(next, cachedRows) ? cachedRows : next; }; @@ -222,27 +347,23 @@ export const createRequestCache = ({ const remove = (id: string): void => { forgetProgress(id); - if (rows?.some((row) => row.id === id)) { + if (rows && indexById(rows).has(id)) { commit(rows.filter((row) => row.id !== id)); } }; - // A state event carries the full row, so it replaces the cached one. Its - // bytes are the row's own, so the progress seen before it is out of date. - const onState = (event: StateEvent): void => { - // The cached row is a plain row; the unhandled-key reason is the event's. - const row: StateEvent = { ...event }; - delete row.reason; + /** Puts `row` in the cache, in place of the row with its id. */ + const upsert = (row: RequestRow): void => { const hadProgress = forgetProgress(row.id); if (!rows) { return; } - const index = rows.findIndex((cached) => cached.id === row.id); - if (index < 0) { + const cached = indexById(rows).get(row.id); + if (!cached) { commit([...rows, row]); return; } - if (jsonEqual(rows[index], row)) { + if (rowEqual(cached, row)) { // The row is the same, but the progress view showed newer bytes. if (hadProgress) { withProgress = undefined; @@ -250,11 +371,39 @@ export const createRequestCache = ({ } return; } - commit(rows.map((cached, i) => (i === index ? row : cached))); + commit(rows.map((other) => (other === cached ? row : other))); + }; + + /** Asks native for one row: replaces it, or drops it when native has none. */ + const refreshId = (id: string): void => { + const current = readSafely(); + if (!current) { + return; + } + const row = current.find((candidate) => candidate.id === id); + if (row) { + upsert(row); + } else { + remove(id); + } + }; + + // A state event carries the full row, so it replaces the cached one. Its + // bytes are the row's own, so the progress seen before it is out of date. + const onState = (event: StateEvent): void => { + if (event.reason === 'unhandled-key') { + // JS builds this row from the journaled outcome, not from the entry. + // A same-id mutate() may have reopened the entry since, so ask native. + if (rows) { + refreshId(event.id); + } + return; + } + upsert(event); }; const onProgress = (event: ProgressEvent): void => { - if (!rows?.some((row) => row.id === event.id)) { + if (!rows || !indexById(rows).has(event.id)) { return; } progress.set(event.id, event); @@ -266,23 +415,27 @@ export const createRequestCache = ({ rowListeners.size + progressListeners.size > 0; const start = (): void => { - subscriptions = [ - addListener('state', onState), - addListener('progress', onProgress), - ]; - // Rows kept from an earlier listener can have missed events. - if (!fresh || !rows) { - refresh(); - } + stateSubscription = addListener('state', onState); + // Always check: an event emitted between an earlier read (a render) and + // this subscribe went to no listener. reconcile() keeps the same array + // when nothing changed, so this adds no render. + refresh(); }; // The rows stay, so a component that replaces the last listener in one // commit does not render twice. The next start() checks them against native. const stop = (): void => { - subscriptions.forEach((subscription) => subscription.remove()); - subscriptions = []; - // Whatever changes from here on is checked at the next start(). - fresh = false; + stateSubscription?.remove(); + stateSubscription = undefined; + }; + + const startProgress = (): void => { + progressSubscription = addListener('progress', onProgress); + }; + + const stopProgress = (): void => { + progressSubscription?.remove(); + progressSubscription = undefined; progress.clear(); overlaid.clear(); withProgress = undefined; @@ -292,9 +445,14 @@ export const createRequestCache = ({ (listeners: Set) => (listener: Listener): (() => void) => { const first = !listening(); + const firstProgress = + listeners === progressListeners && listeners.size === 0; // A wrapper, so the same function subscribed twice is two entries. const entry = () => listener(); listeners.add(entry); + if (firstProgress) { + startProgress(); + } if (first) { start(); } @@ -302,6 +460,9 @@ export const createRequestCache = ({ if (!listeners.delete(entry)) { return; } + if (listeners === progressListeners && listeners.size === 0) { + stopProgress(); + } if (!listening()) { stop(); } @@ -313,6 +474,10 @@ export const createRequestCache = ({ return withProgress; } const base = ensureRows(); + if (progress.size === 0) { + withProgress = base; + return base; + } const next = base.map((row) => { const event = progress.get(row.id); if (!event) { @@ -335,37 +500,37 @@ export const createRequestCache = ({ }; const acked = ({ id, kind }: { id: string; kind: string }): void => { - // Native forgets a row at the ack of its completed or cancelled outcome. - // An error row stays until cancel() or a same-id mutate(). - if (kind !== 'completed' && kind !== 'cancelled') { - return; - } - const row = rows?.find((cached) => cached.id === id); + const row = rows ? indexById(rows).get(id) : undefined; if (!row) { return; } - if (row.state === kind) { - remove(id); + if (kind === 'error') { + // Native keeps an error row. iOS can move a live row to its outcome + // during the ack with no event, so a row not shown as an error is + // checked. + if (row.state !== 'error') { + refreshId(id); + } return; } - // The row moved on, for example a cancelled entry that a same-id - // mutate() reopened, whose old ack does not forget it. Ask native. - const current = readSafely(); - if (!current) { + if (kind !== 'completed' && kind !== 'cancelled') { return; } - const native = current.find((candidate) => candidate.id === id); - if (!native) { + // Native forgets a row at the ack of its completed or cancelled outcome. + if (row.state === kind) { remove(id); - } else if (!jsonEqual(native, row)) { - onState(native); + return; } + // The row moved on, for example a cancelled entry that a same-id + // mutate() reopened, whose old ack does not forget it; or a live row that + // native settled and forgot during the ack. Ask native. + refreshId(id); }; const cancelled = (id: string): void => { // A settled entry is forgotten at once, with no event. A live one // settles 'cancelled' with an event, and its ack forgets it. - const row = rows?.find((cached) => cached.id === id); + const row = rows ? indexById(rows).get(id) : undefined; if (row && !LIVE_STATES.includes(row.state)) { remove(id); } @@ -377,8 +542,15 @@ export const createRequestCache = ({ subscribe: subscriber(rowListeners), subscribeWithProgress: subscriber(progressListeners), read: () => { - if (!listening()) { - refresh(); + if (listening()) { + return rows ?? []; + } + // Nothing listens, so nothing holds these rows: take native's as they + // are. The next start() reconciles them. + const current = readSafely(); + if (current) { + rows = current; + withProgress = undefined; } return rows ?? []; }, @@ -401,7 +573,10 @@ export const useRequestRows = ( const id = filter?.id; const includeCancelled = filter?.includeCancelled === true; const getSnapshot = useMemo( - () => createSelector(source, { key, id, includeCancelled }), + () => + id === undefined + ? sharedSelector(source, { key, includeCancelled }) + : createSelector(source, { key, id, includeCancelled }), [source, key, id, includeCancelled], ); return useSyncExternalStore(subscribe, getSnapshot); diff --git a/src/types.ts b/src/types.ts index 115f59ec..a00df921 100644 --- a/src/types.ts +++ b/src/types.ts @@ -432,15 +432,19 @@ export type UploadClient = { */ useRequestsWithProgress: (filter?: RequestsFilter) => readonly RequestRow[]; /** - * Calls `listener` with `getCachedRequests()` each time a row appears, - * changes state, or goes. Keeps the cache up to date while subscribed. + * Calls `listener` with `getCachedRequests()` each time those rows change: + * a row appears, changes state, or goes. There is no call at registration; + * read `getCachedRequests()` after this returns. Keeps the cache up to date + * while subscribed. */ addRequestsListener: (listener: (rows: readonly RequestRow[]) => void) => { remove: () => void; }; /** * The rows the hooks see, for code outside React. With no listener and no - * mounted hook, it reads native first. + * mounted hook, it reads native and returns a new array each call. A result + * filtered by `key` or `id` is a new array each call, so it must not be a + * `useSyncExternalStore` snapshot. */ getCachedRequests: (filter?: RequestsFilter) => readonly RequestRow[]; takeDiagnostics: () => Promise; From 106c251234b415482558c0dd033e070767dad35a Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 15:03:40 -0500 Subject: [PATCH 07/11] Guard the row cache's native callbacks and drop it while idle - The cache's state and progress handlers run inside native's event callbacks. A throw there now goes to the warning sink instead of escaping and stopping the app's later listeners. - With nothing listening, an ack or a cancel() drops the cached rows instead of patching rows that may be out of date. No native read runs in the delivery lane while the journal drains at launch; the next subscribe or read reads native. - Comments in delivery's ack() and the fake's ackEvents() name the one promise turn that settle() counts on. - CHANGELOG: Defined and UploadClient gained required members, which breaks hand-built test doubles of those types. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 ++- src/__tests__/requests.test.ts | 69 ++++++++++++++++++++++++++++++++++ src/delivery.ts | 4 ++ src/requests.ts | 39 ++++++++++++++++++- src/testing.ts | 6 ++- 5 files changed, 119 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b67bf7df..9e7a4e5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,10 @@ rows are left out by default. `addRequestsListener()` and included until their outcome is acknowledged. An app that keeps its own copy of `getRequests()` for display, and wraps `cancel()` to update it, can remove that code. Delivery, acks and cancel are unchanged. `define()` -now returns the two hooks with `key` and `mutate`. +now returns the two hooks with `key` and `mutate`. The `Defined` and +`UploadClient` types gained the new functions as required members, so a +test double built by hand with one of those types no longer compiles until +it adds them. `createFakeNative().settle()` now resolves one promise turn later, after the client has handled the ack, so the row hooks are up to date when it resolves. diff --git a/src/__tests__/requests.test.ts b/src/__tests__/requests.test.ts index 46d075db..36e82042 100644 --- a/src/__tests__/requests.test.ts +++ b/src/__tests__/requests.test.ts @@ -642,3 +642,72 @@ describe('jsonEqual', () => { ); }); }); + +describe('guards', () => { + it('warns instead of throwing out of a native state callback', () => { + const native = createFakeNative(); + const client = createUploadClient({ native }); + native.seedRows([row()]); + const warn = jest.fn(); + const cache = createRequestCache({ + readNative: () => native.getRequests() as RequestRow[], + addListener: client.addListener, + warn, + }); + // The cache's listener is registered first, so a throw in it would stop + // the app's listener below. + cache.subscribe(() => { + throw new Error('boom'); + }); + cache.subscribeWithProgress(() => { + throw new Error('boom'); + }); + const appListener = jest.fn(); + client.addListener('state', appListener); + client.addListener('progress', appListener); + expect(() => + native.emit.state(row({ state: 'running', updatedAt: 2 })), + ).not.toThrow(); + expect(() => + native.emit.progress({ id: 'a', bytesSent: 1, totalBytes: 100 }), + ).not.toThrow(); + expect(appListener).toHaveBeenCalledTimes(2); + expect(warn).toHaveBeenCalledWith( + expect.stringMatching(/from a state event threw/), + expect.any(Error), + ); + expect(warn).toHaveBeenCalledWith( + expect.stringMatching(/from a progress event threw/), + expect.any(Error), + ); + }); +}); + +describe('with nothing listening', () => { + it('drops the rows at an ack instead of reading native', async () => { + const { native, client, ping, getRequests } = setup(); + const { id } = await ping.mutate({ n: 1 }); + // A read with no listener fills the cache, then nobody listens. + client.getCachedRequests(); + native.seedRows([row({ id, key: 'ping', state: 'running' })]); + getRequests.mockClear(); + // The cached row is queued, the outcome completed: with a listener this + // would read native. + await native.settle(id, { kind: 'completed' }); + expect(getRequests).not.toHaveBeenCalled(); + // The next read takes native's rows. + expect(client.getCachedRequests()).toEqual([]); + }); + + it('drops the rows at cancel() too, and the next subscribe reads native', async () => { + const { native, client, getRequests } = setup(); + native.seedRows([row({ id: 'e', state: 'error' })]); + client.getCachedRequests(); + getRequests.mockClear(); + await client.cancel('e'); + expect(getRequests).not.toHaveBeenCalled(); + listen(client); + expect(getRequests).toHaveBeenCalledTimes(1); + expect(client.getCachedRequests()).toEqual([]); + }); +}); diff --git a/src/delivery.ts b/src/delivery.ts index 8c5e6c89..eda1c0cc 100644 --- a/src/delivery.ts +++ b/src/delivery.ts @@ -121,6 +121,10 @@ export const createDelivery = ({ return; } // Display only: the ack has already landed, and a throw here is caught. + // This runs one promise turn after ackEvents() resolves, with no other + // await before it. createFakeNative().settle() counts on that order to + // resolve after the row cache dropped the row; an await added above + // breaks that test helper (requests.test.ts "drops a completed row"). try { onAcked?.(event); } catch (e) { diff --git a/src/requests.ts b/src/requests.ts index 4eb97a8f..1a0389ed 100644 --- a/src/requests.ts +++ b/src/requests.ts @@ -414,8 +414,22 @@ export const createRequestCache = ({ const listening = (): boolean => rowListeners.size + progressListeners.size > 0; + // These run inside native's event callbacks. A throw there would surface + // as an uncaught error and stop the app's later listeners of that event. + const guarded = + (name: string, handle: (event: E) => void) => + (event: E): void => { + try { + handle(event); + } catch (e) { + warn(`requests: updating the rows from a ${name} event threw`, e); + } + }; + const onStateGuarded = guarded('state', onState); + const onProgressGuarded = guarded('progress', onProgress); + const start = (): void => { - stateSubscription = addListener('state', onState); + stateSubscription = addListener('state', onStateGuarded); // Always check: an event emitted between an earlier read (a render) and // this subscribe went to no listener. reconcile() keeps the same array // when nothing changed, so this adds no render. @@ -430,7 +444,7 @@ export const createRequestCache = ({ }; const startProgress = (): void => { - progressSubscription = addListener('progress', onProgress); + progressSubscription = addListener('progress', onProgressGuarded); }; const stopProgress = (): void => { @@ -499,7 +513,25 @@ export const createRequestCache = ({ return withProgress; }; + /** + * With nothing listening, the rows may already be out of date and nobody + * shows them. Drop them instead of patching them: the next subscribe or + * read reads native. This keeps native reads out of the delivery lane + * while the journal drains at launch. + */ + const dropWhenIdle = (): boolean => { + if (listening()) { + return false; + } + rows = undefined; + withProgress = undefined; + return true; + }; + const acked = ({ id, kind }: { id: string; kind: string }): void => { + if (dropWhenIdle()) { + return; + } const row = rows ? indexById(rows).get(id) : undefined; if (!row) { return; @@ -528,6 +560,9 @@ export const createRequestCache = ({ }; const cancelled = (id: string): void => { + if (dropWhenIdle()) { + return; + } // A settled entry is forgotten at once, with no event. A live one // settles 'cancelled' with an event, and its ack forgets it. const row = rows ? indexById(rows).get(id) : undefined; diff --git a/src/testing.ts b/src/testing.ts index 0502de5c..603dc739 100644 --- a/src/testing.ts +++ b/src/testing.ts @@ -587,7 +587,11 @@ export const createFakeNative = ( // Waiters resolve after the journal and rows are up to date, and one // promise turn after the caller's own continuation on this promise. // So when settle() resolves, the client has handled the ack too, for - // example dropped the row from its row cache. + // example dropped the row from its row cache. This depends on + // delivery's ack() handling the ack in the turn after ackEvents() + // resolves (see the comment there). A macrotask would not depend on it, + // but it would make settle() wait on timers, which Jest fake timers + // hold. void acked .then(() => undefined) .then(() => { From 0b56d78fa01bb90d19bf510ae677a4205d388c19 Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 15:04:05 -0500 Subject: [PATCH 08/11] Make held enqueues block later ones in the fake, and validate configure() first - createFakeNative(): while an enqueue is held, later enqueue calls wait behind it and run in order on release, as native runs enqueue one call at a time on both platforms. reset() drops the waiting calls too. - configure() checks every option before it stores any, so an invalid option no longer leaves the configuration half-applied. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 3 +- README.md | 6 ++- src/__tests__/client.test.ts | 32 ++++++++++++++ src/__tests__/testing.test.ts | 35 ++++++++++++--- src/index.ts | 8 ++-- src/testing.ts | 80 +++++++++++++++++++++++------------ 6 files changed, 125 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1753a72b..93955b51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,8 @@ a warning whose sink throws. Added `holdNext('enqueue')` to `createFakeNative()`. The next enqueue stays pending until the returned function is called, so a test can model a native -that is slow to answer. +that is slow to answer. Later enqueues wait behind it, as they do on a +device. iOS no longer deletes a chunked upload's file when `enqueue` cannot save the entry, for example on a full disk. The library had already moved the diff --git a/README.md b/README.md index e92a9055..dafe83ad 100644 --- a/README.md +++ b/README.md @@ -598,8 +598,10 @@ re-emit) or the header merge of `updateHeaders()`; script those with `data` parsed back from JSON. `failNext()` makes the next native call reject with a code such as `E_STORAGE`. `holdNext('enqueue')` keeps the next enqueue pending, as when native is slow to answer, and returns a function that -releases it. The released call then runs as usual, or rejects with a failure -that `failNext()` queued. `reset()` drops held calls. `seedUnacknowledged()` and `seedRows()` +releases it. Native runs enqueue calls one at a time, so later enqueues wait +behind a held one, as they do on a device. On release they all run in order, +as usual, and a failure that `failNext()` queued goes to the first call that +runs. `reset()` drops held calls and the calls behind them. `seedUnacknowledged()` and `seedRows()` model a journal and rows left by a dead session, so `configure()` replays them. diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index 9606758d..c0a761a7 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -195,6 +195,38 @@ describe('configure', () => { ); }); + it('leaves the earlier settings whole when a later option is invalid', async () => { + const client = createUploadClient(); + const send = client.define({ + key: 'k', + request: (_v: { s: string }) => ({ url: 'https://x', data: 1 }), + }); + client.configure({ + lifetimeMs: 1000, + maxVarsBytes: 16, + headers: () => ({ A: '1' }), + }); + expect(() => + client.configure({ + lifetimeMs: 2000, + enqueueTimeoutMs: 50, + maxVarsBytes: 64, + headers: () => ({ B: '2' }), + onWarning: 'log' as unknown as () => void, + }), + ).toThrow(/onWarning/); + expect(native.configure).toHaveBeenCalledTimes(1); + const before = Date.now(); + await send.mutate({ s: 'a'.repeat(8) }); + const entry = native.enqueue.mock.calls.at(-1)![0]; + expect(entry.descriptor.headers).toEqual({ A: '1' }); + expect(entry.descriptor.expiresAt).toBeGreaterThanOrEqual(before + 1000); + expect(entry.descriptor.expiresAt).toBeLessThan(before + 2000); + await expect(send.mutate({ s: 'a'.repeat(9) })).rejects.toThrow( + /the limit is 16/, + ); + }); + it('applies maxVarsBytes to later mutates', async () => { const client = createUploadClient(); const send = client.define({ diff --git a/src/__tests__/testing.test.ts b/src/__tests__/testing.test.ts index d3313174..2e63a5c3 100644 --- a/src/__tests__/testing.test.ts +++ b/src/__tests__/testing.test.ts @@ -531,6 +531,26 @@ describe('holdNext', () => { await expect(fake.enqueue(entry({ id: 'b' }))).resolves.toBe('b'); }); + it('makes later calls wait behind a held call, as native runs enqueue one at a time', async () => { + const fake = createFakeNative(); + const release = fake.holdNext('enqueue'); + const first = fake.enqueue(entry({ id: 'a' })); + const second = fake.enqueue(entry({ id: 'b' })); + fake.failNext('enqueue', 'E_STORAGE'); + const third = fake.enqueue(entry({ id: 'c' })); + await flush(); + expect(await peek(second)).toBe('pending'); + expect(await peek(third)).toBe('pending'); + expect(fake.entries).toEqual([]); + release(); + // The held call runs first, so it takes the queued failure. + await expect(first).rejects.toMatchObject({ code: 'E_STORAGE' }); + await expect(second).resolves.toBe('b'); + await expect(third).resolves.toBe('c'); + expect(fake.entries.map((e) => e.id)).toEqual(['b', 'c']); + await expect(fake.enqueue(entry({ id: 'd' }))).resolves.toBe('d'); + }); + it('applies a failNext failure when the held call is released', async () => { const fake = createFakeNative(); const release = fake.holdNext('enqueue'); @@ -544,19 +564,22 @@ describe('holdNext', () => { expect(fake.entries).toEqual([]); }); - it('uses holds in order and ignores a second release', async () => { + it('uses holds in order, keeps call order, and ignores a second release', async () => { const fake = createFakeNative(); const releaseFirst = fake.holdNext('enqueue'); const releaseSecond = fake.holdNext('enqueue'); const first = fake.enqueue(entry({ id: 'a' })); const second = fake.enqueue(entry({ id: 'b' })); + // The second call is released but still waits behind the first. releaseSecond(); - await expect(second).resolves.toBe('b'); + await flush(); + expect(await peek(second)).toBe('pending'); expect(await peek(first)).toBe('pending'); releaseFirst(); releaseFirst(); await expect(first).resolves.toBe('a'); - expect(fake.entries.map((e) => e.id)).toEqual(['b', 'a']); + await expect(second).resolves.toBe('b'); + expect(fake.entries.map((e) => e.id)).toEqual(['a', 'b']); }); it('cancels the hold when released before the call is made', async () => { @@ -566,16 +589,18 @@ describe('holdNext', () => { await expect(fake.enqueue(entry())).resolves.toBe('a'); }); - it('drops held calls and unused holds on reset()', async () => { + it('drops held calls, the calls behind them, and unused holds on reset()', async () => { const fake = createFakeNative(); const releaseHeld = fake.holdNext('enqueue'); - const releaseUnused = fake.holdNext('enqueue'); const pending = fake.enqueue(entry()); + const behind = fake.enqueue(entry({ id: 'behind' })); + const releaseUnused = fake.holdNext('enqueue'); fake.reset(); releaseHeld(); releaseUnused(); await flush(); expect(await peek(pending)).toBe('pending'); + expect(await peek(behind)).toBe('pending'); expect(fake.entries).toEqual([]); await expect(fake.enqueue(entry({ id: 'b' }))).resolves.toBe('b'); }); diff --git a/src/index.ts b/src/index.ts index e61406ef..0e7bc5ce 100644 --- a/src/index.ts +++ b/src/index.ts @@ -154,7 +154,6 @@ export const createUploadClient = ({ `configure: lifetimeMs must be a positive number, got ${options.lifetimeMs}`, ); } - settings.lifetimeMs = lifetimeMs; const enqueueTimeoutMs = options.enqueueTimeoutMs ?? DEFAULT_ENQUEUE_TIMEOUT_MS; if (!Number.isFinite(enqueueTimeoutMs) || enqueueTimeoutMs <= 0) { @@ -162,14 +161,12 @@ export const createUploadClient = ({ `configure: enqueueTimeoutMs must be a positive number, got ${options.enqueueTimeoutMs}`, ); } - settings.enqueueTimeoutMs = enqueueTimeoutMs; const maxVarsBytes = options.maxVarsBytes ?? MAX_VARS_BYTES; if (!Number.isFinite(maxVarsBytes) || maxVarsBytes <= 0) { throw new Error( `configure: maxVarsBytes must be a positive number, got ${options.maxVarsBytes}`, ); } - settings.maxVarsBytes = maxVarsBytes; if ( options.onWarning !== undefined && typeof options.onWarning !== 'function' @@ -178,6 +175,11 @@ export const createUploadClient = ({ `configure: onWarning must be a function when given, got ${typeof options.onWarning}`, ); } + // Every option is checked before any is stored, so a bad option leaves + // the earlier configuration whole. + settings.lifetimeMs = lifetimeMs; + settings.enqueueTimeoutMs = enqueueTimeoutMs; + settings.maxVarsBytes = maxVarsBytes; settings.headers = options.headers; settings.retry = options.retry; settings.onWarning = options.onWarning; diff --git a/src/testing.ts b/src/testing.ts index 176a61f0..beddde65 100644 --- a/src/testing.ts +++ b/src/testing.ts @@ -168,17 +168,20 @@ export type FakeNative = Spec & { failNext: (method: FailableMethod, code: string, message?: string) => void; /** * The next call of `method` stays pending, as when native is slow to - * answer. Call the returned function to release it: the call then runs as - * usual. It records the entry, emits its row and resolves, or rejects with - * a failure queued by failNext(). A failure is taken when the call is - * released, not when it is made. Releasing before the call is made cancels - * the hold. A second release does nothing. Holds are used in order. + * answer. Native runs enqueue calls one at a time, so later calls wait + * behind a held one. Call the returned function to release it: the held + * call and the calls behind it then run in order, as usual. Each records + * its entry, emits its row and resolves, or rejects with a failure queued + * by failNext(). A failure is taken when the call runs, not when it is + * made. Releasing before the call is made cancels the hold. A second + * release does nothing. Holds are used in order. */ holdNext: (method: HoldableMethod) => () => void; /** * Clears entries, rows, journal, diagnostics, calls, acks, the pause state, queued - * failures and holds. A held call that was not released stays pending - * forever, and its release function does nothing. A + * failures and holds. A held call that was not released, and every call + * behind it, stays pending forever, and the release function does + * nothing. A * settle() that still waits for its ack resolves now, so it cannot reject * later inside another test. Keeps the subscriptions, because a client subscribes one time, at its first * configure(). New eventIds never repeat old ones, so delivery's dedupe @@ -283,10 +286,18 @@ export const createFakeNative = ( const isPaused = (key: string): boolean => pausedAll || pausedKeys.has(key); const failures = new Map(); - // A hold waits for its call in `holds`, then for its release in `held`. - type Hold = { run?: () => void; done: boolean }; + // Native runs enqueue calls one at a time. A hold waits for its call in + // `holds`. Calls wait in `waiting`, in order, while a held call at the + // head has not been released. + type Hold = { released: boolean; dropped: boolean }; + type WaitingCall = { + input: object; + hold?: Hold; + resolve: (id: string) => void; + reject: (reason: unknown) => void; + }; const holds: Hold[] = []; - const held = new Set(); + const waiting: WaitingCall[] = []; const ackWaiters = new Map void>(); const state = makeEmitter(); @@ -447,6 +458,18 @@ export const createFakeNative = ( return raw.id; }); + /** Runs waiting calls in order, up to a held call not yet released. */ + const drainWaiting = (): void => { + for (;;) { + const next = waiting[0]; + if (!next || (next.hold && !next.hold.released)) { + return; + } + waiting.shift(); + enqueueBody(next.input).then(next.resolve, next.reject); + } + }; + const fake: FakeNative = { entries, calls, @@ -457,16 +480,13 @@ export const createFakeNative = ( }, enqueue: (input) => { - const body = () => enqueueBody(input); const hold = holds.shift(); - if (!hold) { - return body(); + if (!hold && waiting.length === 0) { + return enqueueBody(input); } - held.add(hold); return new Promise((resolve, reject) => { - hold.run = () => { - body().then(resolve, reject); - }; + waiting.push({ input, hold, resolve, reject }); + drainWaiting(); }); }, @@ -638,19 +658,18 @@ export const createFakeNative = ( if (method !== 'enqueue') { throw new Error(`holdNext: cannot hold ${String(method)}`); } - const hold: Hold = { done: false }; + const hold: Hold = { released: false, dropped: false }; holds.push(hold); return () => { - if (hold.done) { + if (hold.released || hold.dropped) { return; } - hold.done = true; - const waiting = holds.indexOf(hold); - if (waiting >= 0) { - holds.splice(waiting, 1); + hold.released = true; + const unused = holds.indexOf(hold); + if (unused >= 0) { + holds.splice(unused, 1); } - held.delete(hold); - hold.run?.(); + drainWaiting(); }; }, @@ -669,11 +688,16 @@ export const createFakeNative = ( calls.setWifiOnly.length = 0; calls.updateHeaders.length = 0; failures.clear(); - [...holds, ...held].forEach((hold) => { - hold.done = true; + holds.forEach((hold) => { + hold.dropped = true; + }); + waiting.forEach(({ hold }) => { + if (hold) { + hold.dropped = true; + } }); holds.length = 0; - held.clear(); + waiting.length = 0; // Each waiter clears its own timer and resolves its settle(). ackWaiters.forEach((waiter) => waiter()); ackWaiters.clear(); From 600bfe751719c3c6f9fcc2f048d44cf3fdb28e83 Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Tue, 10 Nov 2026 15:50:27 -0500 Subject: [PATCH 09/11] Say that onWarning gets the JavaScript warnings only Native warnings go to takeDiagnostics(), never to onWarning. The README, the type doc and the CHANGELOG now say so. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 11 ++++++----- README.md | 2 +- src/__tests__/client.test.ts | 2 +- src/index.ts | 7 ++++--- src/types.ts | 8 +++++--- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93955b51..b6de7f3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,11 +16,12 @@ fires. If native never answers, which is a native bug, `mutate()` stays pending and the warning is the only signal. Do not await `mutate()` where a wait blocks other work. -Added `configure({ onWarning })`. When set, every library warning goes to it -instead of `console.warn`, with the error or event as a second argument. An -app can send the slow-enqueue warning, and the others, to its error tracker. -Warnings raised before `configure()` still go to `console.warn`, and so does -a warning whose sink throws. +Added `configure({ onWarning })`. When set, every warning that the library's +JavaScript raises goes to it instead of `console.warn`, with the error or +event as a second argument. An app can send the slow-enqueue warning, and +the others, to its error tracker. Warnings raised before `configure()` still +go to `console.warn`, and so does a warning whose sink throws. Native +warnings do not go to `onWarning`; read them with `takeDiagnostics()`. Added `holdNext('enqueue')` to `createFakeNative()`. The next enqueue stays pending until the returned function is called, so a test can model a native diff --git a/README.md b/README.md index dafe83ad..e96c992a 100644 --- a/README.md +++ b/README.md @@ -394,7 +394,7 @@ outcomes. A second call updates the settings and does not replay again. | `retry` | `{ backoff?: { baseMs, maxMs, jitter }, terminalHttp?: { exempt } }`. Each of the two objects is optional, but one you give must be complete. Defaults 1 s, 2 h, 0.2, `[404]`. | | `headers` | `() => Record`, called at `mutate()`. The descriptor merges over it. | | `enqueueTimeoutMs` | Default 10 s. `mutate()` warns one time when native enqueue has not answered by then, and keeps waiting. It does not make `mutate()` reject. Enqueue includes the time to stage a copy of a `file` body and of form `path` parts, and it waits behind earlier enqueues, so a large file takes longer. | -| `onWarning` | `(message, detail?) => void`. Receives every library warning instead of `console.warn`: a slow enqueue, a slow or rejected handler, a failed ack, and the rest. `detail` is the error or event, when there is one. Warnings raised before `configure()` go to `console.warn`. If the sink throws, the warning and the throw go to `console.warn`. | +| `onWarning` | `(message, detail?) => void`. Receives every warning that the library's JavaScript raises, instead of `console.warn`: a slow enqueue, a slow or rejected handler, a failed ack, and the rest. Native warnings do not come here; read them with `takeDiagnostics()`. `detail` is the error or event, when there is one. Warnings raised before `configure()` go to `console.warn`. If the sink throws, the warning and the throw go to `console.warn`. | | `android` | Notification text and identity: `notificationId/Title/TitleNoWifi/TitleNoInternet/Channel`. Persisted natively. | ### `pause(scope?): Promise` and `resume(scope?): Promise` diff --git a/src/__tests__/client.test.ts b/src/__tests__/client.test.ts index c0a761a7..303d0d35 100644 --- a/src/__tests__/client.test.ts +++ b/src/__tests__/client.test.ts @@ -743,7 +743,7 @@ describe('onWarning', () => { }); afterEach(() => consoleWarn.mockRestore()); - it('receives every library warning, with its detail, instead of console.warn', async () => { + it('receives every JS warning, with its detail, instead of console.warn', async () => { const onWarning = jest.fn(); const { fake, client, failing } = setupClient(onWarning); await flush(); diff --git a/src/index.ts b/src/index.ts index 0e7bc5ce..c852de77 100644 --- a/src/index.ts +++ b/src/index.ts @@ -93,9 +93,10 @@ export const createUploadClient = ({ // registered twice is removed one subscription at a time. const stateListeners = new Set<{ listener: (event: StateEvent) => void }>(); - // Every library warning goes through here. The registry and delivery - // capture this function when the client is built, before configure(), so - // it reads the sink at each call: a sink set later is used from then on. + // Every warning the JS layer raises goes through here. The registry and + // delivery capture this function when the client is built, before + // configure(), so it reads the sink at each call: a sink set later is used + // from then on. Native warnings go to takeDiagnostics() instead. const warn = (message: string, detail?: unknown): void => { const sink = settings.onWarning; if (sink) { diff --git a/src/types.ts b/src/types.ts index 83c74d4c..dd173041 100644 --- a/src/types.ts +++ b/src/types.ts @@ -340,9 +340,11 @@ export type ConfigureOptions = { */ enqueueTimeoutMs?: number; /** - * Receives every library warning instead of `console.warn`: a slow native - * enqueue, a slow or rejected handler, a failed ack, and the rest. - * `detail` is the error or event the warning is about, when there is one. + * Receives every warning that the library's JavaScript raises, instead of + * `console.warn`: a slow native enqueue, a slow or rejected handler, a + * failed ack, and the rest. Native warnings do not come here; read them + * with `takeDiagnostics()`. `detail` is the error or event the warning is + * about, when there is one. * Warnings raised before `configure()` go to `console.warn`. If the sink * throws, the warning and the throw go to `console.warn`. */ From 51958c312b3f834c2b00ac2f0feedc7295a8f4c4 Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Fri, 13 Nov 2026 13:06:15 -0500 Subject: [PATCH 10/11] Row cache: do not add back a row on a late state event On Android a run emits its outcome before its last state event. When the ack of the outcome forgot the entry before that state event reached JS, the cache removed the row at the ack, then the late `completed` event added it back, and it stayed until the next launch. The cache now keeps the ids it removed because native forgot them (up to 256, oldest out first). A state event for one of these ids is checked against native: the row comes back only when native holds it, for example after a same-id mutate(). An id leaves the set when a native read shows a row for it. The first event of a new id is applied as before, with no read. The same applies to an ack while nothing listens, and to cancel(): a row still shown live at cancel() is now read from native, because native may have settled it already and the cancel forgot it. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 4 +- README.md | 8 +- src/__tests__/requests.test.ts | 168 +++++++++++++++++++++++++++++++++ src/requests.ts | 95 +++++++++++++++---- 4 files changed, 256 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 757ad1b8..8e33f9e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,9 @@ return one definition's rows with typed `vars`. The client keeps one copy of the rows in memory, read from native when the first hook mounts and updated by each `state` event. It also removes a row when native forgets it with no event: after the ack of a `completed` or `cancelled` outcome, and after -`cancel()` on a settled entry. The filter takes `includeCancelled`; cancelled +`cancel()` on a settled entry. A `state` event that arrives after the copy +removed its row is checked against native, so it does not add a finished row +back. The filter takes `includeCancelled`; cancelled rows are left out by default. `addRequestsListener()` and `getCachedRequests()` give the same rows outside React. Completed rows are included until their outcome is acknowledged. An app that keeps its diff --git a/README.md b/README.md index daaf7ccb..caee07c2 100644 --- a/README.md +++ b/README.md @@ -533,7 +533,13 @@ on a settled entry. An `error` row stays until `cancel()` or a same-id `mutate()`, as in native. A same-id `mutate()` emits a `state` event, so it needs nothing extra. iOS can settle a live row during an ack with no event; after any ack whose row the copy does not show in the outcome's state, the -library reads that row from native. +library reads that row from native. It does the same after `cancel(id)` on a +row the copy shows live. Events are asynchronous, so a `state` event can +arrive after the library removed its row (Android emits a run's outcome +before its last `state` event). The library keeps the last 256 ids it removed +this way, and checks a `state` event for one of them against native instead +of adding the row back. A same-id `mutate()` of such an id costs one native +read. Limits. While no hook or listener is mounted, the copy is not updated. Each time the first hook or listener subscribes, the library checks the copy diff --git a/src/__tests__/requests.test.ts b/src/__tests__/requests.test.ts index 36e82042..85aa1151 100644 --- a/src/__tests__/requests.test.ts +++ b/src/__tests__/requests.test.ts @@ -14,6 +14,7 @@ import { createRequestCache, createSelector, jsonEqual, + MAX_FORGOTTEN, rowEqual, sharedSelector, } from '../requests'; @@ -362,6 +363,173 @@ describe('the ack of an outcome whose row moved on', () => { }); }); +describe('a state event that arrives after the cache removed its row', () => { + // Android emits a run's outcome, then its last state event. The ack of the + // outcome can forget the row and resolve before that state event reaches + // JS. This stub drives the events by hand to get that order. + const stub = (nativeRows: RequestRow[]) => { + let onState: ((e: unknown) => void) | undefined; + const readNative = jest.fn(() => nativeRows.map((r) => ({ ...r }))); + const cache = createRequestCache({ + readNative, + addListener: ((event: string, listener: (e: never) => void) => { + if (event === 'state') { + onState = listener as (e: unknown) => void; + } + return { + remove: () => { + if (event === 'state') { + onState = undefined; + } + }, + }; + }) as never, + warn: jest.fn(), + }); + const emitState = (r: RequestRow) => onState?.(r); + return { cache, readNative, emitState }; + }; + + /** Ack while the row is running and native has forgotten it. */ + const ackedWhileRunning = () => { + const nativeRows = [row({ id: 'a', state: 'running', updatedAt: 2 })]; + const s = stub(nativeRows); + const listener = jest.fn(); + const subscription = s.cache.subscribe(listener); + nativeRows.length = 0; + s.cache.acked({ id: 'a', kind: 'completed' }); + expect(s.cache.rows()).toEqual([]); + expect(listener).toHaveBeenCalledTimes(1); + listener.mockClear(); + s.readNative.mockClear(); + return { ...s, nativeRows, listener, subscription }; + }; + + it('does not bring the row back on a late completed event', () => { + const { cache, emitState, listener, readNative } = ackedWhileRunning(); + emitState(row({ id: 'a', state: 'completed', updatedAt: 3 })); + expect(cache.rows()).toEqual([]); + expect(listener).not.toHaveBeenCalled(); + // It asked native once instead of trusting the event. + expect(readNative).toHaveBeenCalledTimes(1); + }); + + it('does not bring the row back on a late running event, then completed', () => { + const { cache, emitState, listener } = ackedWhileRunning(); + emitState(row({ id: 'a', state: 'running', updatedAt: 2, bytesSent: 50 })); + emitState(row({ id: 'a', state: 'completed', updatedAt: 3 })); + expect(cache.rows()).toEqual([]); + expect(listener).not.toHaveBeenCalled(); + }); + + it('shows the row of a same-id mutate() after the ack', () => { + const { cache, emitState, listener, nativeRows, readNative } = + ackedWhileRunning(); + const reopened = row({ + id: 'a', + state: 'queued', + vars: { n: 2 }, + updatedAt: 9, + }); + nativeRows.push(reopened); + emitState(reopened); + expect(cache.rows()).toEqual([reopened]); + expect(listener).toHaveBeenCalledTimes(1); + // Native showed the row again, so its next events apply without a read. + readNative.mockClear(); + emitState( + row({ id: 'a', state: 'running', vars: { n: 2 }, updatedAt: 10 }), + ); + expect(readNative).not.toHaveBeenCalled(); + expect(cache.rows()).toEqual([ + row({ id: 'a', state: 'running', vars: { n: 2 }, updatedAt: 10 }), + ]); + }); + + it('does not ask native for the first event of a new id', () => { + const { cache, emitState, readNative } = ackedWhileRunning(); + emitState(row({ id: 'b' })); + expect(readNative).not.toHaveBeenCalled(); + expect(cache.rows()).toEqual([row({ id: 'b' })]); + }); + + it('keeps a bounded number of forgotten ids, oldest out first', () => { + const { cache, emitState, readNative } = stub([]); + cache.subscribe(() => {}); + for (let i = 0; i <= MAX_FORGOTTEN; i++) { + cache.acked({ id: `r${i}`, kind: 'completed' }); + } + readNative.mockClear(); + // r0 was pushed out by the last ack, so its event applies as a new row. + emitState(row({ id: 'r0', state: 'completed' })); + expect(readNative).not.toHaveBeenCalled(); + // r1 is still kept, so its late event is checked and dropped. + emitState(row({ id: 'r1', state: 'completed' })); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows().map((r) => r.id)).toEqual(['r0']); + }); + + it('on a full read, stops checking ids native shows and keeps the others', () => { + const { cache, emitState, readNative, nativeRows, subscription } = + ackedWhileRunning(); + cache.acked({ id: 'b', kind: 'completed' }); + // A component replaces the last listener in one commit: a full read. + subscription(); + const reopened = row({ id: 'b', state: 'queued', updatedAt: 9 }); + nativeRows.push(reopened); + cache.subscribe(() => {}); + expect(cache.rows()).toEqual([reopened]); + readNative.mockClear(); + // 'b' is shown again, so its event applies without a read. + emitState(row({ id: 'b', state: 'running', updatedAt: 10 })); + expect(readNative).not.toHaveBeenCalled(); + // The late event of 'a', queued before the new subscribe, is checked. + emitState(row({ id: 'a', state: 'completed', updatedAt: 3 })); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows().map((r) => r.id)).toEqual(['b']); + }); + + it('does not bring the row back after an ack while nothing listened', () => { + const nativeRows = [row({ id: 'a', state: 'running' })]; + const { cache, emitState } = stub(nativeRows); + const first = cache.subscribe(() => {}); + first(); + nativeRows.length = 0; + cache.acked({ id: 'a', kind: 'completed' }); + cache.subscribe(() => {}); + emitState(row({ id: 'a', state: 'completed', updatedAt: 3 })); + expect(cache.rows()).toEqual([]); + }); + + it('does not bring back a row that cancel() forgot while it was shown live', () => { + // Native settled the row as an error, then cancel() forgot it before + // the error state event reached JS. + const nativeRows = [row({ id: 'a', state: 'running' })]; + const { cache, emitState, readNative } = stub(nativeRows); + cache.subscribe(() => {}); + nativeRows.length = 0; + readNative.mockClear(); + cache.cancelled('a'); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows()).toEqual([]); + emitState(row({ id: 'a', state: 'error', updatedAt: 3 })); + expect(cache.rows()).toEqual([]); + }); + + it('keeps a live row that cancel() moved to cancelled until its ack', () => { + const nativeRows = [row({ id: 'a', state: 'running' })]; + const { cache, emitState } = stub(nativeRows); + cache.subscribe(() => {}); + nativeRows[0] = row({ id: 'a', state: 'cancelled', updatedAt: 3 }); + cache.cancelled('a'); + expect(cache.rows()).toEqual([nativeRows[0]]); + emitState(row({ id: 'a', state: 'cancelled', updatedAt: 3 })); + nativeRows.length = 0; + cache.acked({ id: 'a', kind: 'cancelled' }); + expect(cache.rows()).toEqual([]); + }); +}); + describe('unsubscribe and subscribe again', () => { it('keeps the rows, then checks them against native', async () => { const { native, client, getRequests } = setup(); diff --git a/src/requests.ts b/src/requests.ts index 1a0389ed..827dd2ba 100644 --- a/src/requests.ts +++ b/src/requests.ts @@ -6,6 +6,12 @@ * row with no event, because its own JS makes those calls: the ack of a * completed or cancelled outcome, and cancel() on a settled entry. * + * Events are asynchronous, so a state event can arrive after the cache + * removed its row: Android emits a run's outcome before its last state event, + * and the ack of that outcome can forget the row in between. The ids the + * cache removed this way are kept, and their state events are checked + * against native instead of being applied. + * * Nothing here sends, acknowledges, or cancels anything. A failure here can * only make a row show late or stay longer. */ @@ -28,6 +34,13 @@ const LIVE_STATES: readonly RequestState[] = [ 'paused', ]; +/** + * The most ids the cache keeps as forgotten. A late state event comes within + * moments of the ack, so only the newest ids matter; a burst of acks at + * launch can be a few hundred. + */ +export const MAX_FORGOTTEN = 256; + /** * Deep equality for JSON values, as `vars` and rows hold them. A field set to * `undefined` counts as absent. It allocates nothing, because it runs on @@ -266,11 +279,37 @@ export const createRequestCache = ({ let stateSubscription: { remove: () => void } | undefined; // Native progress is listened to only while a progress view is mounted. let progressSubscription: { remove: () => void } | undefined; + // Ids whose row native forgot, or may have forgotten, with no event. A + // state event for one of them can be late, so it is checked against + // native. Oldest first; an id leaves when native shows a row for it again. + const forgotten = new Set(); const notify = (listeners: Set): void => { listeners.forEach((listener) => listener()); }; + const markForgotten = (id: string): void => { + // Delete first, so a repeated id moves to the newest place. + forgotten.delete(id); + forgotten.add(id); + if (forgotten.size > MAX_FORGOTTEN) { + const oldest = forgotten.values().next(); + if (!oldest.done) { + forgotten.delete(oldest.value); + } + } + }; + + /** After a read of every native row: an id native shows is not forgotten. */ + const unforgetShown = (current: readonly RequestRow[]): void => { + if (forgotten.size === 0) { + return; + } + for (const row of current) { + forgotten.delete(row.id); + } + }; + /** True when the row had progress to forget. */ const forgetProgress = (id: string): boolean => { overlaid.delete(id); @@ -299,7 +338,9 @@ export const createRequestCache = ({ /** Native's rows, or undefined when the read throws. */ const readSafely = (): RequestRow[] | undefined => { try { - return readNative(); + const current = readNative(); + unforgetShown(current); + return current; } catch (e) { warn( 'requests: getRequests failed, so the cached rows were not checked against native', @@ -352,6 +393,12 @@ export const createRequestCache = ({ } }; + /** Removes a row that native forgot, and keeps its id for late events. */ + const forget = (id: string): void => { + markForgotten(id); + remove(id); + }; + /** Puts `row` in the cache, in place of the row with its id. */ const upsert = (row: RequestRow): void => { const hadProgress = forgetProgress(row.id); @@ -384,16 +431,18 @@ export const createRequestCache = ({ if (row) { upsert(row); } else { - remove(id); + forget(id); } }; // A state event carries the full row, so it replaces the cached one. Its // bytes are the row's own, so the progress seen before it is out of date. const onState = (event: StateEvent): void => { - if (event.reason === 'unhandled-key') { - // JS builds this row from the journaled outcome, not from the entry. - // A same-id mutate() may have reopened the entry since, so ask native. + // An unhandled-key row: JS builds it from the journaled outcome, not + // from the entry, and a same-id mutate() may have reopened the entry + // since. A forgotten id: the event may be older than the forget, or be + // the first event of a same-id mutate(). Ask native in both cases. + if (event.reason === 'unhandled-key' || forgotten.has(event.id)) { if (rows) { refreshId(event.id); } @@ -529,45 +578,57 @@ export const createRequestCache = ({ }; const acked = ({ id, kind }: { id: string; kind: string }): void => { + // Native forgets a row at the ack of its completed or cancelled outcome. + const forgets = kind === 'completed' || kind === 'cancelled'; if (dropWhenIdle()) { + // A state event queued before the next subscribe still reaches it. + if (forgets) { + markForgotten(id); + } return; } const row = rows ? indexById(rows).get(id) : undefined; - if (!row) { - return; - } if (kind === 'error') { // Native keeps an error row. iOS can move a live row to its outcome // during the ack with no event, so a row not shown as an error is // checked. - if (row.state !== 'error') { + if (row && row.state !== 'error') { refreshId(id); } return; } - if (kind !== 'completed' && kind !== 'cancelled') { + if (!forgets) { return; } - // Native forgets a row at the ack of its completed or cancelled outcome. - if (row.state === kind) { - remove(id); + if (!row || row.state === kind) { + forget(id); return; } // The row moved on, for example a cancelled entry that a same-id // mutate() reopened, whose old ack does not forget it; or a live row that - // native settled and forgot during the ack. Ask native. + // native settled and forgot during the ack, whose last state event is + // still on its way. Ask native. refreshId(id); }; const cancelled = (id: string): void => { if (dropWhenIdle()) { + markForgotten(id); return; } // A settled entry is forgotten at once, with no event. A live one - // settles 'cancelled' with an event, and its ack forgets it. + // settles 'cancelled' with an event, and its ack forgets it. A row still + // shown live may already be settled in native, with its state event on + // the way, and then cancel() forgot it: ask native. const row = rows ? indexById(rows).get(id) : undefined; - if (row && !LIVE_STATES.includes(row.state)) { - remove(id); + if (!row) { + markForgotten(id); + return; + } + if (LIVE_STATES.includes(row.state)) { + refreshId(id); + } else { + forget(id); } }; From c4986e835500f796931152d1263a099e7a7f0974 Mon Sep 17 00:00:00 2001 From: Dylan Murphy Date: Fri, 13 Nov 2026 13:23:40 -0500 Subject: [PATCH 11/11] Row cache: keep cancelled rows native holds, and check late settled events Follow-up to the late state event fix, after review. That commit's message said the stale row stayed until the next launch. It stayed until the next first subscribe or that id's next mutate(). - cancel() on a row shown as 'cancelled' now asks native. On Android the cancelled state event arrives before cancel() resolves, and native keeps the row until the ack. The cache removed it at once before, so with includeCancelled the row went early, and stayed hidden when the ack failed. - A settled state event for an id the cache does not hold now asks native. A new life starts live, so this costs a read only for a late event. It covers an id pushed out of the 256-id set. - When that native read fails, a live event, or an event for a row already shown, is applied. A reused id is then not hidden for its whole new life. - Tests: Android's event order through the client and the fake, a failed ack of a cancel, the set overflow, failed reads, and cancel() of an id the cache did not hold. Two cancel tests now check the ack, not cancel(), as what removes the row. - README and CHANGELOG wording. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 19 +++-- README.md | 14 ++-- src/__tests__/requests.test.ts | 140 +++++++++++++++++++++++++++++++-- src/requests.ts | 63 ++++++++++----- 4 files changed, 194 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e33f9e5..1dbab6ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,16 +24,15 @@ by each `state` event. It also removes a row when native forgets it with no event: after the ack of a `completed` or `cancelled` outcome, and after `cancel()` on a settled entry. A `state` event that arrives after the copy removed its row is checked against native, so it does not add a finished row -back. The filter takes `includeCancelled`; cancelled -rows are left out by default. `addRequestsListener()` and -`getCachedRequests()` give the same rows outside React. Completed rows are -included until their outcome is acknowledged. An app that keeps its -own copy of `getRequests()` for display, and wraps `cancel()` to update it, -can remove that code. Delivery, acks and cancel are unchanged. `define()` -now returns the two hooks with `key` and `mutate`. The `Defined` and -`UploadClient` types gained the new functions as required members, so a -test double built by hand with one of those types no longer compiles until -it adds them. +back. The filter takes `includeCancelled`; cancelled rows are left out by +default. `addRequestsListener()` and `getCachedRequests()` give the same rows +outside React. Completed rows are included until their outcome is +acknowledged. An app that keeps its own copy of `getRequests()` for display, +and wraps `cancel()` to update it, can remove that code. Delivery, acks and +cancel are unchanged. `define()` now returns the two hooks with `key` and +`mutate`. The `Defined` and `UploadClient` types gained the new functions as +required members, so a test double built by hand with one of those types no +longer compiles until it adds them. `createFakeNative().settle()` now resolves one promise turn later, after the client has handled the ack, so the row hooks are up to date when it resolves. diff --git a/README.md b/README.md index caee07c2..19e4e606 100644 --- a/README.md +++ b/README.md @@ -534,12 +534,14 @@ on a settled entry. An `error` row stays until `cancel()` or a same-id needs nothing extra. iOS can settle a live row during an ack with no event; after any ack whose row the copy does not show in the outcome's state, the library reads that row from native. It does the same after `cancel(id)` on a -row the copy shows live. Events are asynchronous, so a `state` event can -arrive after the library removed its row (Android emits a run's outcome -before its last `state` event). The library keeps the last 256 ids it removed -this way, and checks a `state` event for one of them against native instead -of adding the row back. A same-id `mutate()` of such an id costs one native -read. +row the copy shows live or `cancelled`. Events and call results can reach JS +in either order, so a `state` event can arrive after the library removed its +row (Android emits a run's outcome before its last `state` event). The +library keeps the last 256 ids it removed this way, and checks a `state` +event for one of them against native instead of adding the row back. Past +256 ids the result is the same: a settled `state` event for a row the copy +does not hold is also checked, at the cost of one native read. A same-id +`mutate()` of a kept id costs one native read. Limits. While no hook or listener is mounted, the copy is not updated. Each time the first hook or listener subscribes, the library checks the copy diff --git a/src/__tests__/requests.test.ts b/src/__tests__/requests.test.ts index 85aa1151..7391541d 100644 --- a/src/__tests__/requests.test.ts +++ b/src/__tests__/requests.test.ts @@ -253,12 +253,63 @@ describe('rows native forgets with no event', () => { const { native, client, ping } = setup(); listen(client); const { id } = await ping.mutate({ n: 1 }); + // The fake, as Android, emits the cancelled state before cancel() + // resolves. cancel() itself does not drop the row. await client.cancel(id); + expect(client.getCachedRequests({ includeCancelled: true })).toEqual([ + expect.objectContaining({ id, state: 'cancelled' }), + ]); await flush(); + expect(native.ackedEventIds).toHaveLength(1); expect(native.getRequests()).toEqual([]); expect(client.getCachedRequests({ includeCancelled: true })).toEqual([]); }); + it('keeps a cancelled live row whose ack failed, as native does', async () => { + const { native, client, ping, warnings } = setup(); + listen(client); + const { id } = await ping.mutate({ n: 1 }); + native.failNext('ackEvents', 'E_STORAGE'); + await client.cancel(id); + await flush(); + expect(native.getRequests()).toEqual([ + expect.objectContaining({ id, state: 'cancelled' }), + ]); + expect(client.getCachedRequests({ includeCancelled: true })).toEqual([ + expect.objectContaining({ id, state: 'cancelled' }), + ]); + expect(warnings).toEqual([expect.stringMatching(/ackEvents failed/)]); + }); + + it('drops a row whose outcome was acked before its last state event', async () => { + // Android's order: the outcome, then the last state event. Here the + // client gets the outcome and acks it before that state event arrives. + const fake = createFakeNative(); + let release: (() => void) | undefined; + const native: FakeNative = { + ...fake, + onState: (handler) => + fake.onState((row) => { + if ((row as RequestRow).state === 'completed') { + release = () => void handler(row); + return; + } + return handler(row); + }), + }; + const { client, ping } = setup(native); + const { calls } = listen(client); + const { id } = await ping.mutate({ n: 1 }); + fake.emit.state(row({ id, key: 'ping', state: 'running', updatedAt: 2 })); + await fake.settle(id, { kind: 'completed' }); + expect(fake.getRequests()).toEqual([]); + expect(client.getCachedRequests()).toEqual([]); + const seen = calls.length; + release?.(); + expect(client.getCachedRequests()).toEqual([]); + expect(calls).toHaveLength(seen); + }); + it('drops a settled row when cancel() resolves', async () => { const { native, client } = setup(); native.seedRows([row({ id: 'e', state: 'error' })]); @@ -369,7 +420,13 @@ describe('a state event that arrives after the cache removed its row', () => { // JS. This stub drives the events by hand to get that order. const stub = (nativeRows: RequestRow[]) => { let onState: ((e: unknown) => void) | undefined; - const readNative = jest.fn(() => nativeRows.map((r) => ({ ...r }))); + let failReads = false; + const readNative = jest.fn(() => { + if (failReads) { + throw new Error('E_BOOM'); + } + return nativeRows.map((r) => ({ ...r })); + }); const cache = createRequestCache({ readNative, addListener: ((event: string, listener: (e: never) => void) => { @@ -387,7 +444,10 @@ describe('a state event that arrives after the cache removed its row', () => { warn: jest.fn(), }); const emitState = (r: RequestRow) => onState?.(r); - return { cache, readNative, emitState }; + const setFailReads = (fail: boolean) => { + failReads = fail; + }; + return { cache, readNative, emitState, setFailReads }; }; /** Ack while the row is running and native has forgotten it. */ @@ -460,15 +520,54 @@ describe('a state event that arrives after the cache removed its row', () => { cache.acked({ id: `r${i}`, kind: 'completed' }); } readNative.mockClear(); - // r0 was pushed out by the last ack, so its event applies as a new row. - emitState(row({ id: 'r0', state: 'completed' })); + // r0 was pushed out by the last ack, so a live event applies as a new + // row with no read. + emitState(row({ id: 'r0', state: 'running' })); expect(readNative).not.toHaveBeenCalled(); // r1 is still kept, so its late event is checked and dropped. - emitState(row({ id: 'r1', state: 'completed' })); + emitState(row({ id: 'r1', state: 'running' })); expect(readNative).toHaveBeenCalledTimes(1); expect(cache.rows().map((r) => r.id)).toEqual(['r0']); }); + it('does not bring back an id pushed out of the set on a late settled event', () => { + const { cache, emitState, listener, readNative } = ackedWhileRunning(); + for (let i = 0; i < MAX_FORGOTTEN; i++) { + cache.acked({ id: `o${i}`, kind: 'completed' }); + } + listener.mockClear(); + readNative.mockClear(); + // 'a' is out of the set, but a settled state for an id the cache does + // not hold is checked too. + emitState(row({ id: 'a', state: 'completed', updatedAt: 3 })); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows()).toEqual([]); + expect(listener).not.toHaveBeenCalled(); + }); + + it('shows the live events of a reused id when the native read fails', () => { + const { cache, emitState, nativeRows, setFailReads } = ackedWhileRunning(); + setFailReads(true); + nativeRows.push(row({ id: 'a', state: 'queued', updatedAt: 9 })); + emitState(row({ id: 'a', state: 'queued', updatedAt: 9 })); + expect(cache.rows()).toEqual([ + row({ id: 'a', state: 'queued', updatedAt: 9 }), + ]); + emitState(row({ id: 'a', state: 'running', updatedAt: 10 })); + // The row is shown now, so its settled state applies too. + emitState(row({ id: 'a', state: 'error', updatedAt: 11 })); + expect(cache.rows()).toEqual([ + row({ id: 'a', state: 'error', updatedAt: 11 }), + ]); + }); + + it('drops a late settled event of a forgotten id when the native read fails', () => { + const { cache, emitState, setFailReads } = ackedWhileRunning(); + setFailReads(true); + emitState(row({ id: 'a', state: 'completed', updatedAt: 3 })); + expect(cache.rows()).toEqual([]); + }); + it('on a full read, stops checking ids native shows and keeps the others', () => { const { cache, emitState, readNative, nativeRows, subscription } = ackedWhileRunning(); @@ -517,17 +616,42 @@ describe('a state event that arrives after the cache removed its row', () => { }); it('keeps a live row that cancel() moved to cancelled until its ack', () => { + // Android's order: the cancelled state event arrives before cancel() + // resolves, and native keeps the row until the ack. + const cancelledRow = row({ id: 'a', state: 'cancelled', updatedAt: 3 }); const nativeRows = [row({ id: 'a', state: 'running' })]; const { cache, emitState } = stub(nativeRows); cache.subscribe(() => {}); - nativeRows[0] = row({ id: 'a', state: 'cancelled', updatedAt: 3 }); + nativeRows[0] = cancelledRow; + emitState(cancelledRow); cache.cancelled('a'); - expect(cache.rows()).toEqual([nativeRows[0]]); - emitState(row({ id: 'a', state: 'cancelled', updatedAt: 3 })); + expect(cache.rows()).toEqual([cancelledRow]); nativeRows.length = 0; cache.acked({ id: 'a', kind: 'cancelled' }); expect(cache.rows()).toEqual([]); }); + + it('drops a cancelled row that a second cancel() forgot', () => { + const nativeRows = [row({ id: 'a', state: 'cancelled', updatedAt: 3 })]; + const { cache } = stub(nativeRows); + cache.subscribe(() => {}); + nativeRows.length = 0; + cache.cancelled('a'); + expect(cache.rows()).toEqual([]); + }); + + it('shows a same-id mutate() after cancel() of an id it did not hold', () => { + const nativeRows: RequestRow[] = []; + const { cache, emitState, readNative } = stub(nativeRows); + cache.subscribe(() => {}); + cache.cancelled('a'); + const reopened = row({ id: 'a', state: 'queued', updatedAt: 9 }); + nativeRows.push(reopened); + readNative.mockClear(); + emitState(reopened); + expect(readNative).toHaveBeenCalledTimes(1); + expect(cache.rows()).toEqual([reopened]); + }); }); describe('unsubscribe and subscribe again', () => { diff --git a/src/requests.ts b/src/requests.ts index 827dd2ba..79477070 100644 --- a/src/requests.ts +++ b/src/requests.ts @@ -35,9 +35,10 @@ const LIVE_STATES: readonly RequestState[] = [ ]; /** - * The most ids the cache keeps as forgotten. A late state event comes within - * moments of the ack, so only the newest ids matter; a burst of acks at - * launch can be a few hundred. + * The most ids the cache keeps as forgotten. Ids acked earlier have no state + * event on the way, so only the ids acked in the last moments matter, and a + * burst of a few hundred acks at launch does not need a larger bound. An id + * pushed out is still covered for settled states: see onState(). */ export const MAX_FORGOTTEN = 256; @@ -282,6 +283,10 @@ export const createRequestCache = ({ // Ids whose row native forgot, or may have forgotten, with no event. A // state event for one of them can be late, so it is checked against // native. Oldest first; an id leaves when native shows a row for it again. + // This relies on native events reaching JS in the order native sent them: + // a read can show a same-id mutate()'s new row while the old life's last + // event is still on the way, and that event then replaces the row until + // the new life's own next event corrects it. const forgotten = new Set(); const notify = (listeners: Set): void => { @@ -422,10 +427,11 @@ export const createRequestCache = ({ }; /** Asks native for one row: replaces it, or drops it when native has none. */ - const refreshId = (id: string): void => { + /** False when the native read failed and nothing changed. */ + const refreshId = (id: string): boolean => { const current = readSafely(); if (!current) { - return; + return false; } const row = current.find((candidate) => candidate.id === id); if (row) { @@ -433,21 +439,40 @@ export const createRequestCache = ({ } else { forget(id); } + return true; }; // A state event carries the full row, so it replaces the cached one. Its // bytes are the row's own, so the progress seen before it is out of date. const onState = (event: StateEvent): void => { - // An unhandled-key row: JS builds it from the journaled outcome, not - // from the entry, and a same-id mutate() may have reopened the entry - // since. A forgotten id: the event may be older than the forget, or be - // the first event of a same-id mutate(). Ask native in both cases. - if (event.reason === 'unhandled-key' || forgotten.has(event.id)) { - if (rows) { - refreshId(event.id); - } + if (!rows) { return; } + if (event.reason === 'unhandled-key') { + // JS builds this row from the journaled outcome, not from the entry. + // A same-id mutate() may have reopened the entry since, so ask native. + refreshId(event.id); + return; + } + const held = indexById(rows).has(event.id); + // A forgotten id: the event may be older than the forget, or be the + // first event of a same-id mutate(). A settled state for an id the cache + // does not hold: a new life starts live, so this is a late event too, + // for example of an id pushed out of `forgotten`. Ask native. + if ( + forgotten.has(event.id) || + (!held && !LIVE_STATES.includes(event.state)) + ) { + if (refreshId(event.id)) { + return; + } + // The read failed. A live state, or a row already shown, is most + // likely the new life of a reused id: show it rather than hide that + // life until the next read. + if (!held && !LIVE_STATES.includes(event.state)) { + return; + } + } upsert(event); }; @@ -616,16 +641,18 @@ export const createRequestCache = ({ markForgotten(id); return; } - // A settled entry is forgotten at once, with no event. A live one - // settles 'cancelled' with an event, and its ack forgets it. A row still - // shown live may already be settled in native, with its state event on - // the way, and then cancel() forgot it: ask native. + // Native forgets a completed or error entry at once, with no event. It + // moves a live entry to 'cancelled', usually with the event already + // here, and keeps it until the ack. So a 'cancelled' row may be one + // native keeps or one this cancel forgot (an entry cancelled before). A + // row still shown live may already be settled in native, with its state + // event on the way, and this cancel forgot it. Ask native for both. const row = rows ? indexById(rows).get(id) : undefined; if (!row) { markForgotten(id); return; } - if (LIVE_STATES.includes(row.state)) { + if (row.state === 'cancelled' || LIVE_STATES.includes(row.state)) { refreshId(id); } else { forget(id);