mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(sse): silence noisy proxy-failure log on caller-initiated abort (#7266)
* fix(sse): silence noisy proxy-failure log on caller-initiated abort The pinned-proxy dispatch path in proxyFetch.ts logged every failure — including a plain caller abort or the caller's own AbortSignal timeout firing — as "[ProxyFetch] Proxy request failed ... fail-closed". A client cancelling its own request is not a proxy transport failure and shouldn't be misreported as one in ops logs/alerting; it still propagates to the caller unchanged (fail-closed behavior is untouched). Co-authored-by: TuyulSpam <287281626+TuyulSpam@users.noreply.github.com> Inspired-by: https://github.com/decolua/9router/pull/2589 * chore(changelog): fragment for #7266 --------- Co-authored-by: TuyulSpam <287281626+TuyulSpam@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
6bb3207912
commit
6f57c88de1
1
changelog.d/fixes/7266-proxyfetch-caller-abort-log.md
Normal file
1
changelog.d/fixes/7266-proxyfetch-caller-abort-log.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(sse):** stop logging a caller-initiated request abort/timeout as a noisy proxy transport failure in `proxyFetch`. (thanks @TuyulSpam)
|
||||
@@ -296,6 +296,19 @@ export function resolveProxyForRequest(targetUrl) {
|
||||
return { source: "direct", proxyUrl: null };
|
||||
}
|
||||
|
||||
/**
|
||||
* A caller-initiated abort/timeout is not a proxy transport failure — it must
|
||||
* not be misreported as one. Prefer `signal.aborted` because
|
||||
* `AbortController.abort(reason)` may surface a custom Error rather than a
|
||||
* standard AbortError/TimeoutError name.
|
||||
* Ported from decolua/9router#2589 (`isCallerAbort`).
|
||||
*/
|
||||
function isCallerAbort(error: unknown, signal: AbortSignal | null | undefined): boolean {
|
||||
if (signal?.aborted === true) return true;
|
||||
const name = (error as { name?: unknown } | null)?.name;
|
||||
return name === "AbortError" || name === "TimeoutError";
|
||||
}
|
||||
|
||||
function getTargetUrl(input) {
|
||||
if (typeof input === "string") return input;
|
||||
if (input && typeof input.url === "string") return input.url;
|
||||
@@ -614,8 +627,12 @@ async function patchedFetch(
|
||||
dispatcher,
|
||||
});
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
console.error(`[ProxyFetch] Proxy request failed (${source}, fail-closed): ${message}`);
|
||||
// A caller abort/timeout must propagate unchanged and without a noisy
|
||||
// "Proxy request failed" log — it's not a proxy transport failure.
|
||||
if (!isCallerAbort(error, options?.signal)) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
console.error(`[ProxyFetch] Proxy request failed (${source}, fail-closed): ${message}`);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
106
tests/unit/proxyfetch-caller-abort-log-suppression.test.ts
Normal file
106
tests/unit/proxyfetch-caller-abort-log-suppression.test.ts
Normal file
@@ -0,0 +1,106 @@
|
||||
/**
|
||||
* Ported from decolua/9router#2589 ("harden proxy routing"): the pinned-proxy
|
||||
* dispatch path in `proxyFetch.ts` logged every failure — including a plain
|
||||
* caller-initiated abort/timeout — as `console.error("[ProxyFetch] Proxy
|
||||
* request failed ... fail-closed")`. A client cancelling its own request (or
|
||||
* its own AbortSignal.timeout firing) is not a proxy transport failure; it
|
||||
* shouldn't be misreported as one in ops logs/alerting.
|
||||
*
|
||||
* This only changes what gets logged — the abort error itself still
|
||||
* propagates to the caller unchanged (fail-closed behavior is untouched).
|
||||
*/
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import http from "node:http";
|
||||
|
||||
import proxyFetch, { runWithProxyContext } from "../../open-sse/utils/proxyFetch.ts";
|
||||
|
||||
async function withHttpServer(handler, fn) {
|
||||
const server = http.createServer(handler);
|
||||
await new Promise((resolve, reject) => {
|
||||
server.once("error", reject);
|
||||
server.listen(0, "127.0.0.1", resolve);
|
||||
});
|
||||
const address = server.address();
|
||||
assert.ok(address && typeof address === "object");
|
||||
try {
|
||||
return await fn(`http://127.0.0.1:${address.port}`);
|
||||
} finally {
|
||||
await new Promise((resolve, reject) => {
|
||||
server.close((error) => (error ? reject(error) : resolve()));
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
test("pinned-proxy dispatch does not log a noisy 'Proxy request failed' error for a caller-initiated abort", async () => {
|
||||
await withHttpServer(
|
||||
(_req, res) => res.end("proxy-reachable"),
|
||||
async (proxyUrl) => {
|
||||
const parsed = new URL(proxyUrl);
|
||||
const originalConsoleError = console.error;
|
||||
const loggedMessages: string[] = [];
|
||||
console.error = (...args: unknown[]) => {
|
||||
loggedMessages.push(args.map(String).join(" "));
|
||||
};
|
||||
|
||||
try {
|
||||
const controller = new AbortController();
|
||||
controller.abort();
|
||||
|
||||
await assert.rejects(
|
||||
runWithProxyContext(
|
||||
{ type: "http", host: parsed.hostname, port: parsed.port },
|
||||
async () =>
|
||||
proxyFetch("https://example.invalid/", {
|
||||
signal: controller.signal,
|
||||
})
|
||||
)
|
||||
);
|
||||
} finally {
|
||||
console.error = originalConsoleError;
|
||||
}
|
||||
|
||||
assert.ok(
|
||||
!loggedMessages.some((m) => m.includes("Proxy request failed")),
|
||||
`expected no 'Proxy request failed' log for a caller abort, got: ${JSON.stringify(loggedMessages)}`
|
||||
);
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
test("pinned-proxy dispatch still logs genuine (non-abort) proxy transport failures", async () => {
|
||||
await withHttpServer(
|
||||
(_req, res) => res.end("proxy-reachable"),
|
||||
async (proxyUrl) => {
|
||||
const parsed = new URL(proxyUrl);
|
||||
const originalConsoleError = console.error;
|
||||
const loggedMessages: string[] = [];
|
||||
console.error = (...args: unknown[]) => {
|
||||
loggedMessages.push(args.map(String).join(" "));
|
||||
};
|
||||
// Inject a throwing undici mock — a real dispatcher-level transport
|
||||
// failure (e.g. tunnel refused), NOT a caller abort — must still log.
|
||||
const throwingUndici = async () => {
|
||||
throw new Error("proxy tunnel refused");
|
||||
};
|
||||
|
||||
try {
|
||||
await assert.rejects(
|
||||
runWithProxyContext(
|
||||
{ type: "http", host: parsed.hostname, port: parsed.port },
|
||||
async () =>
|
||||
proxyFetch("https://example.invalid/", {}, { undiciFetch: throwingUndici })
|
||||
),
|
||||
/proxy tunnel refused/
|
||||
);
|
||||
} finally {
|
||||
console.error = originalConsoleError;
|
||||
}
|
||||
|
||||
assert.ok(
|
||||
loggedMessages.some((m) => m.includes("Proxy request failed")),
|
||||
`expected a 'Proxy request failed' log for a genuine transport failure, got: ${JSON.stringify(loggedMessages)}`
|
||||
);
|
||||
}
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user