fix(infra): avoid detached finally unhandled rejection in fetch wrapper (#18014)

Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: 4ec21c89cb
Co-authored-by: Jackten <2895479+Jackten@users.noreply.github.com>
Co-authored-by: sebslight <19554889+sebslight@users.noreply.github.com>
Reviewed-by: @sebslight

CHANGELOG.md

@@ -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.

src/infra/fetch.test.ts

@@ -50,4 +50,136 @@ describe("wrapFetchWithAbortSignal", () => {
 
     await promise;
   });
+
+  it("does not emit an extra unhandled rejection when wrapped fetch rejects", async () => {
+    const unhandled: unknown[] = [];
+    const onUnhandled = (reason: unknown) => {
+      unhandled.push(reason);
+    };
+    process.on("unhandledRejection", onUnhandled);
+
+    const fetchError = new TypeError("fetch failed");
+    const fetchImpl = vi.fn((_input: RequestInfo | URL, _init?: RequestInit) =>
+      Promise.reject(fetchError),
+    );
+    const wrapped = wrapFetchWithAbortSignal(fetchImpl);
+
+    let abortHandler: (() => void) | null = null;
+    const removeEventListener = vi.fn((event: string, handler: () => void) => {
+      if (event === "abort" && abortHandler === handler) {
+        abortHandler = null;
+      }
+    });
+
+    const fakeSignal = {
+      aborted: false,
+      addEventListener: (event: string, handler: () => void) => {
+        if (event === "abort") {
+          abortHandler = handler;
+        }
+      },
+      removeEventListener,
+    } as AbortSignal;
+
+    try {
+      await expect(wrapped("https://example.com", { signal: fakeSignal })).rejects.toBe(fetchError);
+      await Promise.resolve();
+      await new Promise((resolve) => setTimeout(resolve, 0));
+
+      expect(unhandled).toEqual([]);
+      expect(removeEventListener).toHaveBeenCalledOnce();
+    } finally {
+      process.off("unhandledRejection", onUnhandled);
+    }
+  });
+
+  it("cleans up listener and rethrows when fetch throws synchronously", () => {
+    const syncError = new TypeError("sync fetch failure");
+    const fetchImpl = vi.fn(() => {
+      throw syncError;
+    });
+    const wrapped = wrapFetchWithAbortSignal(fetchImpl);
+
+    let abortHandler: (() => void) | null = null;
+    const removeEventListener = vi.fn((event: string, handler: () => void) => {
+      if (event === "abort" && abortHandler === handler) {
+        abortHandler = null;
+      }
+    });
+
+    const fakeSignal = {
+      aborted: false,
+      addEventListener: (event: string, handler: () => void) => {
+        if (event === "abort") {
+          abortHandler = handler;
+        }
+      },
+      removeEventListener,
+    } as AbortSignal;
+
+    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();
+  });
 });

src/infra/fetch.ts

@@ -45,18 +45,32 @@ 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 response = fetchImpl(input, { ...patchedInit, signal: controller.signal });
-    if (typeof signal.removeEventListener === "function") {
-      void response.finally(() => {
+    const cleanup = () => {
+      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 {
+      const response = fetchImpl(input, { ...patchedInit, signal: controller.signal });
+      return response.finally(cleanup);
+    } catch (error) {
+      cleanup();
+      throw error;
     }
-    return response;
   }) as FetchWithPreconnect;
 
   const fetchWithPreconnect = fetchImpl as FetchWithPreconnect;