fix(chatCore): stop executeWithUpstreamStartTimeout leaking its abortPromise listener (hedge-cancelled process exit) - #12406
Conversation
mergeAbortSignals() attached "abort" listeners to its primary/secondary signals but never removed them once the merged signal settled. Every executor fetch attempt calls this (fetchWithStartTimeout, once per URL/retry), so a busy combo request accumulated one live listener per call on the long-lived combo/client signal. A leaked listener still fires when that signal is later aborted (e.g. a hedge cancellation arriving after this merge's own caller already finished), for a merged output nothing is watching anymore. Mirrors the already-correct self-cleaning pattern in open-sse/utils/directResponseStartTimeout.ts's local mergeAbortSignals. Regression test measures listener growth across repeated merges of the same long-lived signal: 25 merges leaked exactly 25 listeners pre-fix, 0 post-fix. (cherry picked from commit 0796965)
Production crash 2026-08-31 (omniroute.log): on a client disconnect,
handleDisconnect aborted the combo controller and a late abort listener
threw the abort reason on an empty stack:
Error [AbortError]: hedge-cancelled
at ... AbortController.abort ... handleDisconnect
file:///.../src/shared/utils/httpClientAbortGuard.mjs:130 throw err;
isClientAbortError() only knew Node's stream codes and "aborted", so
shouldSwallowUncaught() said false and the guard re-threw, taking the
whole server down.
- Port upstream's AbortError line (name "AbortError" + abort-flavoured
message) so request_signal_aborted / DOMException aborts are absorbed.
- Add an exact-message match for the combo abort reasons from
open-sse/services/combo/comboAbortReasons.ts ("hedge-cancelled",
"combo-per-model-timeout"), name-agnostic because the raw reason is a
plain Error that only gets name="AbortError" stamped on the way out.
A losing hedge / stalled target is never a server fault. Inlined so
this .mjs stays dependency-free for scripts/dev/run-next.mjs.
Tests: port upstream's guard tests, add the exact crash shape, a
child-process replay of the crash (dies pre-fix, survives post-fix), a
genuine-error case that must still crash, and a sync check against
comboAbortReasons.ts. The child-process helper passes a file:// URL, not
a bare path, so the tests run on Windows.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 90c9bce)
…Promise listener Root cause of the 2026-08-31 production exit (Error [AbortError]: hedge-cancelled), verified by mapping the crash frames in .build/next/server/chunks/13721.js back to this file: - The abortPromise abort listener registered on the long-lived client / stream signal was never removed in the finally block (only abortListener and timeoutAbortListener were), so every executor attempt (and every retry) leaked one listener onto that signal. - Promise.race only subscribes to abortPromise/timeoutPromise once the array literal has been evaluated. When execute() threw synchronously the race never ran, abortPromise was orphaned, and the next hedge cancellation / client disconnect aborted the signal with the string reason streamHandler.ts forwards; createAbortError() rebuilt it as an AbortError-named Error and rejected a promise nothing awaited. That unhandledRejection reached the process crash guard, which re-threw it as an uncaughtException and exited with code 7. Keep a handle to the listener and remove it with the others, and mark the two race-loser promises as handled so a synchronous throw from execute() can never orphan them. Race semantics are unchanged (the race still observes their rejections). Regression tests: (1) a resolving execute leaves the listener count on the client signal unchanged; (2) a synchronously throwing execute leaks no listener and a later abort with the string "hedge-cancelled" produces no unhandledRejection. Both fail against the previous implementation. Note: commit 0796965 (mergeAbortSignals cleanup) is correct listener hygiene but is not on this crash path; this is the fix for the incident. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit e68a50a)
…ment the verified crash path Follow-ups from the adversarial review of 90c9bce: - open-sse/utils/streamHandler.ts aborts the stream controller with a raw string reason (getClientAbortReason / handleDisconnect) and undici rejects with signal.reason verbatim, so a cancellation can reach process level as a bare string. isClientAbortError() returned false for every non-object, which would still have exited the process. Absorb the combo abort reasons and the stream-handler disconnect reasons when they arrive as strings. - Correct the mechanism comment: the 2026-08-31 exit was a leaked upstreamTimeouts.ts abortPromise listener rejecting a promise nothing awaited (unhandledRejection), escalated by this guard, not a listener throwing synchronously. The leak is fixed at the source in the previous commit; this guard remains the last-resort net. - Reword the inlining rationale (plain node launcher, no reliance on type-stripping for the .ts constants module). - Tests: the child-process replay now also exercises the unhandledRejection route with the exact production error shape and with raw string reasons; add unit coverage for string reasons and non-object look-alikes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 696fcc8)
…am-timeouts-abort-listener-leak Keep the abort-listener leak fix (executeWithUpstreamStartTimeout + mergeAbortSignals) and the crash-guard combo/string abort absorption on top of release/v3.8.51. Extract mergeAbortSignals out of frozen open-sse/executors/base.ts so the file-size cap is not raised. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Solid fix for the abort-listener leak, and the extraction of |
Resolves the conflict in open-sse/executors/base.ts — kept the base's single-line `cliFingerprints` import with its `// prettier-ignore` marker (the branch had only reformatted that line); the mergeAbortSignals extraction into ./base/mergeAbortSignals.ts merged cleanly. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Thanks @Beexly — merging via the release merge-train. Validated in local merge-train /tmp/mt-train3b.log on 192.168.0.113 @ train tip 408e2128791696a18966681953136fc96aeb99b0 (32 PRs boarded): static gates green; full test:unit 40694 tests, 17 failing — every one reproduces on the pure release tip (base-red sweep list), zero new reds. Merged --admin per merge-gates §4/§7. |
Re-sync onto the current tip — both sides appended tests at the end of tests/unit/httpClientAbortGuard.test.mjs (this PR's raw-string abort reasons, the base's diegosouzapw#14064 full-error logging); kept both, and restored the fileURLToPath import the auto-merge dropped. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
|
Thanks @Beexly — merging via the release merge-train. Validated in local merge-train (merge-train-20260918-130526-suite.log) on the devbox @ train tip 28fb420c9ad860ba275294ebb4ebbecb9da83318 with the sibling PRs of this batch: static gates green; changed-area node:test 293/294 (0 failing) + vitest 482/482 (fast parity — full suite ran today on the tip via the base-red and 3b trains). Merged --admin per merge-gates §7. |
…no live body Root cause: diegosouzapw#14342 changed executeWithUpstreamStartTimeout to keep the client-signal `abortListener` (the link that aborts combinedController) whenever the start-timeout race settled successfully, so a client abort after headers could still reach the upstream fetch. It kept the link for every resolved result, including results that carry no streaming body, so one listener stayed on the client signal after the race settled. That broke the diegosouzapw#12406 guard ("every listener registered for the race must be removed once it settles", 1 !== 0). Fix: keep the link only when the settled result (a Response or the executor's `{ response }` wrapper) still has an unconsumed body that will stream on the combined signal. Otherwise remove it in the finally, as for failed attempts. The abortPromise listener is still always removed. The diegosouzapw#14342 post-headers propagation is unchanged for streaming responses. Tests: the diegosouzapw#12406 guard is green again; two new cases pin both sides (a body-less Response releases the link; a streaming body keeps exactly one link, propagates the abort, and it self-removes on abort). Refs diegosouzapw#14547
…Promise listener (hedge-cancelled process exit) (diegosouzapw#12406) * fix(sse): stop mergeAbortSignals from leaking abort listeners mergeAbortSignals() attached "abort" listeners to its primary/secondary signals but never removed them once the merged signal settled. Every executor fetch attempt calls this (fetchWithStartTimeout, once per URL/retry), so a busy combo request accumulated one live listener per call on the long-lived combo/client signal. A leaked listener still fires when that signal is later aborted (e.g. a hedge cancellation arriving after this merge's own caller already finished), for a merged output nothing is watching anymore. Mirrors the already-correct self-cleaning pattern in open-sse/utils/directResponseStartTimeout.ts's local mergeAbortSignals. Regression test measures listener growth across repeated merges of the same long-lived signal: 25 merges leaked exactly 25 listeners pre-fix, 0 post-fix. (cherry picked from commit 0796965) * fix(server): stop the crash guard re-throwing combo abort reasons Production crash 2026-08-31 (omniroute.log): on a client disconnect, handleDisconnect aborted the combo controller and a late abort listener threw the abort reason on an empty stack: Error [AbortError]: hedge-cancelled at ... AbortController.abort ... handleDisconnect file:///.../src/shared/utils/httpClientAbortGuard.mjs:130 throw err; isClientAbortError() only knew Node's stream codes and "aborted", so shouldSwallowUncaught() said false and the guard re-threw, taking the whole server down. - Port upstream's AbortError line (name "AbortError" + abort-flavoured message) so request_signal_aborted / DOMException aborts are absorbed. - Add an exact-message match for the combo abort reasons from open-sse/services/combo/comboAbortReasons.ts ("hedge-cancelled", "combo-per-model-timeout"), name-agnostic because the raw reason is a plain Error that only gets name="AbortError" stamped on the way out. A losing hedge / stalled target is never a server fault. Inlined so this .mjs stays dependency-free for scripts/dev/run-next.mjs. Tests: port upstream's guard tests, add the exact crash shape, a child-process replay of the crash (dies pre-fix, survives post-fix), a genuine-error case that must still crash, and a sync check against comboAbortReasons.ts. The child-process helper passes a file:// URL, not a bare path, so the tests run on Windows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 90c9bce) * fix(chatCore): stop executeWithUpstreamStartTimeout leaking its abortPromise listener Root cause of the 2026-08-31 production exit (Error [AbortError]: hedge-cancelled), verified by mapping the crash frames in .build/next/server/chunks/13721.js back to this file: - The abortPromise abort listener registered on the long-lived client / stream signal was never removed in the finally block (only abortListener and timeoutAbortListener were), so every executor attempt (and every retry) leaked one listener onto that signal. - Promise.race only subscribes to abortPromise/timeoutPromise once the array literal has been evaluated. When execute() threw synchronously the race never ran, abortPromise was orphaned, and the next hedge cancellation / client disconnect aborted the signal with the string reason streamHandler.ts forwards; createAbortError() rebuilt it as an AbortError-named Error and rejected a promise nothing awaited. That unhandledRejection reached the process crash guard, which re-threw it as an uncaughtException and exited with code 7. Keep a handle to the listener and remove it with the others, and mark the two race-loser promises as handled so a synchronous throw from execute() can never orphan them. Race semantics are unchanged (the race still observes their rejections). Regression tests: (1) a resolving execute leaves the listener count on the client signal unchanged; (2) a synchronously throwing execute leaks no listener and a later abort with the string "hedge-cancelled" produces no unhandledRejection. Both fail against the previous implementation. Note: commit 0796965 (mergeAbortSignals cleanup) is correct listener hygiene but is not on this crash path; this is the fix for the incident. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit e68a50a) * fix(server): absorb raw string abort reasons in the crash guard; document the verified crash path Follow-ups from the adversarial review of 90c9bce: - open-sse/utils/streamHandler.ts aborts the stream controller with a raw string reason (getClientAbortReason / handleDisconnect) and undici rejects with signal.reason verbatim, so a cancellation can reach process level as a bare string. isClientAbortError() returned false for every non-object, which would still have exited the process. Absorb the combo abort reasons and the stream-handler disconnect reasons when they arrive as strings. - Correct the mechanism comment: the 2026-08-31 exit was a leaked upstreamTimeouts.ts abortPromise listener rejecting a promise nothing awaited (unhandledRejection), escalated by this guard, not a listener throwing synchronously. The leak is fixed at the source in the previous commit; this guard remains the last-resort net. - Reword the inlining rationale (plain node launcher, no reliance on type-stripping for the .ts constants module). - Tests: the child-process replay now also exercises the unhandledRejection route with the exact production error shape and with raw string reasons; add unit coverage for string reasons and non-object look-alikes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 696fcc8) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: Beexly <Beexly@users.noreply.github.com>
…leak, MCP bundle deadlocks, sidebar keys, flush-empty-retry, proxy-status and pack-policy tests (#14820) Fixes the reds still present on the tip: client-abort listener leak (#14342 vs #12406), MCP bundle deadlock (quotaCache init cycle via the quotaCacheState leaf, plus the new #14892 proxyLogs -> upstreamStatusCapture -> usage/migrations cycle via the zero-import timingMs leaf), sidebar Model catalog keys, and the stale flush-empty-retry / proxy-status / pack-policy tests. Combo pre-content retry and zh-TW glossary were already fixed on the tip and dropped. Merged tree: 102/102 focused incl. mcp-bundle-startup (red on the tip), typecheck:core clean, open-sse typecheck 0, check:cycles OK, file-size OK.
Summary
Fixes the
Error [AbortError]: hedge-cancelledprocess exit (exit code 7) seen on a production instance on 2026-08-31, and hardens the process crash guard around it.Root cause (mapped from the built chunks back to source and reproduced):
executeWithUpstreamStartTimeoutinopen-sse/handlers/chatCore/upstreamTimeouts.tsregistered a{ once: true }abort listener forabortPromiseon the long-lived client/stream signal and never removed it infinally, so every executor attempt (and every retry) leaked one listener.Promise.raceonly subscribes once the array literal has been evaluated; whenexecute()threw synchronously the race never ran,abortPromisewas orphaned, and the next hedge cancellation / client disconnect aborted the signal with the string reason thatstreamHandler.tsforwards.createAbortError()rebuilt it as anAbortError-named Error and rejected a promise nothing awaited. ThatunhandledRejectionreachedsrc/shared/utils/httpClientAbortGuard.mjs, whose handler re-threw it as anuncaughtException, and the process exited.Commits
fix(sse): stop mergeAbortSignals from leaking abort listeners- the listener-hygiene change from fix(sse): stop mergeAbortSignals from leaking abort listeners #12391 (open-sse/executors/base.ts), included so this branch is self-contained. Note: that change is correct but is not on the crash path above.fix(server): stop the crash guard re-throwing combo abort reasons-isClientAbortError()now absorbsAbortErrors and the combo abort reasons fromcomboAbortReasons.ts(hedge-cancelled,combo-per-model-timeout). Ports the upstream guard tests and makes the child-process tests Windows-portable (afile://URL instead of a bare path in dynamicimport()).fix(chatCore): stop executeWithUpstreamStartTimeout leaking its abortPromise listener- the root-cause fix: keep a handle to the listener and remove it with the others infinally; mark the two race-loser promises as handled so a synchronous throw fromexecute()can never orphan them. Race semantics are unchanged.fix(server): absorb raw string abort reasons in the crash guard; document the verified crash path-streamHandler.tsaborts with raw string reasons and undici rejects withsignal.reasonverbatim, so a cancellation can reach process level as a bare string; the guard now absorbs those too. Comments corrected to describe the verified mechanism.Tests
tests/unit/chatcore-upstream-timeouts.test.ts(+2; both fail against the previous implementation: one asserts no listener growth after a resolvingexecute, one asserts that a synchronously throwingexecutefollowed by a latehedge-cancelledabort produces nounhandledRejection).tests/unit/httpClientAbortGuard.test.mjs(+9), including a real child-process replay of the production shape on both theuncaughtExceptionandunhandledRejectionroutes, and a genuine-error case that must still crash.httpClientAbortGuard18/18,chatcore-upstream-timeouts7/7,executor-base-utils23/23; 127 related unit test files 968/970 (the 2 failures reproduce identically on the base commit: a stale G13 golden and a Windows temp-dir EPERM on cleanup).tsc --noEmit -p open-sse/tsconfig.jsonclean; eslint clean on the changed files.Related
scripts/build/bootstrap-env.mjsresolvesDATA_DIRto%APPDATA%\omniroutewhilesrc/lib/dataPaths.tsprefers a legacy~/.omniroutewhen it exists, so the firstnpm startof a source checkout can auto-generate a newSTORAGE_ENCRYPTION_KEYand orphan every previously encrypted credential. Happy to file it as an issue.🤖 Generated with Claude Code