Skip to content
2 changes: 1 addition & 1 deletion contributingGuides/SEQUENTIAL_QUEUE.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ These are **two distinct deferral mechanisms** that are easy to confuse.

**Problem `queueFlushedData` solves.** Apply a small piece of data **only after a full drain**: mark the app as loaded only once the queue has actually emptied, not mid-drain.

**How `queueFlushedData` works.** It is a **distinct, Onyx-persisted** buffer (`QUEUE_FLUSHED_DATA`), separate from the in-memory `QueuedOnyxUpdates`. `SequentialQueue.saveQueueFlushedData` appends a successfully-processed request's `queueFlushedData` field; the queue applies it via `Onyx.update` and clears it only when fully drained (after `flushOnyxUpdatesQueue`). Its sole producer is `App.getOnyxDataForOpenOrReconnect` (`OPEN_APP` / `ReconnectApp`), currently carrying exactly one entry: a merge of `HAS_LOADED_APP = true`.
**How `queueFlushedData` works.** It is a **distinct, Onyx-persisted** buffer (`QUEUE_FLUSHED_DATA`), separate from the in-memory `QueuedOnyxUpdates`. `SequentialQueue.saveQueueFlushedData` appends a request's `queueFlushedData` field only when the response's `jsonCode` is `CONST.JSON_CODE.SUCCESS`; a resolved-but-failed response (`HttpUtils.xhr` resolves application-level failures instead of rejecting them) does not save it, so it cannot wrongly mark `HAS_LOADED_APP` true on the next boot. The queue applies it via `Onyx.update` and clears it only when fully drained (after `flushOnyxUpdatesQueue`). Its sole producer is `App.getOnyxDataForOpenOrReconnect` (`OPEN_APP` / `ReconnectApp`), currently carrying exactly one entry: a merge of `HAS_LOADED_APP = true`.

**Sharp edges.**
- Both apply **only** when the queue reaches fully-empty. Under sustained WRITE pressure neither applies, so `HAS_LOADED_APP` never flips and the buffers accumulate.
Expand Down
5 changes: 4 additions & 1 deletion src/libs/Network/SequentialQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ function process(): Promise<void> {
});
endPersistedRequestAndRemoveFromQueue(requestToProcess);

if (requestToProcess.queueFlushedData) {
// Only commit queueFlushedData (e.g. HAS_LOADED_APP: true) on success — HttpUtils resolves (not rejects) app-level
Comment thread
mountiny marked this conversation as resolved.
// failures, so committing on a failed-but-resolved OpenApp/ReconnectApp would wrongly mark the app as loaded and
// break self-healing on the next boot.
if (requestToProcess.queueFlushedData && response?.jsonCode === CONST.JSON_CODE.SUCCESS) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

response?.jsonCode has type number | string. While the backend usually returns a number, using strict equality (===) could fail if "200" is ever returned as a string. Should we normalize it with Number(response?.jsonCode) === CONST.JSON_CODE.SUCCESS, or confirm whether the backend API contract strictly guarantees a number here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional, it matches the successData gate in OnyxUpdates.ts so both commit under the same condition. Normalizing one side only could make them diverge

Log.info('[SequentialQueue] Will store queueFlushedData.', false, {
command: requestToProcess.command,
queueFlushedDataLength: requestToProcess.queueFlushedData.length,
Expand Down
32 changes: 32 additions & 0 deletions tests/unit/SequentialQueueTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {MockFetch} from '../utils/TestHelper';

import * as SequentialQueue from '../../src/libs/Network/SequentialQueue';
import * as RequestModule from '../../src/libs/Request';
import getOnyxValue from '../utils/getOnyxValue';
import * as TestHelper from '../utils/TestHelper';
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';

Expand Down Expand Up @@ -560,4 +561,35 @@ describe('SequentialQueue - QueueFlushedData', () => {
await SequentialQueue.clearQueueFlushedData();
expect(SequentialQueue.getQueueFlushedData()).toEqual([]);
});

afterEach(() => {
jest.restoreAllMocks();
});

// Pushes an OpenApp request carrying queueFlushedData, with processWithMiddleware mocked to resolve with the given jsonCode.
async function pushOpenAppAndWaitForIdle(jsonCode: number) {
await Onyx.set(ONYXKEYS.NETWORK, {shouldFailAllRequests: false, shouldForceOffline: false});
await clearPersistedRequests();
await waitForBatchedUpdates();

const flushedUpdate: OnyxUpdate<typeof ONYXKEYS.HAS_LOADED_APP> = {onyxMethod: Onyx.METHOD.MERGE, key: ONYXKEYS.HAS_LOADED_APP, value: true};
jest.spyOn(RequestModule, 'processWithMiddleware').mockResolvedValue({jsonCode});
SequentialQueue.push({command: 'OpenApp', queueFlushedData: [flushedUpdate]});
await SequentialQueue.waitForIdle();
await waitForBatchedUpdates();
}

it('does not commit queueFlushedData when the resolved response is not a 200', async () => {
await pushOpenAppAndWaitForIdle(CONST.JSON_CODE.BAD_REQUEST);

// A failed-but-resolved OpenApp must not stage HAS_LOADED_APP, or the next boot runs ReconnectApp only and can't self-heal.
expect(SequentialQueue.getQueueFlushedData()).toEqual([]);
expect(await getOnyxValue(ONYXKEYS.HAS_LOADED_APP)).toBeFalsy();
});

it('commits queueFlushedData when the resolved response is a 200', async () => {
await pushOpenAppAndWaitForIdle(CONST.JSON_CODE.SUCCESS);

expect(await getOnyxValue(ONYXKEYS.HAS_LOADED_APP)).toBe(true);
});
});
Loading