fix(infra): keep fetch cleanup errors from masking results

This commit is contained in:
sebslight
2026-02-16 08:09:40 -05:00
parent 718fa54aac
commit 4ec21c89cb
3 changed files with 73 additions and 1 deletions

View File

@@ -27,6 +27,7 @@ Docs: https://docs.openclaw.ai
- Skills/Security: restrict `download` installer `targetDir` to the per-skill tools directory to prevent arbitrary file writes. Thanks @Adam55A-code.
- Skills/Linux: harden go installer fallback on apt-based systems by handling root/no-sudo environments safely, doing best-effort apt index refresh, and returning actionable errors instead of failing with spawn errors. (#17687) Thanks @mcrolly.
- Web Fetch/Security: cap downloaded response body size before HTML parsing to prevent memory exhaustion from oversized or deeply nested pages. Thanks @xuemian168.
- Infra/Fetch: ensure foreign abort-signal listener cleanup never masks original fetch successes/failures, while still preventing detached-finally unhandled rejection noise in `wrapFetchWithAbortSignal`. Thanks @Jackten.
- Config/Gateway: make sensitive-key whitelist suffix matching case-insensitive while preserving `passwordFile` path exemptions, preventing accidental redaction of non-secret config values like `maxTokens` and IRC password-file paths. (#16042) Thanks @akramcodez.
- Gateway/Config: prevent `config.patch` from falling back to full-array replacement when an object-array patch includes entries without `id`, so partial `agents.list` updates no longer risk deleting unrelated agents. (#17989) Thanks @stakeswky.
- Dev tooling: harden git `pre-commit` hook against option injection from malicious filenames (for example `--force`), preventing accidental staging of ignored files. Thanks @mrthankyou.

View File

@@ -120,4 +120,66 @@ describe("wrapFetchWithAbortSignal", () => {
expect(() => wrapped("https://example.com", { signal: fakeSignal })).toThrow(syncError);
expect(removeEventListener).toHaveBeenCalledOnce();
});
it("preserves original rejection when listener cleanup throws", async () => {
const fetchError = new TypeError("fetch failed");
const cleanupError = new TypeError("cleanup failed");
const fetchImpl = vi.fn((_input: RequestInfo | URL, _init?: RequestInit) =>
Promise.reject(fetchError),
);
const wrapped = wrapFetchWithAbortSignal(fetchImpl);
const removeEventListener = vi.fn(() => {
throw cleanupError;
});
const fakeSignal = {
aborted: false,
addEventListener: (_event: string, _handler: () => void) => {},
removeEventListener,
} as AbortSignal;
await expect(wrapped("https://example.com", { signal: fakeSignal })).rejects.toBe(fetchError);
expect(removeEventListener).toHaveBeenCalledOnce();
});
it("preserves original sync throw when listener cleanup throws", () => {
const syncError = new TypeError("sync fetch failure");
const cleanupError = new TypeError("cleanup failed");
const fetchImpl = vi.fn(() => {
throw syncError;
});
const wrapped = wrapFetchWithAbortSignal(fetchImpl);
const removeEventListener = vi.fn(() => {
throw cleanupError;
});
const fakeSignal = {
aborted: false,
addEventListener: (_event: string, _handler: () => void) => {},
removeEventListener,
} as AbortSignal;
expect(() => wrapped("https://example.com", { signal: fakeSignal })).toThrow(syncError);
expect(removeEventListener).toHaveBeenCalledOnce();
});
it("skips listener cleanup when foreign signal is already aborted", async () => {
const addEventListener = vi.fn();
const removeEventListener = vi.fn();
const fetchImpl = vi.fn(async () => ({ ok: true }) as Response);
const wrapped = wrapFetchWithAbortSignal(fetchImpl);
const fakeSignal = {
aborted: true,
addEventListener,
removeEventListener,
} as AbortSignal;
await wrapped("https://example.com", { signal: fakeSignal });
expect(addEventListener).not.toHaveBeenCalled();
expect(removeEventListener).not.toHaveBeenCalled();
});
});

View File

@@ -45,14 +45,23 @@ export function wrapFetchWithAbortSignal(fetchImpl: typeof fetch): typeof fetch
}
const controller = new AbortController();
const onAbort = bindAbortRelay(controller);
let listenerAttached = false;
if (signal.aborted) {
controller.abort();
} else {
signal.addEventListener("abort", onAbort, { once: true });
listenerAttached = true;
}
const cleanup = () => {
if (typeof signal.removeEventListener === "function") {
if (!listenerAttached || typeof signal.removeEventListener !== "function") {
return;
}
listenerAttached = false;
try {
signal.removeEventListener("abort", onAbort);
} catch {
// Foreign/custom AbortSignal implementations may throw here.
// Never let cleanup mask the original fetch result/error.
}
};
try {