Skip to content

Commit 6e3ff4a

Browse files
adhorodyskiclaude
andcommitted
Fix SequentialQueue push() ordering regression
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 70d1ac9 commit 6e3ff4a

2 files changed

Lines changed: 30 additions & 19 deletions

File tree

src/libs/Network/SequentialQueue.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ let currentRequestPromise: Promise<void> | null = null;
5757
let isQueuePaused = false;
5858
const sequentialQueueRequestThrottle = new RequestThrottle('SequentialQueue');
5959

60+
// Tracks the latest in-flight Onyx persistence write. process() awaits this before firing
61+
// the next XHR so the network call cannot race the Onyx.set() commit — if the process
62+
// is killed after fire but before disk commit, the request would be lost on next launch.
63+
let pendingPersistencePromise: Promise<void> = Promise.resolve();
64+
6065
/**
6166
* Puts the queue into a paused state so that no requests will be processed
6267
*/
@@ -162,7 +167,9 @@ function process(): Promise<void> {
162167
});
163168

164169
// Set the current request to a promise awaiting its processing so that getCurrentRequest can be used to take some action after the current request has processed.
165-
currentRequestPromise = processWithMiddleware(requestToProcess, true)
170+
// Await pendingPersistencePromise so the XHR is not fired before the Onyx disk commit lands.
171+
currentRequestPromise = pendingPersistencePromise
172+
.then(() => processWithMiddleware(requestToProcess, true))
166173
.then((response) => {
167174
Log.info('[SequentialQueue] Request processed successfully', false, {
168175
command: requestToProcess.command,
@@ -504,7 +511,7 @@ async function handleConflictActions<TKey extends OnyxKey>(conflictAction: Confl
504511
}
505512
}
506513

507-
async function push<TKey extends OnyxKey>(newRequest: OnyxRequest<TKey>): Promise<void> {
514+
function push<TKey extends OnyxKey>(newRequest: OnyxRequest<TKey>): Promise<void> {
508515
const currentRequests = getAllPersistedRequests();
509516
Log.info('[SequentialQueue] push() called', false, {
510517
command: newRequest.command,
@@ -514,10 +521,11 @@ async function push<TKey extends OnyxKey>(newRequest: OnyxRequest<TKey>): Promis
514521
isSequentialQueueRunning,
515522
});
516523

517-
// Persist-before-fire: await disk write before any flush(). The in-memory update
518-
// inside save()/handleConflictActions happens synchronously, but the network call
519-
// must NOT race the Onyx.set() promise. If the process is killed after fire but
520-
// before disk commit, the request is lost — no retry on next launch.
524+
// save()/handleConflictActions update the in-memory queue synchronously and return a
525+
// promise that resolves once the Onyx disk commit completes. flush() runs synchronously
526+
// so isSequentialQueueRunning flips immediately and reads can park behind the queue.
527+
// The persistencePromise is chained onto pendingPersistencePromise so process() can
528+
// await it before firing the XHR — the network call must not race the disk commit.
521529
let persistencePromise: Promise<void>;
522530

523531
if (newRequest.checkAndFixConflictingRequest) {
@@ -545,20 +553,15 @@ async function push<TKey extends OnyxKey>(newRequest: OnyxRequest<TKey>): Promis
545553
persistencePromise = savePersistedRequest(newRequest);
546554
}
547555

548-
await persistencePromise;
549-
Log.info('[SequentialQueue] Request persisted, proceeding to flush', false, {
550-
command: newRequest.command,
551-
isOffline: isOfflineNetwork(),
552-
isSequentialQueueRunning,
553-
});
556+
pendingPersistencePromise = pendingPersistencePromise.then(() => persistencePromise).catch(() => undefined);
554557

555558
// If we are offline we don't need to trigger the queue to empty as it will happen when we come back online
556559
if (isOfflineNetwork()) {
557560
Log.info('[SequentialQueue] Request persisted but not flushing — we are offline', false, {
558561
command: newRequest.command,
559562
queueLength: getAllPersistedRequests().length,
560563
});
561-
return;
564+
return persistencePromise;
562565
}
563566

564567
// If the queue is running this request will run once it has finished processing the current batch
@@ -572,13 +575,14 @@ async function push<TKey extends OnyxKey>(newRequest: OnyxRequest<TKey>): Promis
572575
});
573576
flush(true);
574577
});
575-
return;
578+
return persistencePromise;
576579
}
577580

578581
Log.info('[SequentialQueue] Queue is not running. Flushing the queue.', false, {
579582
command: newRequest.command,
580583
});
581584
flush(true);
585+
return persistencePromise;
582586
}
583587

584588
function getCurrentRequest(): Promise<void> {

tests/unit/SequentialQueuePersistBeforeFireTest.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,19 @@ describe('SequentialQueue.push - persist-before-fire', () => {
8787
});
8888

8989
it('at the await boundary, on-disk PERSISTED_REQUESTS already contains the new request', async () => {
90-
await SequentialQueue.push({...request});
90+
// Pause the queue so process() does not drain (and remove) the request before we can
91+
// read it back. We are asserting persistence, not processing.
92+
SequentialQueue.pause();
93+
try {
94+
await SequentialQueue.push({...request});
9195

92-
// Read straight from Onyx to confirm the persisted shape, not just in-memory state.
93-
const onDisk = await OnyxUtils.get(ONYXKEYS.PERSISTED_REQUESTS);
94-
expect(onDisk).toBeDefined();
95-
expect(onDisk?.some((r) => r.command === request.command)).toBe(true);
96+
// Read straight from Onyx to confirm the persisted shape, not just in-memory state.
97+
const onDisk = await OnyxUtils.get(ONYXKEYS.PERSISTED_REQUESTS);
98+
expect(onDisk).toBeDefined();
99+
expect(onDisk?.some((r) => r.command === request.command)).toBe(true);
100+
} finally {
101+
SequentialQueue.unpause();
102+
}
96103
});
97104

98105
it('offline: push() resolves after persistence and does not call fetch', async () => {

0 commit comments

Comments
 (0)