mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 05:12:16 +03:00
fix(resilience): extend process crash guard to combo hedge cancels and upstream fetch failures (#13636) (#14064)
Closes a real process-killer: the direct-response start timeout could fire after the fetch promise had already settled, and aborting at that point delivered the abort reason to a promise nobody was awaiting — Node promotes that to an `unhandledRejection` → `uncaughtException` and the process dies (#12861). The timer is now a no-op once the attempt has settled, and the same guard is extended to combo hedge cancels and upstream fetch failures. Validated as a combined board first (this PR merged with the 11 siblings of the same batch on the release tip): eslint on every changed file with the suppressions file, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 275 passing / 0 failing focused node:test cases across the 28 test files the batch touches. Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thanks @HouMinXi! Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
This commit is contained in:
@@ -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
|
||||
@@ -87,16 +87,29 @@ export async function directFetchWithBoundedResponseStart(
|
||||
): Promise<Response> {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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"],
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
|
||||
export {
|
||||
isClientAbortError,
|
||||
isRecoverableUpstreamTimeoutError,
|
||||
shouldSwallowUncaught,
|
||||
attachRequestStreamGuards,
|
||||
installProcessCrashGuard,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<Response>((_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<never>((_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<never>((_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);
|
||||
});
|
||||
144
tests/unit/http-client-abort-guard-direct-timeout-12861.test.ts
Normal file
144
tests/unit/http-client-abort-guard-direct-timeout-12861.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user