From 1e43adefb1b73452cabbc825f6737470a748b01e Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Wed, 27 May 2026 13:32:57 +0200 Subject: [PATCH 1/9] SequentialQueue: await persist in push() before flush() --- src/libs/Network/SequentialQueue.ts | 38 +++++++---------------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index ab23074658ca..7586f1473251 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -504,7 +504,7 @@ async function handleConflictActions(conflictAction: Confl } } -function push(newRequest: OnyxRequest): Promise { +async function push(newRequest: OnyxRequest): Promise { const currentRequests = getAllPersistedRequests(); Log.info('[SequentialQueue] push() called', false, { command: newRequest.command, @@ -514,19 +514,10 @@ function push(newRequest: OnyxRequest): Promise; if (newRequest.checkAndFixConflictingRequest) { - const requests = currentRequests; - Log.info('[SequentialQueue] Checking for conflicts', false, { - command: newRequest.command, - existingRequestsCount: requests.length, - }); - - const {conflictAction} = newRequest.checkAndFixConflictingRequest(requests as Array>); + const {conflictAction} = newRequest.checkAndFixConflictingRequest(currentRequests as Array>); Log.info('[SequentialQueue] Conflict action determined', false, { command: newRequest.command, conflictType: conflictAction.type, @@ -537,41 +528,30 @@ function push(newRequest: OnyxRequest): Promise { - Log.info('[SequentialQueue] isReadyPromise resolved, flushing queue', false, { - command: newRequest.command, - }); - flush(true); - }); - return persistencePromise; + isReadyPromise.then(() => flush(true)); + return; } - Log.info('[SequentialQueue] Queue is not running. Flushing the queue.', false, { - command: newRequest.command, - }); flush(true); - return persistencePromise; } function getCurrentRequest(): Promise { From d4aa74695797f63ee934be6d5d8da3b67266a216 Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Wed, 27 May 2026 13:35:42 +0200 Subject: [PATCH 2/9] SequentialQueue: restore push() flush trigger logs --- src/libs/Network/SequentialQueue.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 7586f1473251..4ad2c056d7cc 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -547,10 +547,18 @@ async function push(newRequest: OnyxRequest): Promis Log.info('[SequentialQueue] Queue is running. Will flush when the current request is finished.', false, { command: newRequest.command, }); - isReadyPromise.then(() => flush(true)); + isReadyPromise.then(() => { + Log.info('[SequentialQueue] isReadyPromise resolved, flushing queue', false, { + command: newRequest.command, + }); + flush(true); + }); return; } + Log.info('[SequentialQueue] Queue is not running. Flushing the queue.', false, { + command: newRequest.command, + }); flush(true); } From 96aceaecba1f48412ab9f1be9c34c19568319f85 Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Wed, 27 May 2026 13:52:55 +0200 Subject: [PATCH 3/9] SequentialQueue: defer flush onto persistencePromise; update tests --- src/libs/Network/SequentialQueue.ts | 30 ++-- tests/unit/SequentialQueueTest.ts | 222 ++++++++++++++-------------- 2 files changed, 132 insertions(+), 120 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 4ad2c056d7cc..503d354df5db 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -504,7 +504,7 @@ async function handleConflictActions(conflictAction: Confl } } -async function push(newRequest: OnyxRequest): Promise { +function push(newRequest: OnyxRequest): Promise { const currentRequests = getAllPersistedRequests(); Log.info('[SequentialQueue] push() called', false, { command: newRequest.command, @@ -531,35 +531,39 @@ async function push(newRequest: OnyxRequest): Promis persistencePromise = savePersistedRequest(newRequest); } - // Block until the Onyx disk commit lands so a process kill between here and the XHR - // still leaves the request recoverable from storage on next launch. - await persistencePromise; - + // If we are offline we don't need to trigger the queue to empty as it will happen when we come back online if (isOfflineNetwork()) { Log.info('[SequentialQueue] Request persisted but not flushing — we are offline', false, { command: newRequest.command, queueLength: getAllPersistedRequests().length, }); - return; + return persistencePromise; } + // Defer flush() onto persistencePromise so the XHR cannot fire before the Onyx disk + // commit lands. If the process is killed between fire and commit, the request would be + // lost from storage on next launch. The in-memory queue is already updated synchronously + // above, so callers checking getAll()/getLength() right after push() still see the new state. if (isSequentialQueueRunning) { Log.info('[SequentialQueue] Queue is running. Will flush when the current request is finished.', false, { command: newRequest.command, }); - isReadyPromise.then(() => { - Log.info('[SequentialQueue] isReadyPromise resolved, flushing queue', false, { - command: newRequest.command, + persistencePromise + .then(() => isReadyPromise) + .then(() => { + Log.info('[SequentialQueue] isReadyPromise resolved, flushing queue', false, { + command: newRequest.command, + }); + flush(true); }); - flush(true); - }); - return; + return persistencePromise; } Log.info('[SequentialQueue] Queue is not running. Flushing the queue.', false, { command: newRequest.command, }); - flush(true); + persistencePromise.then(() => flush(true)); + return persistencePromise; } function getCurrentRequest(): Promise { diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index b49478391eaa..e90d64e542bb 100644 --- a/tests/unit/SequentialQueueTest.ts +++ b/tests/unit/SequentialQueueTest.ts @@ -36,29 +36,30 @@ describe('SequentialQueue', () => { expect(getLength()).toBe(2); }); - it('should push two requests with conflict resolution and replace', () => { - SequentialQueue.push(request); - const requestWithConflictResolution: Request = { - command: 'ReconnectApp', - data: {accountID: 56789}, - checkAndFixConflictingRequest: (persistedRequests) => { - // should be one instance of ReconnectApp, get the index to replace it later - const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); - if (index === -1) { - return {conflictAction: {type: 'push'}}; - } - - return { - conflictAction: {type: 'replace', index}, - }; - }, - }; - SequentialQueue.push(requestWithConflictResolution); - expect(getLength()).toBe(1); - // We know there is only one request and it's ongoing. - // We can get it and verify that the ongoing request is the second one. - const ongoingRequest = getOngoingRequest(); - expect(ongoingRequest?.data?.accountID).toBe(56789); + it('should push two requests with conflict resolution and replace', async () => { + // Pause the queue so `process()` does not consume the first request before + // the conflict resolver runs. Under persist-before-fire `push()` is async, + // so we await both pushes and then assert on the on-disk queue directly. + SequentialQueue.pause(); + try { + await SequentialQueue.push(request); + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + return {conflictAction: {type: 'replace', index}}; + }, + }; + await SequentialQueue.push(requestWithConflictResolution); + expect(getLength()).toBe(1); + expect(getAll().at(0)?.data?.accountID).toBe(56789); + } finally { + SequentialQueue.unpause(); + } }); it('should push two requests with conflict resolution and push', () => { @@ -88,105 +89,112 @@ describe('SequentialQueue', () => { }); it('should add a new request even if a similar one is ongoing', async () => { - // .push at the end flush the queue - SequentialQueue.push(request); + // Pause fetch so the first request lands as `ongoingRequest` but never completes. + // The conflict checker on push 2 inspects the persisted queue (which excludes the + // ongoing request), so it cannot find a 'ReconnectApp' to replace and falls back + // to 'push'. The new request is therefore added to the queue. + const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; + mockedFetch.pause(); + try { + await SequentialQueue.push(request); + await waitForBatchedUpdates(); + expect(getOngoingRequest()?.command).toBe('ReconnectApp'); + + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + return {conflictAction: {type: 'replace', index}}; + }, + }; - // wait for Onyx.connect execute the callback and start processing the queue - await Promise.resolve(); + await SequentialQueue.push(requestWithConflictResolution); - const requestWithConflictResolution: Request = { - command: 'ReconnectApp', - data: {accountID: 56789}, - checkAndFixConflictingRequest: (persistedRequests) => { - // should be one instance of ReconnectApp, get the index to replace it later + // The new request is in the persisted queue with the expected accountID. + expect(getAll().some((r) => r.data?.accountID === 56789)).toBe(true); + } finally { + await mockedFetch.resume(); + } + }); + + it('should replace request request in queue while a similar one is ongoing', async () => { + const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; + mockedFetch.pause(); + try { + await SequentialQueue.push(request); + await waitForBatchedUpdates(); + expect(getOngoingRequest()?.command).toBe('ReconnectApp'); + + const conflictResolver = (persistedRequests: Array>): ConflictActionData => { const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); if (index === -1) { return {conflictAction: {type: 'push'}}; } - - return { - conflictAction: {type: 'replace', index}, - }; - }, - }; - - SequentialQueue.push(requestWithConflictResolution); - - const ongoingRequest = getOngoingRequest(); - expect(ongoingRequest?.data?.accountID).toBe(56789); - }); - - it('should replace request request in queue while a similar one is ongoing', async () => { - // .push at the end flush the queue - SequentialQueue.push(request); - - // wait for Onyx.connect execute the callback and start processing the queue - await Promise.resolve(); - - const conflictResolver = (persistedRequests: Array>): ConflictActionData => { - // should be one instance of ReconnectApp, get the index to replace it later - const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); - if (index === -1) { - return {conflictAction: {type: 'push'}}; - } - - return { - conflictAction: {type: 'replace', index}, + return {conflictAction: {type: 'replace', index}}; }; - }; - const requestWithConflictResolution: Request = { - command: 'ReconnectApp', - data: {accountID: 56789}, - checkAndFixConflictingRequest: conflictResolver, - }; + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: conflictResolver, + }; - const requestWithConflictResolution2: Request = { - command: 'ReconnectApp', - data: {accountID: 56789}, - checkAndFixConflictingRequest: conflictResolver, - }; + const requestWithConflictResolution2: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: conflictResolver, + }; - SequentialQueue.push(requestWithConflictResolution); - SequentialQueue.push(requestWithConflictResolution2); + // First conflict push: queue is empty (ongoing not in queue) → push action → queue=[r2]. + // Second conflict push: queue=[r2] → replace at 0 → queue=[r3]. + // Total in-flight items: ongoing + queue = 2. + await SequentialQueue.push(requestWithConflictResolution); + await SequentialQueue.push(requestWithConflictResolution2); - expect(getLength()).toBe(2); + expect(getLength()).toBe(2); + } finally { + await mockedFetch.resume(); + } }); - it('should replace request request in queue while a similar one is ongoing and keep the same index', () => { - SequentialQueue.push({command: 'OpenReport'}); - SequentialQueue.push(request); - - const requestWithConflictResolution: Request = { - command: 'ReconnectApp', - data: {accountID: 56789}, - checkAndFixConflictingRequest: (persistedRequests) => { - // should be one instance of ReconnectApp, get the index to replace it later - const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); - if (index === -1) { - return {conflictAction: {type: 'push'}}; - } - - return { - conflictAction: {type: 'replace', index}, - }; - }, - }; - - SequentialQueue.push(requestWithConflictResolution); - SequentialQueue.push({command: 'AddComment'}); - SequentialQueue.push({command: 'OpenReport'}); - - expect(getLength()).toBe(4); - const persistedRequests = getAll(); - const ongoingRequest = getOngoingRequest(); + it('should replace request request in queue while a similar one is ongoing and keep the same index', async () => { + const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; + mockedFetch.pause(); + try { + // First push moves into `ongoingRequest`; subsequent pushes stack in the queue. + await SequentialQueue.push({command: 'OpenReport'}); + await waitForBatchedUpdates(); + expect(getOngoingRequest()?.command).toBe('OpenReport'); + + await SequentialQueue.push(request); + + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + return {conflictAction: {type: 'replace', index}}; + }, + }; - // The first OpenReport call is ongoing - expect(ongoingRequest?.command).toBe('OpenReport'); + await SequentialQueue.push(requestWithConflictResolution); + await SequentialQueue.push({command: 'AddComment'}); + await SequentialQueue.push({command: 'OpenReport'}); - // We know ReconnectApp is at index 0 in the queue now, so we can get it to verify - // that was replaced by the new request. - expect(persistedRequests.at(0)?.data?.accountID).toBe(56789); + expect(getLength()).toBe(4); + const persistedRequests = getAll(); + expect(getOngoingRequest()?.command).toBe('OpenReport'); + expect(persistedRequests.at(0)?.data?.accountID).toBe(56789); + } finally { + await mockedFetch.resume(); + } }); // need to test a rance condition between processing the next request and then pushing a new request with conflict resolver From ce33aa97492daf99fba7771fab1d3f0223c6c8e7 Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Wed, 27 May 2026 14:46:22 +0200 Subject: [PATCH 4/9] SequentialQueue: async push() with idempotent isReadyPromise marker --- src/libs/Network/SequentialQueue.ts | 73 ++++++++++++++++------------- 1 file changed, 41 insertions(+), 32 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 503d354df5db..35e27716f8ab 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -44,13 +44,28 @@ type RequestError = Error & { status?: string; }; -let resolveIsReadyPromise: ((args?: unknown[]) => void) | undefined; -let isReadyPromise = new Promise((resolve) => { - resolveIsReadyPromise = resolve; -}); +let resolveIsReadyPromise: (() => void) | undefined; +let isReadyPromise: Promise = Promise.resolve(); +let isReadyPromisePending = false; -// Resolve the isReadyPromise immediately so that the queue starts working as soon as the page loads -resolveIsReadyPromise?.(); +/** + * Marks isReadyPromise as pending so any READ that consults waitForIdle() parks behind us. + * Idempotent: if already pending, no-op (avoids orphaning subscribers from prior pushes). + * Called from push()'s sync prelude before the first await, so READs on the next sync line + * see the pending promise. + */ +function setIsReadyPromisePending() { + if (isReadyPromisePending) { + return; + } + isReadyPromise = new Promise((resolve) => { + resolveIsReadyPromise = () => { + isReadyPromisePending = false; + resolve(); + }; + }); + isReadyPromisePending = true; +} let isSequentialQueueRunning = false; let currentRequestPromise: Promise | null = null; @@ -334,10 +349,9 @@ function flush(shouldResetPromise = true) { isSequentialQueueRunning = true; if (shouldResetPromise) { - // Reset the isReadyPromise so that the queue will be flushed as soon as the request is finished - isReadyPromise = new Promise((resolve) => { - resolveIsReadyPromise = resolve; - }); + // Mark isReadyPromise as pending so READs (waitForIdle) park behind us. + // Idempotent — safe if push() already marked it pending in its sync prelude. + setIsReadyPromisePending(); } // Ensure persistedRequests are read from storage before proceeding with the queue @@ -504,7 +518,7 @@ async function handleConflictActions(conflictAction: Confl } } -function push(newRequest: OnyxRequest): Promise { +async function push(newRequest: OnyxRequest): Promise { const currentRequests = getAllPersistedRequests(); Log.info('[SequentialQueue] push() called', false, { command: newRequest.command, @@ -531,39 +545,35 @@ function push(newRequest: OnyxRequest): Promise isReadyPromise) - .then(() => { - Log.info('[SequentialQueue] isReadyPromise resolved, flushing queue', false, { - command: newRequest.command, - }); - flush(true); - }); - return persistencePromise; + isReadyPromise.then(() => flush(false)); + return; } Log.info('[SequentialQueue] Queue is not running. Flushing the queue.', false, { command: newRequest.command, }); - persistencePromise.then(() => flush(true)); - return persistencePromise; + flush(false); } function getCurrentRequest(): Promise { @@ -588,10 +598,9 @@ function resetQueue(): void { isSequentialQueueRunning = false; currentRequestPromise = null; isQueuePaused = false; - isReadyPromise = new Promise((resolve) => { - resolveIsReadyPromise = resolve; - }); - resolveIsReadyPromise?.(); + isReadyPromise = Promise.resolve(); + isReadyPromisePending = false; + resolveIsReadyPromise = undefined; } export { From 31ad82cc1718b5f9fb8544408c19347aa559e12f Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Mon, 1 Jun 2026 14:56:39 +0200 Subject: [PATCH 5/9] SequentialQueue: resolve isReadyPromise when conflict empties the queue push() marks isReadyPromise pending in its sync prelude before awaiting persistence. If a conflict resolver deletes the only request without pushing a replacement (e.g. resolveEnableFeatureConflicts on a rapid feature toggle), flush() exits via the "no requests" early-return without resolving it, leaving API.read() callers parked on waitForIdle() hanging until unrelated queue activity releases the stale promise. Resolve isReadyPromise at that early-return so readiness is always released when there is nothing to flush. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libs/Network/SequentialQueue.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 4ed8c0e40051..5253764a3679 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -337,6 +337,10 @@ function flush(shouldResetPromise = true) { if (persistedRequestsLength === 0 && !currentOngoingRequest && !hasOnyxUpdates) { Log.info('[SequentialQueue] Unable to flush. No requests or queued Onyx updates to process.'); + // push() may have marked isReadyPromise pending in its sync prelude (e.g. a conflict + // resolver deleted the only request without pushing a replacement). Resolve here so READs + // parked on waitForIdle() don't hang until unrelated queue activity releases them. + resolveIsReadyPromise?.(); return; } From b4285d8f82cec0a8062eea95042b40edb85495ac Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Tue, 2 Jun 2026 14:25:51 +0200 Subject: [PATCH 6/9] SequentialQueue: resolve isReadyPromise on follower tabs; fix test typos Address PR review feedback: - push() arms isReadyPromise pending in its sync prelude, but flush() returns early on non-leader clients before resolving it, hanging waitForIdle() forever on follower tabs. Resolve it in the !isClientTheLeader() branch, mirroring the empty-queue fix. - Fix "rance condition" -> "race condition" and remove duplicated word in two test descriptions. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libs/Network/SequentialQueue.ts | 4 ++++ tests/unit/SequentialQueueTest.ts | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 5253764a3679..e04b0df38b34 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -357,6 +357,10 @@ function flush(shouldResetPromise = true) { persistedRequestsLength, hasOngoingRequest: !!currentOngoingRequest, }); + // push() may have marked isReadyPromise pending in its sync prelude. Followers never + // process the queue, so resolve here — otherwise READs parked on waitForIdle() would + // hang forever on this tab after any write. + resolveIsReadyPromise?.(); return; } diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index 5f258a226f43..f53a0e91bba0 100644 --- a/tests/unit/SequentialQueueTest.ts +++ b/tests/unit/SequentialQueueTest.ts @@ -123,7 +123,7 @@ describe('SequentialQueue', () => { } }); - it('should replace request request in queue while a similar one is ongoing', async () => { + it('should replace request in queue while a similar one is ongoing', async () => { const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; mockedFetch.pause(); try { @@ -163,7 +163,7 @@ describe('SequentialQueue', () => { } }); - it('should replace request request in queue while a similar one is ongoing and keep the same index', async () => { + it('should replace request in queue while a similar one is ongoing and keep the same index', async () => { const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; mockedFetch.pause(); try { @@ -199,7 +199,7 @@ describe('SequentialQueue', () => { } }); - // need to test a rance condition between processing the next request and then pushing a new request with conflict resolver + // need to test a race condition between processing the next request and then pushing a new request with conflict resolver it('should resolve the conflict and replace the correct request in the queue while a new request is picked up after unpausing', async () => { SequentialQueue.pause(); for (let i = 0; i < 5; i++) { From 5155cb2ea03e80690be4be8bdb0165522074bddf Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Tue, 2 Jun 2026 17:58:50 +0200 Subject: [PATCH 7/9] SequentialQueue: resolve isReadyPromise if push() goes offline during persist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the network flips offline while push() awaits the Onyx disk write, the sync prelude has already parked waitForIdle() callers via setIsReadyPromisePending(), but the subsequent flush(false) early-returns on its offline guard without resolving isReadyPromise — leaving READs hung until an unrelated reconnect drains the queue. Re-check isOfflineNetwork() after the await and resolve/return, matching flush()'s existing offline-resolves-readiness behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libs/Network/SequentialQueue.ts | 12 ++++++++++ tests/unit/SequentialQueueTest.ts | 36 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index e04b0df38b34..8009166745a5 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -586,6 +586,18 @@ async function push(newRequest: OnyxRequest): Promis // a process kill in that window would lose the request on next launch. await persistencePromise; + // The network may have flipped offline while we awaited the disk write. flush() would + // early-return on its offline check without resolving isReadyPromise, leaving READs parked + // on waitForIdle() until an unrelated reconnect drains the queue. Resolve here so READs + // proceed — consistent with flush() resolving isReadyPromise when offline. + if (isOfflineNetwork()) { + Log.info('[SequentialQueue] Went offline during persist — resolving isReadyPromise without flushing', false, { + command: newRequest.command, + }); + resolveIsReadyPromise?.(); + return; + } + if (isSequentialQueueRunning) { Log.info('[SequentialQueue] Queue is running. Will flush when the current request is finished.', false, { command: newRequest.command, diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index f53a0e91bba0..5082928796fe 100644 --- a/tests/unit/SequentialQueueTest.ts +++ b/tests/unit/SequentialQueueTest.ts @@ -1,5 +1,6 @@ import Onyx from 'react-native-onyx'; import type {OnyxKey, OnyxUpdate} from 'react-native-onyx'; +import * as NetworkState from '@libs/NetworkState'; import {clear as clearPersistedRequests, getAll, getLength, getOngoingRequest, updateOngoingRequest} from '@userActions/PersistedRequests'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -38,6 +39,41 @@ describe('SequentialQueue', () => { expect(getLength()).toBe(2); }); + it('should resolve waitForIdle without flushing when the network goes offline during persist', async () => { + // push()'s sync prelude marks isReadyPromise pending while online, then awaits the disk + // write. If the network flips offline during that await, flush() would early-return on its + // offline guard without resolving isReadyPromise — leaving waitForIdle() (READs) hung until + // an unrelated reconnect. push() must instead resolve isReadyPromise and skip flushing. + const offlineSpy = jest.spyOn(NetworkState, 'getIsOffline').mockReturnValue(false); + let timeoutId: ReturnType | undefined; + try { + // Kick off the push while "online": the synchronous prelude runs up to `await persistencePromise`. + const pushPromise = SequentialQueue.push(request); + + // Flip offline while the awaited disk write is still pending. + offlineSpy.mockReturnValue(true); + + await pushPromise; + + // The request is still persisted — not flushed, not dropped. + expect(getLength()).toBe(1); + + // waitForIdle() must resolve rather than hang. + const idleOrTimeout = await Promise.race([ + SequentialQueue.waitForIdle().then(() => 'resolved' as const), + new Promise<'timeout'>((resolve) => { + timeoutId = setTimeout(() => resolve('timeout'), 1000); + }), + ]); + expect(idleOrTimeout).toBe('resolved'); + } finally { + if (timeoutId) { + clearTimeout(timeoutId); + } + offlineSpy.mockRestore(); + } + }); + it('should push two requests with conflict resolution and replace', async () => { // Pause the queue so `process()` does not consume the first request before // the conflict resolver runs. Under persist-before-fire `push()` is async, From 6944ea45d400fbd2bc3a6cc8406c27bfe754ffcc Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Wed, 10 Jun 2026 12:53:45 +0200 Subject: [PATCH 8/9] SequentialQueue: don't block the queue when a persist write fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conflict-resolution persist paths (update / deleteRequestsByIndices) returned a raw Onyx.set that could reject. Awaiting that in push() before flush() stranded isReadyPromise on a disk-write failure, hanging every READ parked on waitForIdle() and never flushing the request. These paths now swallow the rejection like save() already did — the in-memory queue stays the source of truth for retry — and surface it via Log.alert so a disk-write failure is observable rather than silent. push() keeps a defensive try/catch so a persist rejection can never wedge the queue; on failure it flushes best-effort, matching pre-async behavior. Adds a regression test that fails on the pre-fix code. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libs/Network/SequentialQueue.ts | 8 +++++- src/libs/actions/PersistedRequests.ts | 30 ++++++++++++++++----- tests/unit/SequentialQueueTest.ts | 39 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 8009166745a5..9c4a1b366bf7 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -584,7 +584,13 @@ async function push(newRequest: OnyxRequest): Promis // Block until the Onyx disk commit lands so flush() → XHR cannot race the disk write — // a process kill in that window would lose the request on next launch. - await persistencePromise; + try { + await persistencePromise; + } catch { + // Backstop: persistence alerts+swallows on failure, so this shouldn't reject. If it ever does, + // flush anyway (the request is already in the in-memory queue) rather than stranding isReadyPromise. + Log.info('[SequentialQueue] Persist rejected — flushing anyway', false, {command: newRequest.command}); + } // The network may have flipped offline while we awaited the disk write. flush() would // early-return on its offline check without resolving isReadyPromise, leaving READs parked diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index a689e238a2d7..847946eeb51e 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -260,10 +260,12 @@ function save(requestToPersist: Request): Promise { - Log.info('[PersistedRequests] ERROR: Failed to persist request to disk', false, { + .catch((error) => { + // Disk-write failure risks losing this request on a crash — alert as a storage emergency. + Log.alert('[PersistedRequests] ERROR: Failed to persist request to disk', { command: requestToPersist.command, queueLength: getLength(), + error, }); }); } @@ -334,9 +336,18 @@ function deleteRequestsByIndices(indices: number[]): Promise { persistedRequests = persistedRequests.filter((_, index) => !indicesSet.has(index)); // Update the persisted requests in storage or state as necessary - return trackOnyxWrite(Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests)).then(() => { - Log.info(`Multiple (${indices.length}) requests removed from the queue. Queue length is ${persistedRequests.length}`); - }); + return trackOnyxWrite(Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests)) + .then(() => { + Log.info(`Multiple (${indices.length}) requests removed from the queue. Queue length is ${persistedRequests.length}`); + }) + .catch((error) => { + // Swallow so the conflict promise resolves (in-memory queue is the source of truth); alert as a storage emergency. + Log.alert('[PersistedRequests] ERROR: Failed to persist request deletion to disk', { + indicesCount: indices.length, + queueLength: getLength(), + error, + }); + }); } function update(oldRequestIndex: number, newRequest: Request): Promise { @@ -349,7 +360,14 @@ function update(oldRequestIndex: number, newRequest: Reque if (requestIndex != null) { knownRequestIDs.add(requestIndex); } - return trackOnyxWrite(Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests)); + return trackOnyxWrite(Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests)).catch((error) => { + // Swallow so the conflict promise resolves (in-memory queue is the source of truth); alert as a storage emergency. + Log.alert('[PersistedRequests] ERROR: Failed to persist updated request to disk', { + command: newRequest.command, + queueLength: getLength(), + error, + }); + }); } function shouldPersistOngoingRequest(request: AnyRequest | null): boolean { diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index 5082928796fe..a0a3dedcebff 100644 --- a/tests/unit/SequentialQueueTest.ts +++ b/tests/unit/SequentialQueueTest.ts @@ -74,6 +74,45 @@ describe('SequentialQueue', () => { } }); + it('should not block the queue when a disk write fails during persist', async () => { + // If a conflict-resolution disk write rejects (storage full / corruption), push() must not throw + // or strand isReadyPromise — the request should still flush and waitForIdle() should resolve. + const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; + const originalSet = Onyx.set.bind(Onyx); + + mockedFetch.pause(); + try { + await SequentialQueue.push({command: 'OpenReport'}); // occupies ongoingRequest + await waitForBatchedUpdates(); + await SequentialQueue.push(request); // ReconnectApp stacks in the queue + + // Fail the conflict-resolution persist (a raw Onyx.set on the persisted-requests key). + const setMock = jest + .spyOn(Onyx, 'set') + .mockImplementation((key, value) => (key === ONYXKEYS.PERSISTED_REQUESTS ? Promise.reject(new Error('simulated disk-write failure')) : originalSet(key, value))); + try { + const replacing: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + return {conflictAction: index === -1 ? {type: 'push'} : {type: 'replace', index}}; + }, + }; + // The failed disk write must not reject the caller. + await expect(SequentialQueue.push(replacing)).resolves.toBeUndefined(); + } finally { + setMock.mockRestore(); + } + } finally { + await mockedFetch.resume(); + } + + // The queue still drains and READs unblock — a hang here would fail the test by timing out. + await SequentialQueue.waitForIdle(); + expect(getLength()).toBe(0); + }); + it('should push two requests with conflict resolution and replace', async () => { // Pause the queue so `process()` does not consume the first request before // the conflict resolver runs. Under persist-before-fire `push()` is async, From f68d4c906a5560486fb41be5e9dc097906181be2 Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Wed, 10 Jun 2026 16:33:21 +0200 Subject: [PATCH 9/9] Fix ESLint no-unsafe-type-assertion in SequentialQueue test The test needed a MockFetch-typed handle (pause/resume) for the new disk-write-failure cases. Casting global.fetch to MockFetch tripped @typescript-eslint/no-unsafe-type-assertion in CI. Expose createGlobalFetchMock() returning MockFetch directly (the mock is already built as MockFetch internally), and keep getGlobalFetchMock as a thin typeof-fetch wrapper so existing callers are unaffected. The test now gets a fresh, typed mock per test with no type assertion. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/SequentialQueueTest.ts | 25 ++++++++++++------------- tests/utils/TestHelper.ts | 9 +++++++-- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index a0a3dedcebff..67adffd451ad 100644 --- a/tests/unit/SequentialQueueTest.ts +++ b/tests/unit/SequentialQueueTest.ts @@ -8,6 +8,7 @@ import * as SequentialQueue from '../../src/libs/Network/SequentialQueue'; import * as RequestModule from '../../src/libs/Request'; import type Request from '../../src/types/onyx/Request'; import type {AnyRequest, ConflictActionData} from '../../src/types/onyx/Request'; +import type {MockFetch} from '../utils/TestHelper'; import * as TestHelper from '../utils/TestHelper'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; @@ -16,13 +17,15 @@ const request: Request<'userMetadata'> = { successData: [{key: 'userMetadata', onyxMethod: 'set', value: {accountID: 1234}}], failureData: [{key: 'userMetadata', onyxMethod: 'set', value: {}}], }; +let mockFetch: MockFetch; beforeAll(() => { Onyx.init({ keys: ONYXKEYS, }); }); beforeEach(() => { - global.fetch = TestHelper.getGlobalFetchMock(); + mockFetch = TestHelper.createGlobalFetchMock(); + global.fetch = mockFetch; return Onyx.clear() .then(() => SequentialQueue.clearQueueFlushedData()) .then(waitForBatchedUpdates); @@ -77,10 +80,9 @@ describe('SequentialQueue', () => { it('should not block the queue when a disk write fails during persist', async () => { // If a conflict-resolution disk write rejects (storage full / corruption), push() must not throw // or strand isReadyPromise — the request should still flush and waitForIdle() should resolve. - const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; const originalSet = Onyx.set.bind(Onyx); - mockedFetch.pause(); + mockFetch.pause(); try { await SequentialQueue.push({command: 'OpenReport'}); // occupies ongoingRequest await waitForBatchedUpdates(); @@ -105,7 +107,7 @@ describe('SequentialQueue', () => { setMock.mockRestore(); } } finally { - await mockedFetch.resume(); + await mockFetch.resume(); } // The queue still drains and READs unblock — a hang here would fail the test by timing out. @@ -170,8 +172,7 @@ describe('SequentialQueue', () => { // The conflict checker on push 2 inspects the persisted queue (which excludes the // ongoing request), so it cannot find a 'ReconnectApp' to replace and falls back // to 'push'. The new request is therefore added to the queue. - const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; - mockedFetch.pause(); + mockFetch.pause(); try { await SequentialQueue.push(request); await waitForBatchedUpdates(); @@ -194,13 +195,12 @@ describe('SequentialQueue', () => { // The new request is in the persisted queue with the expected accountID. expect(getAll().some((r) => r.data?.accountID === 56789)).toBe(true); } finally { - await mockedFetch.resume(); + await mockFetch.resume(); } }); it('should replace request in queue while a similar one is ongoing', async () => { - const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; - mockedFetch.pause(); + mockFetch.pause(); try { await SequentialQueue.push(request); await waitForBatchedUpdates(); @@ -234,13 +234,12 @@ describe('SequentialQueue', () => { expect(getLength()).toBe(2); } finally { - await mockedFetch.resume(); + await mockFetch.resume(); } }); it('should replace request in queue while a similar one is ongoing and keep the same index', async () => { - const mockedFetch = global.fetch as ReturnType & {pause: () => void; resume: () => Promise}; - mockedFetch.pause(); + mockFetch.pause(); try { // First push moves into `ongoingRequest`; subsequent pushes stack in the queue. await SequentialQueue.push({command: 'OpenReport'}); @@ -270,7 +269,7 @@ describe('SequentialQueue', () => { expect(getOngoingRequest()?.command).toBe('OpenReport'); expect(persistedRequests.at(0)?.data?.accountID).toBe(56789); } finally { - await mockedFetch.resume(); + await mockFetch.resume(); } }); diff --git a/tests/utils/TestHelper.ts b/tests/utils/TestHelper.ts index b710ea226cde..edea0633e606 100644 --- a/tests/utils/TestHelper.ts +++ b/tests/utils/TestHelper.ts @@ -217,7 +217,7 @@ function signOutTestUser() { * - fail() - start returning a failure response * - success() - go back to returning a success response */ -function getGlobalFetchMock(mockResponse?: Partial): typeof fetch { +function createGlobalFetchMock(mockResponse?: Partial): MockFetch { let queue: QueueItem[] = []; // eslint-disable-next-line @typescript-eslint/no-explicit-any let responses = new Map OnyxResponse>(); @@ -280,7 +280,11 @@ function getGlobalFetchMock(mockResponse?: Partial): typeof fetch { mockFetch.mockAPICommand = (command: TCommand, responseHandler: (params: ApiRequestCommandParameters[TCommand]) => OnyxResponse): void => { responses.set(command, responseHandler); }; - return mockFetch as typeof fetch; + return mockFetch; +} + +function getGlobalFetchMock(mockResponse?: Partial): typeof fetch { + return createGlobalFetchMock(mockResponse); } function setupGlobalFetchMock(): MockFetch { @@ -394,6 +398,7 @@ export { buildTestReportComment, getFetchMockCalls, getGlobalFetchMock, + createGlobalFetchMock, setPersonalDetails, signInWithTestUser, signOutTestUser,