From f80cba2cde5bd4ec3ec57febd13f6a12e2ca0756 Mon Sep 17 00:00:00 2001 From: "R. Beltran" Date: Wed, 7 Oct 2026 00:14:09 +0200 Subject: [PATCH] fix(resilience): keep RELAY_TIMEOUT from killing the server via the crash guard (#15340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Onda 5. Este squash entra com o crédito de @Theadd (fix(resilience): keep RELAY_TIMEOUT from killing the server via the crash guard). O board único validou os 28 PRs juntos: file-size OK, changelog OK, i18n drift OK, typecheck:core OK, complexity 2976/3218, cognitive 1358/1437. Testes focados: 545 passaram, 0 falharam, 3 skip em 52 arquivos. Cinco timeouts de resilience-http-e2e eram carga e passaram isolados. O rebaseline de file-size e o adopt de i18n ficam num follow-up, fora deste squash. ⚠️ base-red inherited: #15306 Obrigado, @Theadd. --- .../fixes/15339-relay-timeout-crash-guard.md | 1 + src/shared/utils/httpClientAbortGuard.mjs | 55 ++++++--- ...p-client-abort-guard-relay-timeout.test.ts | 114 ++++++++++++++++++ tests/unit/httpClientAbortGuard.test.mjs | 33 ++++- 4 files changed, 184 insertions(+), 19 deletions(-) create mode 100644 changelog.d/fixes/15339-relay-timeout-crash-guard.md create mode 100644 tests/unit/http-client-abort-guard-relay-timeout.test.ts diff --git a/changelog.d/fixes/15339-relay-timeout-crash-guard.md b/changelog.d/fixes/15339-relay-timeout-crash-guard.md new file mode 100644 index 0000000000..ed2fb647e3 --- /dev/null +++ b/changelog.d/fixes/15339-relay-timeout-crash-guard.md @@ -0,0 +1 @@ +- **fix(resilience):** a pooled relay attempt that exceeds `RELAY_FETCH_TIMEOUT_MS` no longer escapes as an `unhandledRejection` and kills the server (exit code 7) — the process crash guard now classifies `RELAY_TIMEOUT`/`relay_timeout` as a recoverable upstream timeout, like the direct-fetch `DIRECT_RESPONSE_START_TIMEOUT` case ([#PR](https://github.com/diegosouzapw/OmniRoute/pull/PR)) diff --git a/src/shared/utils/httpClientAbortGuard.mjs b/src/shared/utils/httpClientAbortGuard.mjs index 206146f6a8..2117627923 100644 --- a/src/shared/utils/httpClientAbortGuard.mjs +++ b/src/shared/utils/httpClientAbortGuard.mjs @@ -32,6 +32,12 @@ * promise nobody is awaiting anymore, which otherwise kills the whole process * over a single upstream stall that the retry path was built to handle. * + * The same escape reached production on 2026-10-02 (exit code 7) with the + * pooled relay path's per-attempt timeout (`RELAY_TIMEOUT`, + * `open-sse/utils/proxyFetch.ts`): a stray copy of the 504 the combo/executor + * layer already handles took the process down. Both timeout codes are + * classified in `isRecoverableUpstreamTimeoutError` below. + * * Two layers: * 1. `attachRequestStreamGuards(req, res)` — per-request listeners that absorb * client-abort errors so they never bubble to the process level. Call it @@ -116,18 +122,27 @@ 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. + * A recoverable upstream-fetch timeout whose request-layer handling already + * exists, but a stray copy can still reach this guard as an + * unhandledRejection/uncaughtException and must not be process-fatal: * - * Kept as a bare string-code check (no import of the `.ts` source of truth) + * - `DIRECT_RESPONSE_START_TIMEOUT` (#12861): `proxyFetch.ts` retries it on a + * fresh socket (see `open-sse/utils/directResponseStartTimeout.ts`). A narrow + * timer/promise-settlement race can deliver its abort reason to a promise + * nobody is awaiting anymore. + * - `RELAY_TIMEOUT` (2026-10-02 production exit 7): the pooled relay path + * (`open-sse/utils/proxyFetch.ts`, #9100/#9158) fails a hung attempt fast as + * a 504 after `RELAY_FETCH_TIMEOUT_MS` and the combo/executor layer falls + * back. The thrown error escaped its execute chain as a stray rejection and + * this guard re-threw it because the code was not classified here. + * + * Kept as bare string-code checks (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. + * docstring. Canonical definitions: `DIRECT_RESPONSE_START_TIMEOUT_CODE` in + * `open-sse/utils/directResponseStartTimeout.ts`, and the `RELAY_TIMEOUT` / + * `relay_timeout` stamps in `open-sse/utils/proxyFetch.ts` (the same pair + * `src/lib/usage/glmResetCards.ts` matches on). Keep these literals in sync + * with them. * * @param {unknown} err * @returns {boolean} @@ -135,9 +150,20 @@ export function isClientAbortError(err) { 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 === "DIRECT_RESPONSE_START_TIMEOUT" || + err === "RELAY_TIMEOUT" || + err === "relay_timeout" + ) { + return true; + } if (!err || typeof err !== "object") return false; - return /** @type {NodeJS.ErrnoException} */ (err).code === "DIRECT_RESPONSE_START_TIMEOUT"; + const e = /** @type {NodeJS.ErrnoException & { errorCode?: string }} */ (err); + return ( + e.code === "DIRECT_RESPONSE_START_TIMEOUT" || + e.code === "RELAY_TIMEOUT" || + e.errorCode === "relay_timeout" + ); } /** @@ -207,8 +233,9 @@ export function isUpstreamNetworkError(err) { /** * Decide whether a process-level uncaughtException/unhandledRejection should be - * swallowed (benign client-abort, or a recoverable upstream timeout that a - * retry path already handles — #12861) or allowed to surface (genuine bug). + * swallowed (benign client-abort, or a recoverable upstream timeout the + * request layer already handles — #12861 / RELAY_TIMEOUT) or allowed to + * surface (genuine bug). * * Pure + exported so it can be unit-tested without poking process listeners. * diff --git a/tests/unit/http-client-abort-guard-relay-timeout.test.ts b/tests/unit/http-client-abort-guard-relay-timeout.test.ts new file mode 100644 index 0000000000..87fd86cfce --- /dev/null +++ b/tests/unit/http-client-abort-guard-relay-timeout.test.ts @@ -0,0 +1,114 @@ +// 2026-10-02 production exit 7 — RELAY_TIMEOUT (stamped by +// open-sse/utils/proxyFetch.ts on the pooled relay path, #9100/#9158, when an +// attempt exceeds RELAY_FETCH_TIMEOUT_MS) escaped as an unhandledRejection / +// uncaughtException and the process crash guard re-threw it because it is not +// one of its benign classes. Same escape class as #12861 +// (DIRECT_RESPONSE_START_TIMEOUT): the request layer already handles the error +// as a plain 504 / combo fallback, so a stray copy delivered to nobody must +// never be process-fatal. +import test from "node:test"; +import assert from "node:assert/strict"; +import { + isClientAbortError, + isRecoverableUpstreamTimeoutError, + shouldSwallowUncaught, +} from "../../src/shared/utils/httpClientAbortGuard.mjs"; + +function relayTimeout(): Error & { code?: string; errorCode?: string; statusCode?: number } { + // Exact production shape (open-sse/utils/proxyFetch.ts relay loop). + return Object.assign( + new Error("[ProxyFetch] Relay timed out after 25000ms (https://relay.example)"), + { code: "RELAY_TIMEOUT", errorCode: "relay_timeout", statusCode: 504 } + ); +} + +test("isRecoverableUpstreamTimeoutError recognizes the RELAY_TIMEOUT production shape", () => { + assert.equal(isRecoverableUpstreamTimeoutError(relayTimeout()), true); + // errorCode-only variant (lib/usage/glmResetCards.ts accepts the same pair). + assert.equal( + isRecoverableUpstreamTimeoutError( + Object.assign(new Error("relay stalled"), { errorCode: "relay_timeout" }) + ), + true + ); + // A raw string abort reason rejects waiters with the string itself. + assert.equal(isRecoverableUpstreamTimeoutError("RELAY_TIMEOUT"), true); + assert.equal(isRecoverableUpstreamTimeoutError("relay_timeout"), true); +}); + +test("isRecoverableUpstreamTimeoutError still recognizes DIRECT_RESPONSE_START_TIMEOUT (#12861)", () => { + assert.equal( + isRecoverableUpstreamTimeoutError( + Object.assign(new Error("Direct response did not start within 30000ms"), { + code: "DIRECT_RESPONSE_START_TIMEOUT", + }) + ), + true + ); + assert.equal(isRecoverableUpstreamTimeoutError("DIRECT_RESPONSE_START_TIMEOUT"), true); +}); + +test("isRecoverableUpstreamTimeoutError rejects unrelated errors", () => { + assert.equal(isRecoverableUpstreamTimeoutError(new Error("boom")), false); + assert.equal( + isRecoverableUpstreamTimeoutError(Object.assign(new Error("x"), { code: "ECONNRESET" })), + false + ); + assert.equal( + isRecoverableUpstreamTimeoutError(Object.assign(new Error("x"), { errorCode: "relay_timeou" })), + 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 codes", () => { + const timeoutErr = relayTimeout(); + 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 RELAY_TIMEOUT for both process-level origins", () => { + const err = relayTimeout(); + assert.equal(shouldSwallowUncaught(err, "uncaughtException"), true); + assert.equal(shouldSwallowUncaught(err, "unhandledRejection"), true); + assert.equal(shouldSwallowUncaught(err, undefined), true); +}); + +test("shouldSwallowUncaught still surfaces genuine errors after the extension", () => { + const genuineBug = new TypeError("Cannot read properties of undefined"); + assert.equal(shouldSwallowUncaught(genuineBug, "uncaughtException"), false); + assert.equal(shouldSwallowUncaught(genuineBug, "unhandledRejection"), false); + + const unknownCode = Object.assign(new Error("mystery"), { code: "RELAY_TIMEOU" }); + assert.equal(shouldSwallowUncaught(unknownCode, "unhandledRejection"), false); +}); + +test("shouldSwallowUncaught still swallows the pre-existing benign classes (no regression)", () => { + assert.equal(shouldSwallowUncaught(new Error("aborted"), "uncaughtException"), true); + assert.equal( + shouldSwallowUncaught( + Object.assign(new Error("Direct response did not start within 30000ms"), { + code: "DIRECT_RESPONSE_START_TIMEOUT", + }), + "unhandledRejection" + ), + true + ); + assert.equal( + shouldSwallowUncaught( + Object.assign(new Error("hedge-cancelled"), { name: "AbortError" }), + "unhandledRejection" + ), + true + ); + assert.equal( + shouldSwallowUncaught(Object.assign(new TypeError("fetch failed"), {}), "unhandledRejection"), + true + ); +}); diff --git a/tests/unit/httpClientAbortGuard.test.mjs b/tests/unit/httpClientAbortGuard.test.mjs index 2c595ce089..98b7ec5e76 100644 --- a/tests/unit/httpClientAbortGuard.test.mjs +++ b/tests/unit/httpClientAbortGuard.test.mjs @@ -4,7 +4,6 @@ import assert from "node:assert"; import { test } from "node:test"; import { EventEmitter } from "node:events"; import { spawn } from "node:child_process"; -import { fileURLToPath } from "node:url"; import { isClientAbortError, shouldSwallowUncaught, @@ -262,6 +261,30 @@ test("installProcessCrashGuard() survives the real hedge-cancelled uncaughtExcep assert.match(stdout, /ALIVE/); }); +test("installProcessCrashGuard() survives the real RELAY_TIMEOUT unhandledRejection (2026-10-02 exit-7)", async () => { + // Production crash 2026-10-02 (exit code 7): a pooled relay attempt exceeded + // RELAY_FETCH_TIMEOUT_MS and open-sse/utils/proxyFetch.ts threw + // Error: [ProxyFetch] Relay timed out after 25000ms (...) + // code: 'RELAY_TIMEOUT', errorCode: 'relay_timeout', statusCode: 504 + // The rejection escaped its execute chain (stack: proxyFetch -> executor + // execute x2 -> open-sse handler) as an unhandledRejection and the guard + // re-threw it because RELAY_TIMEOUT was not one of its benign classes. + const { status, stdout, stderr } = await runGuardChild(` + const { installProcessCrashGuard } = await import(process.argv[1]); + installProcessCrashGuard(); + const timeoutErr = Object.assign( + new Error("[ProxyFetch] Relay timed out after 25000ms (https://relay.example)"), + { code: "RELAY_TIMEOUT", errorCode: "relay_timeout", statusCode: 504 } + ); + Promise.reject(timeoutErr); + Promise.reject("RELAY_TIMEOUT"); + process.once("exit", (code) => { if (code === 0) console.log("ALIVE"); }); + setTimeout(() => process.exit(0), 50); + `); + assert.equal(status, 0, "child must survive the relay timeout; stderr: " + stderr); + assert.match(stdout, /ALIVE/); +}); + test("installProcessCrashGuard still crashes on genuine errors (no over-swallowing)", async () => { const { status, stdout } = await runGuardChild(` const { installProcessCrashGuard } = await import(process.argv[1]); @@ -295,9 +318,9 @@ test("isClientAbortError absorbs raw string abort reasons from streamHandler (un // 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) - ); + // Same constraint as runGuardChild: the child must receive a file:// URL, + // not a filesystem path (bare "C:\..." breaks import() on Windows). + const guardUrl = new URL("../../src/shared/utils/httpClientAbortGuard.mjs", import.meta.url).href; const script = ` const { installProcessCrashGuard } = await import(process.argv[1]); installProcessCrashGuard((level, ...args) => { @@ -314,7 +337,7 @@ test("installProcessCrashGuard logs the full error object for swallowed errors", ); `; const { status, stdout } = await new Promise((resolve, reject) => { - const child = spawn(process.execPath, ["--input-type=module", "-e", script, guardPath], { + const child = spawn(process.execPath, ["--input-type=module", "-e", script, guardUrl], { stdio: ["ignore", "pipe", "pipe"], }); let out = "";