diff --git a/changelog.d/fixes/12861-direct-fetch-timeout-unhandled-rejection.md b/changelog.d/fixes/12861-direct-fetch-timeout-unhandled-rejection.md new file mode 100644 index 0000000000..3821c75945 --- /dev/null +++ b/changelog.d/fixes/12861-direct-fetch-timeout-unhandled-rejection.md @@ -0,0 +1 @@ +- **fix(resilience):** a recoverable direct-fetch response-start timeout (`DIRECT_RESPONSE_START_TIMEOUT`) could, in a narrow timer/promise-settlement race, escape as an `unhandledRejection` → `uncaughtException` and kill the server process — even though `proxyFetch` already retries this exact condition on a fresh socket. Guarded the timer callback so it can no longer fire against an already-settled attempt, and extended the process-level crash guard (already used by the WS/API-bridge servers) to recognize and swallow this code if it ever escapes anyway. Also installs that same guard in the production server entrypoint (`dist/server-ws.mjs`), which never had it even though the dev server already did ([#12861](https://github.com/diegosouzapw/OmniRoute/issues/12861)) — thanks @insoln diff --git a/open-sse/utils/directResponseStartTimeout.ts b/open-sse/utils/directResponseStartTimeout.ts index c1cf40e408..7325183c02 100644 --- a/open-sse/utils/directResponseStartTimeout.ts +++ b/open-sse/utils/directResponseStartTimeout.ts @@ -87,16 +87,29 @@ export async function directFetchWithBoundedResponseStart( ): Promise { if (!timeoutMs || timeoutMs <= 0) return fetchImpl(input, options); const attemptController = new AbortController(); - const timer = setTimeout( - () => attemptController.abort(createDirectResponseStartTimeout(timeoutMs)), - timeoutMs - ); + // #12861: guards a narrow but real race between the timer macrotask and the + // fetch promise settling. If `fetchImpl` has already resolved/rejected by + // the time this timer fires, aborting now delivers the abort reason to a + // promise nobody is awaiting anymore — Node promotes that to an + // unhandledRejection -> uncaughtException and kills the process. Once the + // attempt has settled, the timer becomes a no-op instead: the caller + // already has its answer, and there's nothing left to abort for. + let settled = false; + const timer = setTimeout(() => { + if (settled) return; + attemptController.abort(createDirectResponseStartTimeout(timeoutMs)); + }, timeoutMs); timer.unref?.(); try { - return await fetchImpl(input, { + const response = await fetchImpl(input, { ...options, signal: mergeAbortSignals(options.signal, attemptController.signal), }); + settled = true; + return response; + } catch (err) { + settled = true; + throw err; } finally { clearTimeout(timer); } diff --git a/scripts/build/assembleStandalone.mjs b/scripts/build/assembleStandalone.mjs index d6e2010ef2..16ee2426c2 100644 --- a/scripts/build/assembleStandalone.mjs +++ b/scripts/build/assembleStandalone.mjs @@ -229,6 +229,15 @@ const EXTRA_MODULE_ENTRIES = [ src: ["scripts", "dev", "responses-ws-proxy.mjs"], dest: ["responses-ws-proxy.mjs"], }, + { + // server-ws.mjs imports ./httpClientAbortGuard.mjs. In the repo that path is + // the scripts/dev shim re-exporting the shared implementation, but the + // assembled bundle has no src/ tree, so ship the real self-contained + // implementation (no relative imports of its own) under the same file name. + label: "http client abort guard (server-ws.mjs dependency)", + src: ["src", "shared", "utils", "httpClientAbortGuard.mjs"], + dest: ["httpClientAbortGuard.mjs"], + }, { label: "ChatGPT Web Codex MCP tunnel entrypoint", src: ["bin", "chatgpt-web-codex-mcp.mjs"], diff --git a/scripts/dev/httpClientAbortGuard.mjs b/scripts/dev/httpClientAbortGuard.mjs index 9fdabf7a61..8c032372d9 100644 --- a/scripts/dev/httpClientAbortGuard.mjs +++ b/scripts/dev/httpClientAbortGuard.mjs @@ -11,6 +11,7 @@ export { isClientAbortError, + isRecoverableUpstreamTimeoutError, shouldSwallowUncaught, attachRequestStreamGuards, installProcessCrashGuard, diff --git a/scripts/dev/standalone-server-ws.mjs b/scripts/dev/standalone-server-ws.mjs index 65fb3ab65a..ec4508f295 100644 --- a/scripts/dev/standalone-server-ws.mjs +++ b/scripts/dev/standalone-server-ws.mjs @@ -9,6 +9,17 @@ import headResponseGuard from "./head-response-guard.cjs"; import { resolveTlsOptions, createServerListener } from "./tls-options.mjs"; import { getMainServerTimeoutConfig } from "./main-server-timeouts.mjs"; import { createSystemdNotifier } from "./systemd-notify.mjs"; +import { installProcessCrashGuard } from "./httpClientAbortGuard.mjs"; + +// Safety net (#12861): this is the actual production entry point (see the +// keepAliveTimeout comment below for why `run-next.mjs`-only fixes don't +// reach real installs). Without this, a client abort OR a recoverable +// upstream-fetch timeout that a retry path already handles (see +// open-sse/utils/directResponseStartTimeout.ts) can surface as an +// unhandledRejection -> uncaughtException and take the whole server down — +// exactly the asymmetry `run-next.mjs` already closed for dev. Benign errors +// are swallowed and logged; genuine bugs still crash loudly. +installProcessCrashGuard(); // systemd sd_notify (Type=notify / WatchdogSec=): this process is the one // whose event loop can freeze (cold /v1/models rebuild), so it must own the diff --git a/src/shared/utils/httpClientAbortGuard.mjs b/src/shared/utils/httpClientAbortGuard.mjs index 41d5b07cb5..9465ff3208 100644 --- a/src/shared/utils/httpClientAbortGuard.mjs +++ b/src/shared/utils/httpClientAbortGuard.mjs @@ -1,7 +1,8 @@ "use strict"; /** - * HTTP client-abort crash guard (#fix-dev-server-aborted). + * HTTP client-abort / recoverable-upstream-timeout crash guard + * (#fix-dev-server-aborted, #12861). * * Node's http.Server turns an 'error' event on an IncomingMessage/ServerResponse * into an uncaughtException (and therefore a process exit) WHENEVER the emitter @@ -16,14 +17,29 @@ * connections + a live WebSocket; stray client-side socket closes during * navigation/HMR were taking the dev server down. * + * Two more categories were added after the 2026-09-14 agnes-cn upstream storm + * produced two sibling escapes in production: an intentional combo hedge + * cancellation (`AbortError: hedge-cancelled` — the sibling leg already won, + * so the cancellation is expected, not a fault) and undici fetch failures + * (`TypeError: fetch failed` with a socket-level code) against a flapping + * upstream. Both are runtime/environmental conditions the request layer + * already handles; neither is a process-fatal logic bug. + * + * A further, unrelated category covers #12861: `directFetchWithBoundedResponseStart`'s + * response-start timeout (`DIRECT_RESPONSE_START_TIMEOUT`) is a *recoverable* + * signal `proxyFetch.ts` already retries on a fresh socket — but a narrow + * timer/promise-settlement race can still deliver its abort reason to a + * promise nobody is awaiting anymore, which otherwise kills the whole process + * over a single upstream stall that the retry path was built to handle. + * * Two layers: * 1. `attachRequestStreamGuards(req, res)` — per-request listeners that absorb * client-abort errors so they never bubble to the process level. Call it * inside every `http.createServer((req, res) => …)` request listener. * 2. `installProcessCrashGuard()` — a last-resort safety net on * `process.on('uncaughtException' | 'unhandledRejection')` that swallows - * the same benign client-abort errors but otherwise preserves the existing - * crash semantics (so genuine bugs still surface). Idempotent. + * the same benign errors but otherwise preserves the existing crash + * semantics (so genuine bugs still surface). Idempotent. * * Kept as a `.mjs` module (no build step) so it is importable both from the * Node-only dev server (`scripts/dev/run-next.mjs`) and from the TypeScript @@ -63,9 +79,100 @@ export function isClientAbortError(err) { } } +/** + * #12861: a recoverable upstream-fetch timeout that `proxyFetch.ts` already + * retries on a fresh socket (see `open-sse/utils/directResponseStartTimeout.ts`). + * A narrow timer/promise-settlement race can still deliver its abort reason to + * a promise nobody is awaiting anymore, which otherwise surfaces here as an + * unhandledRejection/uncaughtException — even though the retry path already + * handles this exact condition and normally logs it as a plain 504. + * + * Kept as a bare string-code check (no import of the `.ts` source of truth) + * because this file has to stay build-free/plain-JS-loadable — see the module + * docstring. `DIRECT_RESPONSE_START_TIMEOUT_CODE` in + * `open-sse/utils/directResponseStartTimeout.ts` is the canonical definition; + * keep this string literal in sync with it. + * + * @param {unknown} err + * @returns {boolean} + */ +export function isRecoverableUpstreamTimeoutError(err) { + // Same reason-shape tolerance as isIntentionalComboAbort: a bare string + // reason rejects waiters with the string itself, not an Error object. + if (err === "DIRECT_RESPONSE_START_TIMEOUT") return true; + if (!err || typeof err !== "object") return false; + return /** @type {NodeJS.ErrnoException} */ (err).code === "DIRECT_RESPONSE_START_TIMEOUT"; +} + +/** + * Intentional combo-leg cancellation. When a combo dispatches hedged targets, + * the losing legs are aborted with a distinctive reason once a sibling wins + * (`hedge-cancelled`) or exceeds its per-model budget (`combo-per-model-timeout`) + * — see `COMBO_HEDGE_CANCELLED_REASON` / `COMBO_PER_MODEL_TIMEOUT_REASON` in + * `open-sse/services/combo/comboAbortReasons.ts` (bare literals duplicated here + * because this file must stay build-free; keep in sync). On 2026-09-14 such a + * cancellation escaped its promise chain and killed production with + * `Error [AbortError]: hedge-cancelled` — the request it belonged to had + * already completed 200 via the winning leg. + * + * Distinct from a *client* abort: only these exact reasons qualify, so an + * AbortError from an unknown subsystem still crashes loudly. + * + * @param {unknown} err + * @returns {boolean} + */ +export function isIntentionalComboAbort(err) { + const reasons = new Set(["hedge-cancelled", "combo-per-model-timeout"]); + // AbortSignal.reason is whatever was handed to abort(): a raw string + // reason rejects waiters with the string itself, not an Error object. + if (typeof err === "string") return reasons.has(err); + if (!err || typeof err !== "object") return false; + const e = /** @type {NodeJS.ErrnoException} */ (err); + if (e.name !== "AbortError") return false; + if (reasons.has(String(e.message))) return true; + const cause = /** @type {{ cause?: unknown }} */ (err).cause; + return typeof cause === "string" && reasons.has(cause); +} + +/** + * A network/IO failure against an upstream or its proxy — undici surfaces it + * as `TypeError: fetch failed` (fixed message; the syscall code rides on + * `cause`) or as an error carrying a `PROXY_UNREACHABLE` / `UND_ERR_*` code. + * On 2026-09-14 one of these (`PROXY_UNREACHABLE` / ECONNRESET to + * api.agnes-ai.cn) escaped as an uncaughtException and killed production. + * The request that triggered the fetch already fails through the normal + * error path; the stray copy delivered to nobody must not be process-fatal. + * + * The "fetch failed" message match is exact on purpose: it is undici's fixed + * wrapping message, so arbitrary TypeErrors still crash loudly. + * + * @param {unknown} err + * @returns {boolean} + */ +export function isUpstreamNetworkError(err) { + if (!err || typeof err !== "object") return false; + const e = /** @type {NodeJS.ErrnoException} */ (err); + if (e.name === "TypeError" && e.message === "fetch failed") return true; + switch (e.code) { + case "PROXY_UNREACHABLE": + case "UND_ERR_SOCKET": + case "UND_ERR_CONNECT_TIMEOUT": + case "UND_ERR_HEADERS_TIMEOUT": + case "UND_ERR_BODY_TIMEOUT": + case "ECONNREFUSED": + case "EHOSTUNREACH": + case "ENETUNREACH": + case "EAI_AGAIN": + return true; + default: + return false; + } +} + /** * Decide whether a process-level uncaughtException/unhandledRejection should be - * swallowed (benign client-abort) or allowed to surface (genuine bug). + * swallowed (benign client-abort, or a recoverable upstream timeout that a + * retry path already handles — #12861) or allowed to surface (genuine bug). * * Pure + exported so it can be unit-tested without poking process listeners. * @@ -75,7 +182,14 @@ export function isClientAbortError(err) { * @returns {boolean} true => swallow (log only), false => re-throw / let crash. */ export function shouldSwallowUncaught(err, origin) { - if (!isClientAbortError(err)) return false; + if ( + !isClientAbortError(err) && + !isRecoverableUpstreamTimeoutError(err) && + !isIntentionalComboAbort(err) && + !isUpstreamNetworkError(err) + ) { + return false; + } // Only swallow when the origin matches what the guard installed for. If some // other subsystem raised it (e.g. a deliberate `throw` in a domain), keep the // existing crash semantics. @@ -131,7 +245,9 @@ export function installProcessCrashGuard(log) { process.on("uncaughtException", (err, origin) => { if (shouldSwallowUncaught(err, origin)) { - logger("warn", "[server] swallowed client-abort uncaughtException:", err?.message ?? err); + // The warn line is the only evidence a swallowed error ever happened; + // pass the full error object so the stack survives. + logger("warn", "[server] swallowed benign uncaughtException:", err); return; } throw err; @@ -139,11 +255,7 @@ export function installProcessCrashGuard(log) { process.on("unhandledRejection", (reason) => { if (shouldSwallowUncaught(reason, "unhandledRejection")) { - logger( - "warn", - "[server] swallowed client-abort unhandledRejection:", - reason?.message ?? reason - ); + logger("warn", "[server] swallowed benign unhandledRejection:", reason); return; } throw reason; diff --git a/tests/unit/direct-response-start-timeout-settled-guard-12861.test.ts b/tests/unit/direct-response-start-timeout-settled-guard-12861.test.ts new file mode 100644 index 0000000000..24053128b1 --- /dev/null +++ b/tests/unit/direct-response-start-timeout-settled-guard-12861.test.ts @@ -0,0 +1,178 @@ +// #12861 — proxyFetch: DIRECT_RESPONSE_START_TIMEOUT escapes as +// unhandledRejection -> uncaughtException, server process exits. +// +// A narrow race: if the timer fires AFTER the wrapped fetch has already +// settled (resolved or rejected) — e.g. the awaiting frame was already torn +// down — aborting the (by-then-irrelevant) AbortController can deliver its +// abort reason to a promise nobody is awaiting anymore, which Node promotes +// to an unhandledRejection -> uncaughtException. These tests use node:test's +// mock timer API to deterministically force exactly that ordering, rather +// than relying on real wall-clock timing (which cannot reliably reproduce a +// race this narrow). +import test, { mock } from "node:test"; +import assert from "node:assert/strict"; +import { + directFetchWithBoundedResponseStart, + isDirectResponseStartTimeout, + resolveDirectHeadersTimeoutMs, +} from "../../open-sse/utils/directResponseStartTimeout.ts"; + +test.afterEach(() => { + mock.timers.reset(); +}); + +test("resolves normally when the fetch settles well before the timeout", async () => { + const response = new Response("ok"); + const result = await directFetchWithBoundedResponseStart( + "http://example.test", + {}, + async () => response, + 30_000 + ); + assert.equal(result, response); +}); + +test("rejects with DIRECT_RESPONSE_START_TIMEOUT when the fetch never settles before the timeout", async () => { + mock.timers.enable({ apis: ["setTimeout"] }); + try { + const fetchImpl = (_input: RequestInfo | URL, options: RequestInit) => + new Promise((_resolve, reject) => { + options.signal?.addEventListener("abort", () => { + reject((options.signal as AbortSignal).reason); + }); + }); + + const pending = directFetchWithBoundedResponseStart( + "http://example.test", + {}, + fetchImpl, + 5_000 + ); + const assertion = assert.rejects(pending, (err: unknown) => { + assert.equal(isDirectResponseStartTimeout(err), true); + return true; + }); + + await Promise.resolve(); + mock.timers.tick(5_000); + await assertion; + } finally { + mock.timers.reset(); + } +}); + +test("#12861: a timer firing AFTER the fetch already settled does not escape as an unhandled rejection", async () => { + // This is the actual race the report describes: `clearTimeout()` runs in + // the `finally` block, but the timer callback has already been dequeued by + // the time it runs, so clearing it has no effect. Node's real timer/ + // microtask scheduler can't be forced into that exact interleaving + // deterministically from a test, so the observable consequence is forced + // directly instead: neuter clearTimeout so the timer fires regardless of + // whether the code "tried" to cancel it, exactly as it would if clearTimeout + // had lost that race. + const realClearTimeout = globalThis.clearTimeout; + const realSetTimeout = globalThis.setTimeout; + globalThis.clearTimeout = (() => {}) as typeof clearTimeout; + + let unhandled: unknown = null; + const onUnhandledRejection = (reason: unknown) => { + unhandled = reason; + }; + process.on("unhandledRejection", onUnhandledRejection); + + try { + const response = new Response("ok"); + // Simulates what a real fetch/undici implementation does internally: some + // async chain tied to the same abort signal that the OUTER caller never + // awaits or attaches a .catch() to (e.g. background body-stream cleanup). + // This is the actual mechanism the issue traces the escaped rejection + // back to — not the outer `await fetchImpl(...)` itself, which normal + // control flow already handles fine. + const fetchImpl = async (_input: RequestInfo | URL, options: RequestInit) => { + const detachedInternalChain = new Promise((_resolve, reject) => { + options.signal?.addEventListener( + "abort", + () => reject((options.signal as AbortSignal).reason), + { once: true } + ); + }); + void detachedInternalChain; + return response; + }; + + const result = await directFetchWithBoundedResponseStart( + "http://example.test", + {}, + fetchImpl, + 10 + ); + assert.equal(result, response); + + // Real timer, real (short) wait — clearTimeout was neutered above, so the + // 10ms timer WILL fire regardless of the `finally` block having "tried" + // to clear it, exactly reproducing the reported race's end state. + await new Promise((resolve) => realSetTimeout(resolve, 50)); + + assert.equal(unhandled, null, "post-settlement timer fire must not produce a rejection"); + } finally { + globalThis.clearTimeout = realClearTimeout; + process.off("unhandledRejection", onUnhandledRejection); + } +}); + +test("#12861: a timer firing AFTER the fetch already rejected (for an unrelated reason) does not escape either", async () => { + const realClearTimeout = globalThis.clearTimeout; + const realSetTimeout = globalThis.setTimeout; + globalThis.clearTimeout = (() => {}) as typeof clearTimeout; + + let unhandled: unknown = null; + const onUnhandledRejection = (reason: unknown) => { + unhandled = reason; + }; + process.on("unhandledRejection", onUnhandledRejection); + + try { + const clientAbortError = Object.assign(new Error("aborted"), { code: "ECONNRESET" }); + const fetchImpl = async (_input: RequestInfo | URL, options: RequestInit) => { + const detachedInternalChain = new Promise((_resolve, reject) => { + options.signal?.addEventListener( + "abort", + () => reject((options.signal as AbortSignal).reason), + { once: true } + ); + }); + void detachedInternalChain; + throw clientAbortError; + }; + + await assert.rejects( + directFetchWithBoundedResponseStart("http://example.test", {}, fetchImpl, 10), + clientAbortError + ); + + await new Promise((resolve) => realSetTimeout(resolve, 50)); + + assert.equal(unhandled, null, "post-settlement timer fire must not produce a rejection"); + } finally { + globalThis.clearTimeout = realClearTimeout; + process.off("unhandledRejection", onUnhandledRejection); + } +}); + +test("passes through immediately with no timer when timeoutMs is 0 or negative", async () => { + const response = new Response("ok"); + const result = await directFetchWithBoundedResponseStart( + "http://example.test", + {}, + async () => response, + 0 + ); + assert.equal(result, response); +}); + +test("resolveDirectHeadersTimeoutMs defaults to 30000 and respects OMNIROUTE_DIRECT_HEADERS_TIMEOUT_MS", () => { + assert.equal(resolveDirectHeadersTimeoutMs({}), 30_000); + assert.equal(resolveDirectHeadersTimeoutMs({ OMNIROUTE_DIRECT_HEADERS_TIMEOUT_MS: "5000" }), 5_000); + assert.equal(resolveDirectHeadersTimeoutMs({ OMNIROUTE_DIRECT_HEADERS_TIMEOUT_MS: "" }), 30_000); + assert.equal(resolveDirectHeadersTimeoutMs({ OMNIROUTE_DIRECT_HEADERS_TIMEOUT_MS: "not-a-number" }), 0); +}); diff --git a/tests/unit/http-client-abort-guard-direct-timeout-12861.test.ts b/tests/unit/http-client-abort-guard-direct-timeout-12861.test.ts new file mode 100644 index 0000000000..b50e6fa965 --- /dev/null +++ b/tests/unit/http-client-abort-guard-direct-timeout-12861.test.ts @@ -0,0 +1,144 @@ +// #12861 — the shared process-crash guard (already installed for the +// dev server and the WS/API-bridge servers) needs to also recognize the +// recoverable DIRECT_RESPONSE_START_TIMEOUT code so a stray escaped +// rejection from that path is swallowed and logged instead of taking the +// process down, exactly like a benign client-abort already is. +import test from "node:test"; +import assert from "node:assert/strict"; +import { + isClientAbortError, + isIntentionalComboAbort, + isRecoverableUpstreamTimeoutError, + isUpstreamNetworkError, + shouldSwallowUncaught, +} from "../../src/shared/utils/httpClientAbortGuard.mjs"; + +test("isRecoverableUpstreamTimeoutError recognizes DIRECT_RESPONSE_START_TIMEOUT", () => { + const err = Object.assign(new Error("Direct response did not start within 30000ms"), { + code: "DIRECT_RESPONSE_START_TIMEOUT", + name: "TimeoutError", + }); + assert.equal(isRecoverableUpstreamTimeoutError(err), true); + // A raw string abort reason rejects waiters with the string itself. + assert.equal(isRecoverableUpstreamTimeoutError("DIRECT_RESPONSE_START_TIMEOUT"), true); +}); + +test("isRecoverableUpstreamTimeoutError rejects unrelated error codes", () => { + assert.equal(isRecoverableUpstreamTimeoutError(new Error("boom")), false); + assert.equal( + isRecoverableUpstreamTimeoutError(Object.assign(new Error("x"), { code: "ECONNRESET" })), + false + ); + assert.equal(isRecoverableUpstreamTimeoutError(null), false); + assert.equal(isRecoverableUpstreamTimeoutError(undefined), false); + assert.equal(isRecoverableUpstreamTimeoutError("a string, not an object"), false); +}); + +test("isRecoverableUpstreamTimeoutError does not overlap with isClientAbortError's own codes", () => { + // These two predicates should classify disjoint sets of codes; a + // DIRECT_RESPONSE_START_TIMEOUT is not a client abort and vice versa. + const timeoutErr = { code: "DIRECT_RESPONSE_START_TIMEOUT" }; + assert.equal(isClientAbortError(timeoutErr), false); + assert.equal(isRecoverableUpstreamTimeoutError(timeoutErr), true); + + const abortErr = { code: "ECONNRESET" }; + assert.equal(isClientAbortError(abortErr), true); + assert.equal(isRecoverableUpstreamTimeoutError(abortErr), false); +}); + +test("shouldSwallowUncaught swallows DIRECT_RESPONSE_START_TIMEOUT for uncaughtException and unhandledRejection origins", () => { + const err = Object.assign(new Error("timeout"), { code: "DIRECT_RESPONSE_START_TIMEOUT" }); + assert.equal(shouldSwallowUncaught(err, "uncaughtException"), true); + assert.equal(shouldSwallowUncaught(err, "unhandledRejection"), true); + assert.equal(shouldSwallowUncaught(err, undefined), true); +}); + +test("shouldSwallowUncaught still surfaces genuine errors (no code, no client-abort message)", () => { + const genuineBug = new TypeError("Cannot read properties of undefined"); + assert.equal(shouldSwallowUncaught(genuineBug, "uncaughtException"), false); + assert.equal(shouldSwallowUncaught(genuineBug, "unhandledRejection"), false); +}); + +test("shouldSwallowUncaught still swallows the original client-abort cases (no regression)", () => { + const aborted = new Error("aborted"); + assert.equal(shouldSwallowUncaught(aborted, "uncaughtException"), true); + + const econnreset = Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + assert.equal(shouldSwallowUncaught(econnreset, "unhandledRejection"), true); +}); + +test("isIntentionalComboAbort recognizes hedge-cancelled aborts (message and cause variants)", () => { + const byMessage = Object.assign(new Error("hedge-cancelled"), { name: "AbortError" }); + assert.equal(isIntentionalComboAbort(byMessage), true); + + const byCause = Object.assign(new Error("This operation was aborted"), { + name: "AbortError", + cause: "hedge-cancelled", + }); + assert.equal(isIntentionalComboAbort(byCause), true); + + const perModelTimeout = Object.assign(new Error("combo-per-model-timeout"), { + name: "AbortError", + }); + assert.equal(isIntentionalComboAbort(perModelTimeout), true); +}); + +test("isIntentionalComboAbort rejects client aborts with unknown reasons", () => { + const clientGone = Object.assign(new Error("request_signal_aborted"), { name: "AbortError" }); + assert.equal(isIntentionalComboAbort(clientGone), false); + assert.equal(isIntentionalComboAbort(new Error("hedge-cancelled")), false); + assert.equal(isIntentionalComboAbort(null), false); +}); + +test("isIntentionalComboAbort accepts a bare string abort reason", () => { + // AbortSignal.reason is whatever was handed to abort(); a raw string reason + // rejects waiters with the string itself, not an Error object. + assert.equal(isIntentionalComboAbort("hedge-cancelled"), true); + assert.equal(isIntentionalComboAbort("combo-per-model-timeout"), true); + assert.equal(isIntentionalComboAbort("client-gone"), false); + assert.equal(isIntentionalComboAbort(""), false); +}); + +test("isUpstreamNetworkError recognizes fetch failures and proxy unreachable", () => { + const fetchFailed = Object.assign(new TypeError("fetch failed"), { + cause: Object.assign(new Error("socket disconnected"), { code: "ECONNRESET" }), + }); + assert.equal(isUpstreamNetworkError(fetchFailed), true); + + const proxyUnreachable = Object.assign(new TypeError("fetch failed"), { + code: "PROXY_UNREACHABLE", + }); + assert.equal(isUpstreamNetworkError(proxyUnreachable), true); + + const undiciSocket = Object.assign(new Error("other side closed"), { code: "UND_ERR_SOCKET" }); + assert.equal(isUpstreamNetworkError(undiciSocket), true); +}); + +test("isUpstreamNetworkError rejects genuine errors", () => { + assert.equal(isUpstreamNetworkError(new TypeError("Cannot read properties of undefined")), false); + assert.equal(isUpstreamNetworkError(new Error("fetch failedish")), false); + assert.equal(isUpstreamNetworkError(null), false); + assert.equal(isUpstreamNetworkError("a string"), false); +}); + +test("shouldSwallowUncaught swallows the 2026-09-14 agnes-storm crash shapes", () => { + // 06:11:04 exit 7: hedge cancellation escaped while the sibling leg won. + const hedge = Object.assign(new Error("hedge-cancelled"), { name: "AbortError" }); + assert.equal(shouldSwallowUncaught(hedge, "uncaughtException"), true); + assert.equal(shouldSwallowUncaught(hedge, "unhandledRejection"), true); + + // 06:27:38 exit 7: undici fetch failure against a flapping upstream. + const fetchFailed = Object.assign(new TypeError("fetch failed"), { + code: "PROXY_UNREACHABLE", + }); + assert.equal(shouldSwallowUncaught(fetchFailed, "uncaughtException"), true); + assert.equal(shouldSwallowUncaught(fetchFailed, "unhandledRejection"), true); +}); + +test("shouldSwallowUncaught still surfaces genuine bugs after the extension", () => { + const genuineBug = new TypeError("Cannot read properties of undefined"); + assert.equal(shouldSwallowUncaught(genuineBug, "uncaughtException"), false); + + const unknownAbort = Object.assign(new Error("mystery"), { name: "AbortError" }); + assert.equal(shouldSwallowUncaught(unknownAbort, "unhandledRejection"), false); +}); diff --git a/tests/unit/httpClientAbortGuard-default-logger.test.mjs b/tests/unit/httpClientAbortGuard-default-logger.test.mjs index 8154e4d001..f1eac91990 100644 --- a/tests/unit/httpClientAbortGuard-default-logger.test.mjs +++ b/tests/unit/httpClientAbortGuard-default-logger.test.mjs @@ -19,14 +19,14 @@ test("installProcessCrashGuard() with no argument swallows a client abort withou installProcessCrashGuard(); const handlers = process .listeners("uncaughtException") - .filter((fn) => fn.toString().includes("swallowed client-abort")); + .filter((fn) => fn.toString().includes("swallowed benign uncaughtException")); assert.ok(handlers.length > 0, "guard handler must be registered"); const abortErr = Object.assign(new Error("aborted"), { code: "ECONNRESET" }); // A broken default logger (console is an object, not a function) throws // TypeError here — that is what took the production process down. assert.doesNotThrow(() => handlers[0](abortErr, "uncaughtException")); assert.equal(warnings.length, 1, "the swallowed abort must be logged once"); - assert.ok(String(warnings[0][1]).includes("swallowed client-abort")); + assert.ok(String(warnings[0][1]).includes("swallowed benign uncaughtException")); } finally { console.warn = originalWarn; } diff --git a/tests/unit/httpClientAbortGuard.test.mjs b/tests/unit/httpClientAbortGuard.test.mjs index 293e8ae0ca..9345ccef22 100644 --- a/tests/unit/httpClientAbortGuard.test.mjs +++ b/tests/unit/httpClientAbortGuard.test.mjs @@ -202,3 +202,38 @@ test("installProcessCrashGuard still crashes on genuine errors (no over-swallowi assert.notEqual(status, 0, "genuine errors must keep crash semantics"); assert.doesNotMatch(stdout, /SHOULD_NOT_REACH/); }); + +// A swallowed error is the ONLY evidence it ever happened; logging just +// code/message throws away the stack. The logger must receive the full +// error object so the origin stays diagnosable. +test("installProcessCrashGuard logs the full error object for swallowed errors", async () => { + const guardPath = fileURLToPath( + new URL("../../src/shared/utils/httpClientAbortGuard.mjs", import.meta.url) + ); + const script = ` + const { installProcessCrashGuard } = await import(process.argv[1]); + installProcessCrashGuard((level, ...args) => { + console.log( + "LOGARGS", + level, + args.map((a) => (a instanceof Error ? "Error" : typeof a)).join(",") + ); + }); + process.emit( + "unhandledRejection", + Object.assign(new Error("hedge-cancelled"), { name: "AbortError" }), + Promise.resolve() + ); + `; + const { status, stdout } = await new Promise((resolve, reject) => { + const child = spawn(process.execPath, ["--input-type=module", "-e", script, guardPath], { + stdio: ["ignore", "pipe", "pipe"], + }); + let out = ""; + child.stdout.on("data", (d) => (out += d)); + child.on("close", (status) => resolve({ status, stdout: out })); + child.on("error", reject); + }); + assert.equal(status, 0); + assert.match(stdout, /LOGARGS warn string,Error/); +});