From 7817fe88bd86728d1b6a0b2e8a88fed3d28054bd Mon Sep 17 00:00:00 2001
From: euxaristia <25621994+euxaristia@users.noreply.github.com>
Date: Sun, 12 Apr 2026 07:37:08 -0400
Subject: [PATCH] fix(web-search): stop leaking abort listeners in custom
 provider retry (#611)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`fetchWithRetry` created a fresh `AbortController` per attempt and did:

    signal?.addEventListener('abort', () => controller.abort(), { once: true })

The listener was never removed. Consequences:

- On retry, a second listener was attached to the caller's signal,
  each closing over a different controller.
- After a successful fetch, the listener remained on the caller's
  signal indefinitely, referencing a controller whose work was done.
  For a long-lived caller signal this is a slow leak.
- The `{ once: true }` only helps if the signal actually fires — on
  non-aborted signals the listener stays attached forever.

Replace the manual controller + timer + listener dance with
`AbortSignal.any([signal, AbortSignal.timeout(ms)])`, which the
codebase already uses elsewhere (see src/services/mcp/xaa.ts). This:

- has no user-code listener to leak,
- gives each attempt a fresh independent timeout,
- cleanly distinguishes caller-initiated abort from timeout via
  `signal.aborted` vs `timeoutSignal.aborted` before rewriting the
  error as "Custom search timed out after Ns".

Also resets `lastStatus` per attempt so a 5xx on attempt 0 can't leak
into attempt 1's retry decision, and collapses the two redundant
retry branches (`lastStatus >= 500` and `lastStatus === undefined`)
into one.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/tools/WebSearchTool/providers/custom.ts | 46 +++++++++------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/src/tools/WebSearchTool/providers/custom.ts b/src/tools/WebSearchTool/providers/custom.ts
index 7a19a968..d1db2940 100644
--- a/src/tools/WebSearchTool/providers/custom.ts
+++ b/src/tools/WebSearchTool/providers/custom.ts
@@ -406,20 +406,18 @@ async function fetchWithRetry(url: string, init: RequestInit, signal?: AbortSign
   let lastStatus: number | undefined
 
   for (let attempt = 0; attempt < 2; attempt++) {
-    // Create a timeout that races with the external signal
-    const controller = new AbortController()
-    const timer = setTimeout(() => controller.abort(), timeoutMs)
-
-    // If the external signal is already aborted, forward it
-    if (signal?.aborted) {
-      controller.abort()
-    } else {
-      signal?.addEventListener('abort', () => controller.abort(), { once: true })
-    }
+    // Compose timeout with caller signal via AbortSignal.any so each attempt
+    // has a fresh timeout and we don't leak an abort listener on `signal`
+    // (the previous implementation added one per attempt and never removed
+    // it, and the listener kept a reference to a stale AbortController).
+    const timeoutSignal = AbortSignal.timeout(timeoutMs)
+    const combined = signal
+      ? AbortSignal.any([signal, timeoutSignal])
+      : timeoutSignal
 
+    lastStatus = undefined
     try {
-      const res = await fetch(url, { ...init, signal: controller.signal })
-      clearTimeout(timer)
+      const res = await fetch(url, { ...init, signal: combined })
 
       if (!res.ok) {
         lastStatus = res.status
@@ -427,26 +425,20 @@ async function fetchWithRetry(url: string, init: RequestInit, signal?: AbortSign
       }
       return await res.json()
     } catch (err) {
-      clearTimeout(timer)
       lastErr = err instanceof Error ? err : new Error(String(err))
 
-      // AbortError from timeout
-      if (lastErr.name === 'AbortError' && !signal?.aborted) {
+      // Caller-initiated abort wins — propagate without retry or rewrite.
+      if (signal?.aborted) throw lastErr
+
+      // Timeout (TimeoutError on Bun/Node, or AbortError with timeoutSignal aborted).
+      if (timeoutSignal.aborted) {
         throw new Error(`Custom search timed out after ${timeoutSec}s`)
       }
 
-      // Retry on 5xx or network errors only
-      if (attempt === 0) {
-        if (lastStatus !== undefined && lastStatus >= 500) {
-          await new Promise(r => setTimeout(r, 500))
-          continue
-        }
-        if (lastStatus === undefined) {
-          // Network error — retry
-          await new Promise(r => setTimeout(r, 500))
-          continue
-        }
-        // 4xx — don't retry
+      // Retry once on 5xx or network errors; do not retry 4xx.
+      if (attempt === 0 && (lastStatus === undefined || lastStatus >= 500)) {
+        await new Promise(r => setTimeout(r, 500))
+        continue
       }
       throw lastErr
     }