mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-10-19 05:20:05 +00:00
fix(resilience): keep RELAY_TIMEOUT from killing the server via the crash guard (#15340)
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.
This commit is contained in:
parent
6fed1939ec
commit
f80cba2cde
4 changed files with 184 additions and 19 deletions
1
changelog.d/fixes/15339-relay-timeout-crash-guard.md
Normal file
1
changelog.d/fixes/15339-relay-timeout-crash-guard.md
Normal file
|
|
@ -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))
|
||||
|
|
@ -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.
|
||||
*
|
||||
|
|
|
|||
114
tests/unit/http-client-abort-guard-relay-timeout.test.ts
Normal file
114
tests/unit/http-client-abort-guard-relay-timeout.test.ts
Normal file
|
|
@ -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
|
||||
);
|
||||
});
|
||||
|
|
@ -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 = "";
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue