[Due for payment 2026-06-12] [Due for payment 2026-06-09] [$50] Flaky test: MiddlewareTest › HandleUnusedOptimisticID - fetch call count assertion fails intermittently

[mountiny]

Description

Two tests in `tests/unit/MiddlewareTest.ts` are flaky and fail intermittently in CI:

• `Middleware › HandleUnusedOptimisticID › Request with preexistingReportID` (line 149)
• `Middleware › HandleUnusedOptimisticID › OpenReport to a chat with preexistingReportID and clean up optimistic participant data` (line 343)

Both fail on the same assertion:

```
expect(global.fetch).toHaveBeenCalledTimes(2)
```

Evidence

Observed in CI run #33684 (attempt 1 failed, attempt 2 passed — confirming this is a flaky/timing issue).

The logs also report a worker teardown leak:

A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks. Active timers can also cause this.

Likely cause

The tests use `waitForNetworkPromises()` / `waitForBatchedUpdates()` before asserting on `global.fetch` call count. A race condition between the `SequentialQueue.unpause()` call and the fetch mock resolution can cause the second fetch to fire after the assertion runs. The worker teardown leak suggests an open handle (timer or promise) that affects subsequent test isolation.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~022054904870462893459
• Upwork Job ID: 2054904870462893459
• Last Price Increase: 2026-05-14

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @dmkt9 (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022054904870462893459

[melvin-bot]

Upwork job price has been updated to $50

[mountiny]

Making this $50 as its flaky test

[yusufdeveloper2903]

🚨 Edited by proposal-police: This proposal was edited at 2026-05-14 12:46:58 UTC.

Proposal

What is the root cause of that problem?

Both flaky tests push two requests onto the SequentialQueue, then assert expect(global.fetch).toHaveBeenCalledTimes(2) after only a fixed number of microtask/macrotask flushes — waitForNetworkPromises() (two waitForBatchedUpdates rounds) in the first test, and a single waitForBatchedUpdates() in the second:

• App/tests/unit/MiddlewareTest.ts

  Lines 146 to 149 in 1b89b63

  	SequentialQueue.unpause();
  	await waitForNetworkPromises();
  	expect(global.fetch).toHaveBeenCalledTimes(2);

• App/tests/unit/MiddlewareTest.ts

  Lines 340 to 343 in 1b89b63

  	SequentialQueue.unpause();
  	await waitForBatchedUpdates();
  	expect(global.fetch).toHaveBeenCalledTimes(2);

But the queue does not process both requests in a fixed number of cycles. process() handles one request at a time through a recursive chain — it resolves the fetch mock (two async hops for the async mock + its async json()), runs the middleware chain (which performs async Onyx writes), applies the response via Onyx.update, and only then does .then(() => process()) pick up the next request and fire its fetch:

App/src/libs/Network/SequentialQueue.ts

Lines 165 to 196 in 1b89b63

	currentRequestPromise = processWithMiddleware(requestToProcess, true)
	.then((response) => {
	Log.info('[SequentialQueue] Request processed successfully', false, {
	command: requestToProcess.command,
	shouldPauseQueue: response?.shouldPauseQueue ?? false,
	hasQueueFlushedData: !!requestToProcess.queueFlushedData,
	});
	// A response might indicate that the queue should be paused. This happens when a gap in onyx updates is detected between the client and the server and
	// that gap needs resolved before the queue can continue.
	if (response?.shouldPauseQueue) {
	Log.info("[SequentialQueue] Handled 'shouldPauseQueue' in response. Pausing the queue.");
	pause();
	}
	Log.info('[SequentialQueue] Removing persisted request because it was processed successfully.', false, {
	command: requestToProcess.command,
	remainingRequests: getAllPersistedRequests().length,
	});
	endPersistedRequestAndRemoveFromQueue(requestToProcess);
	if (requestToProcess.queueFlushedData) {
	Log.info('[SequentialQueue] Will store queueFlushedData.', false, {
	command: requestToProcess.command,
	queueFlushedDataLength: requestToProcess.queueFlushedData.length,
	});
	saveQueueFlushedData(...requestToProcess.queueFlushedData);
	}
	sequentialQueueRequestThrottle.clear();
	Log.info('[SequentialQueue] Continuing to process next request');
	return process();

The async depth required for the second fetch to fire is variable. Under CI scheduling pressure it exceeds the fixed number of flushes, so the assertion runs while only the first fetch has been called → toHaveBeenCalledTimes(2) fails. The same unbounded process() chain still running after the test returns is also the open handle behind the "worker process failed to exit gracefully" teardown leak.

Note: commit a7ede572585 improved this file's test isolation (beforeEach resets), but it did not address this assertion-timing race.

What changes do you think we should make in order to solve the problem?

Stop waiting a fixed number of cycles and instead wait for the queue to actually drain, using the existing SequentialQueue.waitForIdle() idiom (already used in ReportAttachments, WorkspaceUpgradeTest, PaginationTest). push() of the first request synchronously calls flush(true), which resets isReadyPromise before the test reaches its wait; that promise resolves in process().finally() only after the whole recursive chain has drained both requests:

App/src/libs/Network/SequentialQueue.ts

Lines 583 to 589 in 1b89b63

	/**
	* Returns a promise that resolves when the sequential queue is done processing all persisted write requests.
	*/
	function waitForIdle(): Promise<unknown> {
	return isReadyPromise;
	}

Middleware › HandleUnusedOptimisticID › Request with preexistingReportID (L147):

            SequentialQueue.unpause();
-           await waitForNetworkPromises();
+           await SequentialQueue.waitForIdle();

Middleware › HandleUnusedOptimisticID › OpenReport to a chat with preexistingReportID and clean up optimistic participant data (L341):

            SequentialQueue.unpause();
-           await waitForBatchedUpdates();
+           await SequentialQueue.waitForIdle();
+           await waitForBatchedUpdates();

(SequentialQueue is already imported in this file; the trailing waitForBatchedUpdates() in the second test keeps the follow-up Onyx reads consistent.) This makes the fetch count deterministic and guarantees the queue is fully drained before teardown, which also removes the worker-exit leak.

The identical fixed-cycle pattern exists at the other SequentialQueue.unpause() sites in this file (L93-L94, L198-L199, L253-L254, L438-L439); applying the same waitForIdle() wait there prevents the same flake from resurfacing in those tests.

What alternative solutions did you explore? (Optional)

Adding more waitForBatchedUpdates() cycles (or widening waitForNetworkPromises) — rejected: it only lowers the probability of the race, it does not eliminate it, since the queue's async depth is unbounded relative to any fixed cycle count. await SequentialQueue.getCurrentRequest() — rejected: currentRequestPromise is null synchronously after unpause() (the chain starts inside an async Onyx.connectWithoutView callback), so awaiting it immediately resolves and waits for nothing.