#17290
Issues/diegosouzapw/OmniRoute/#17290

test(perplexity): isolated-pipeline stream-error-boundary test is flaky under load (90 s fixture timeout)

Opendiegosouzapwopened 2d ago

Summary

tests/unit/perplexity-web-stream-error-boundary.test.ts ("Perplexity stream failures preserve protocol semantics in an isolated full pipeline") is flaky: it spawns tests/fixtures/perplexity-web-stream-error-boundary.fixture.ts as a subprocess with a 90 s timeout, and under load the fixture hangs until that timeout.

Evidence (2026-10-20)

  • Unit Tests fast-path (1/4) (quality.yml) failed on this test alone for #17237 and three times in a row for #17250 (reruns 39424169703), on PRs that touch no code the test exercises (docs/i18n gate, release scripts).
  • Locally on the release/v3.8.53 tree (dba56ba9 + #17250), three consecutive runs: 2 failures at the fixture timeout (89797 ms, 89721 ms), 1 pass — same machine, no code change between runs.
  • Earlier failures of the same job today on unrelated branches (fix/empty-turn-retry-direct-path, fix/streamed-content-presence-measurement).

Failure output when it hangs:

✖ isolated Perplexity fixture failed (signal=none)
AssertionError [ERR_ASSERTION]: the downstream pipeline must finalize the stream failure
SqliteError: wvxc route injected failure at /secret/path.ts:1

Impact

The shard is required, so a 30–60 % failure rate on an unrelated test blocks every PR into release/v3.8.53 until a rerun lands green (two reruns were needed on #17250). The release skill's merge watcher had to treat it as "known flaky".

Suggested fix

  • Find why the fixture does not finish its stream-failure finalization under load (the injected SQLite failure path + event-bus isolation); add an explicit AbortController/settle wait instead of relying on process exit within 90 s.
  • Until then: mark the isolated-pipeline case as retried-once by the shard runner, or move it to the integration lane (tests/integration/) where a 90 s subprocess fixture belongs.

Validation plan

Run node --import tsx/esm --test tests/unit/perplexity-web-stream-error-boundary.test.ts 10× on a loaded devbox (two npm run build in parallel) — 10/10 passes before closing.

Activity

4
diegosouzapw2d ago

Root cause found — this is not a flaky test, it is a race introduced by the chatCore decomposition in #14725 (2026-10-06).

runStreamingTail creates onPipelineStreamError / onClientDisconnectFinalize and returns them to chatCore through carry, i.e. only after the tail has resolved. chatCore's stream controller reads them through getters at event time, so from "pipeline assembled and pumping" until "tail returned" the getter answers null. An upstream failure pumped through the pipeline during the tail's remaining awaits finds no handler: handleError falls through to clearPendingRequest, nothing calls handleStreamFailure, and the request is never finalized as a stream failure (no onStreamFailure, no stream_pipeline_error record). That is exactly the fixture's the downstream pipeline must finalize the stream failure assertion.

Fast machines rarely hit the window; hosted CI runners hit it on 16 of 21 unit-shard attempts across the release/v3.8.53 PRs today (2026-10-20).

Fix: #17329 — chatCore owns a holder shared by reference with the tail, and the tail publishes both handlers into it right after creating them, before assembleStreamingPipeline starts pumping (the pre-#14725 closure timing). Regression test tests/unit/chatcore/late-bound-stream-handlers.test.ts.

Separate, unrelated: on the devbox the fixture subprocess stalls ~60–80 s at import time (environment), which is the "90 s timeout" mode noted above.

diegosouzapw2d ago

Additional bounded local evidence: the eight tests in tests/fixtures/perplexity-web-stream-error-boundary.fixture.ts passed on release base 0df2f855cab65b9b6bde06a210e8d98a72139eda, Node 24.20.0, with isolated synthetic data and completed dependencies:

node --max-old-space-size=768 --import tsx/esm --test --test-force-exit --test-concurrency=1 tests/fixtures/perplexity-web-stream-error-boundary.fixture.ts

Total process duration was 215.823 seconds; the assertions themselves completed after the lengthy module initialization. This is a direct fixture run, not a pass of the 90-second wrapper, Node 24.22 CI, or ten repetitions under load. An earlier run started before dependencies were fully linked and failed module resolution; that setup failure is not bug evidence.

The previous Quality failure on #17255 contains finalizedFailure assertion failures, so timeout, finalizer ordering (#17226), and reporter selection (#17321) need separate evidence. The current release advanced to e9818ae7 with an Electron-workflow change; this local receipt does not certify the new tip or any PR for merge. Keeping this open while the existing stream work and required checks are reconciled.

diegosouzapw2d ago

Root cause found: this is not load-dependent flakiness, it is deterministic on Node 24.22.0 and the "flaky" pattern comes from hosted runners resolving node-version: 24 to different cached patch releases.

Evidence:

  • Same merge commit (129d16e6, #17162): CI attempt 1 ran on Node 24.21.0 and passed; attempt 2 ran on 24.22.0 and failed with the 4 content-then-failure subtests (finalizedFailure null). The #17250-era failure on fix/read-shim-respect-client-schema (run 39451142450) also ran on 24.22.0.
  • Locally, running the fixture directly on the release/v3.8.53 tip: Node 24.22.0 → fail 4 on 3/3 runs; Node 24.21.0 and 24.20.0 → pass 8 every time (also with 6 parallel runs and with the process pinned to a saturated core, so load is not the trigger).
  • Instrumented run: on 24.22.0 the upstream error reaches streamController.handleError, but getPipelineStreamError() in chatStreamController.ts still returns null. chatCore only assigns onPipelineStreamError from the carry after await runStreamingTail(...) returns, and on 24.22.0 the stream starts being consumed (and fails) before that, during the awaited response hooks. The failure is therefore never finalized or notified.

#17226 (@pacocartones) fixes exactly this by registering the finalizers before the pipeline is assembled. With its diff applied on the tip, the fixture passes 8/8 on Node 24.22.0 (twice) and on 24.21.0.

The 90 s hang described above is likely the same missing finalization seen from a different code path; it should be re-checked after #17226 lands. Moving the test to the integration lane or retrying it would hide a real production bug: users on Node 24.22 lose stream-failure accounting and connection failure handling.

diegosouzapw1d ago

Follow-up: #17329 (my duplicate of the same fix) is closed in favour of #17226 by @pacocartones, which was opened first. #17226 was brought up to the current release/v3.8.53 tip so its checks run against a green base; it merges as soon as they pass.

Extra data point for the Node 24.22.0 finding: on the .113 (Node 24.18.0) the fixture passed 15/15 on the unfixed tip, including runs pinned to two cores under CPU hogs — consistent with the failure being patch-version-specific rather than load-dependent.

This issue should close with #17226.