mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-22 06:42:19 +03:00
* 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 07969655147d3236969b38bfb41280ab4fb52b79)
* 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 90c9bce8c474b60c37cc63f4421d50feae4c0ad2)
* 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 079696551 (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 e68a50ad854a1945c23e747da4ef15a820cc148d)
* fix(server): absorb raw string abort reasons in the crash guard; document the verified crash path
Follow-ups from the adversarial review of 90c9bce8c:
- 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 696fcc8fe1b9b9bc43e6e5f5f5e44b619a86e68a)
---------
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>
125 lines
4.3 KiB
TypeScript
125 lines
4.3 KiB
TypeScript
import test from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import { getEventListeners } from "node:events";
|
|
|
|
import {
|
|
createBodyTimeoutError,
|
|
createUpstreamStartTimeoutError,
|
|
createAbortError,
|
|
executeWithUpstreamStartTimeout,
|
|
computeBillableTokens,
|
|
getExecutorTimeoutMs,
|
|
normalizeExecutorResult,
|
|
} from "../../open-sse/handlers/chatCore/upstreamTimeouts.ts";
|
|
|
|
test("error factories set name and message", () => {
|
|
const body = createBodyTimeoutError(1234);
|
|
assert.equal(body.name, "BodyTimeoutError");
|
|
assert.match(body.message, /1234ms/);
|
|
|
|
const start = createUpstreamStartTimeoutError(500, "openai", "gpt-4o");
|
|
assert.equal(start.name, "TimeoutError");
|
|
assert.match(start.message, /openai\/gpt-4o/);
|
|
|
|
const ctrl = new AbortController();
|
|
ctrl.abort("nope");
|
|
const ab = createAbortError(ctrl.signal);
|
|
assert.equal(ab.name, "AbortError");
|
|
});
|
|
|
|
test("computeBillableTokens sums input+output+reasoning (no cache double-count)", () => {
|
|
const total = computeBillableTokens({
|
|
prompt_tokens: 10,
|
|
completion_tokens: 5,
|
|
reasoning_tokens: 2,
|
|
});
|
|
assert.equal(total, 17);
|
|
});
|
|
|
|
test("getExecutorTimeoutMs floors valid values and falls back to default", () => {
|
|
assert.equal(getExecutorTimeoutMs({ getTimeoutMs: () => 1234.9 }), 1234);
|
|
assert.equal(getExecutorTimeoutMs({ getTimeoutMs: () => NaN }), getExecutorTimeoutMs(null));
|
|
assert.ok(Number.isFinite(getExecutorTimeoutMs(null)));
|
|
});
|
|
|
|
test("normalizeExecutorResult wraps bare Response and passes through rich result", () => {
|
|
const r = new Response("x");
|
|
const wrapped = normalizeExecutorResult(r);
|
|
assert.equal(wrapped.response, r);
|
|
assert.equal(wrapped.url, "");
|
|
const rich = normalizeExecutorResult({ response: r, url: "u", headers: { a: "b" } });
|
|
assert.equal(rich.url, "u");
|
|
assert.equal(rich.headers.a, "b");
|
|
});
|
|
|
|
test("normalizeExecutorResult rejects malformed executor output", () => {
|
|
assert.throws(() => normalizeExecutorResult({}), /must contain a Response/);
|
|
assert.throws(
|
|
() => normalizeExecutorResult({ response: "not-a-response" }),
|
|
/must contain a Response/
|
|
);
|
|
});
|
|
test("executeWithUpstreamStartTimeout leaves no abort listener on the client signal after a resolving execute", async () => {
|
|
const client = new AbortController();
|
|
const before = getEventListeners(client.signal, "abort").length;
|
|
const result = await executeWithUpstreamStartTimeout({
|
|
executor: {},
|
|
provider: "test-provider",
|
|
model: "test-model",
|
|
connectionTimeoutMs: 5_000,
|
|
signal: client.signal,
|
|
execute: async () => "ok",
|
|
});
|
|
assert.equal(result, "ok");
|
|
assert.equal(
|
|
getEventListeners(client.signal, "abort").length,
|
|
before,
|
|
"every listener registered for the race must be removed once it settles"
|
|
);
|
|
});
|
|
|
|
test("executeWithUpstreamStartTimeout: a synchronously throwing execute cannot orphan abortPromise (2026-08-31 hedge-cancelled exit)", async () => {
|
|
// Production shape: execute() threw before Promise.race subscribed, so the
|
|
// abortPromise listener stayed on the long-lived client signal with nobody
|
|
// awaiting its rejection. The next hedge cancellation / client disconnect
|
|
// then aborted that signal with a string reason and the rejection became an
|
|
// unhandledRejection: `Error [AbortError]: hedge-cancelled`.
|
|
const client = new AbortController();
|
|
const before = getEventListeners(client.signal, "abort").length;
|
|
await assert.rejects(
|
|
executeWithUpstreamStartTimeout({
|
|
executor: {},
|
|
provider: "test-provider",
|
|
model: "test-model",
|
|
connectionTimeoutMs: 5_000,
|
|
signal: client.signal,
|
|
execute: () => {
|
|
throw new Error("sync failure before the race subscribed");
|
|
},
|
|
}),
|
|
/sync failure before the race subscribed/
|
|
);
|
|
assert.equal(
|
|
getEventListeners(client.signal, "abort").length,
|
|
before,
|
|
"no listener may leak onto the client signal when execute() throws synchronously"
|
|
);
|
|
|
|
let unhandled: unknown = null;
|
|
const onUnhandled = (reason: unknown) => {
|
|
unhandled = reason;
|
|
};
|
|
process.on("unhandledRejection", onUnhandled);
|
|
try {
|
|
client.abort("hedge-cancelled");
|
|
await new Promise((resolve) => setTimeout(resolve, 25));
|
|
} finally {
|
|
process.off("unhandledRejection", onUnhandled);
|
|
}
|
|
assert.equal(
|
|
unhandled,
|
|
null,
|
|
"a late abort must not surface as an unhandledRejection: " + String(unhandled)
|
|
);
|
|
});
|