From c8838ce3a2f210ffe4349b553f6f063cff2c5352 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 28 Apr 2026 13:23:30 +0200 Subject: [PATCH 1/5] feat(adapter-utils): add fetchWithRetry helper for adapter HTTP calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generic HTTP retry wrapper with exponential backoff and Retry-After support, intended for adapters that hit external HTTP APIs (e.g. provider model-discovery or token-introspection endpoints) and need to handle transient rate-limit / server errors gracefully. Origin: extracted from #3629 (HearthCore) so the helper can land on its own merit without waiting on the full copilot_local adapter port. - Default retryable statuses: 429, 502, 503, 504 (403 NOT included by default — opt-in for APIs like Copilot that use 403 for TPM rate limits). - Honors Retry-After (seconds or HTTP-date) and caps wait at maxDelayMs. - Combines a per-attempt timeout signal with any caller-provided signal via AbortSignal.any() so external cancellation is always respected. - Returns the final Response; only throws on network/abort errors. Tests cover happy path, retry-then-success, exhaustion, non-retryable status, onRetry callback shape, custom retryableStatuses, Retry-After cap, and per-attempt timeout abort. --- .../adapter-utils/src/server-utils.test.ts | 168 +++++++++++++++++- packages/adapter-utils/src/server-utils.ts | 120 +++++++++++++ 2 files changed, 287 insertions(+), 1 deletion(-) diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 3b497239e8..e26557f9fe 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { applyPaperclipWorkspaceEnv, appendWithByteCap, @@ -12,6 +12,7 @@ import { buildInvocationEnvForLogs, buildPaperclipEnv, DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE, + fetchWithRetry, materializePaperclipSkillCopy, refreshPaperclipWorkspaceEnvForExecution, renderPaperclipWakePrompt, @@ -2686,5 +2687,170 @@ describe("buildPaperclipEnv", () => { const env = buildPaperclipEnv({ id: "agent-1", companyId: "company-1" }); expect(env.PAPERCLIP_API_URL).toBe("http://localhost:3200"); }); + +describe("fetchWithRetry", () => { + let originalFetch: typeof fetch; + + beforeEach(() => { + originalFetch = globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + vi.restoreAllMocks(); + }); + + function makeResponse(status: number, headers: Record = {}) { + return new Response("", { status, headers }); + } + + it("returns the first successful response without retrying", async () => { + const fetchMock = vi.fn().mockResolvedValue(makeResponse(200)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const res = await fetchWithRetry("https://example.test", {}); + + expect(res.status).toBe(200); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("retries on 429 and returns the eventual 200", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(makeResponse(429)) + .mockResolvedValueOnce(makeResponse(200)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const res = await fetchWithRetry( + "https://example.test", + {}, + { baseDelayMs: 1, maxDelayMs: 1 }, + ); + + expect(res.status).toBe(200); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("returns the last failed response after exhausting retries", async () => { + const fetchMock = vi.fn().mockResolvedValue(makeResponse(503)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const res = await fetchWithRetry( + "https://example.test", + {}, + { maxRetries: 2, baseDelayMs: 1, maxDelayMs: 1 }, + ); + + expect(res.status).toBe(503); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it("does not retry on non-retryable status codes", async () => { + const fetchMock = vi.fn().mockResolvedValue(makeResponse(400)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const res = await fetchWithRetry( + "https://example.test", + {}, + { baseDelayMs: 1, maxDelayMs: 1 }, + ); + + expect(res.status).toBe(400); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("invokes onRetry with attempt metadata before each wait", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(makeResponse(429, { "retry-after": "0" })) + .mockResolvedValueOnce(makeResponse(200)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const onRetry = vi.fn(); + + await fetchWithRetry( + "https://example.test", + {}, + { + baseDelayMs: 1, + maxDelayMs: 1, + onRetry, + }, + ); + + expect(onRetry).toHaveBeenCalledTimes(1); + expect(onRetry).toHaveBeenCalledWith( + expect.objectContaining({ + attempt: 1, + maxRetries: 3, + status: 429, + retryAfterHeader: "0", + }), + ); + }); + + it("treats custom retryableStatuses as retryable (e.g. 403)", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(makeResponse(403)) + .mockResolvedValueOnce(makeResponse(200)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const res = await fetchWithRetry( + "https://example.test", + {}, + { baseDelayMs: 1, maxDelayMs: 1, retryableStatuses: [403, 429] }, + ); + + expect(res.status).toBe(200); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("caps Retry-After header value at maxDelayMs", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(makeResponse(429, { "retry-after": "60" })) + .mockResolvedValueOnce(makeResponse(200)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const onRetry = vi.fn(); + const start = Date.now(); + + const res = await fetchWithRetry( + "https://example.test", + {}, + { baseDelayMs: 1, maxDelayMs: 5, onRetry }, + ); + + const elapsed = Date.now() - start; + + expect(res.status).toBe(200); + expect(onRetry).toHaveBeenCalledWith( + expect.objectContaining({ delayMs: 5 }), + ); + expect(elapsed).toBeLessThan(1_000); + }); + + it("aborts the request when the per-attempt timeout fires", async () => { + const fetchMock = vi.fn((_url: string | URL, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + const signal = init?.signal as AbortSignal | undefined; + if (signal) { + signal.addEventListener("abort", () => { + reject(new DOMException("aborted", "AbortError")); + }); + } + }); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + fetchWithRetry( + "https://example.test", + {}, + { maxRetries: 0, timeoutMs: 5 }, + ), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(fetchMock).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index a013ada663..da9af1ba82 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -3529,3 +3529,123 @@ export async function runChildProcess( .catch(reject); }); } + +/** + * Options for `fetchWithRetry`. + */ +export interface FetchRetryOptions { + /** Maximum number of retry attempts (default: 3). 0 = no retries. */ + maxRetries?: number; + /** Base delay in ms before first retry — doubles each attempt (default: 2000). */ + baseDelayMs?: number; + /** Maximum delay cap in ms (default: 30000). */ + maxDelayMs?: number; + /** HTTP status codes that trigger a retry (default: [429, 502, 503, 504]). + * 403 is NOT included by default — pass it explicitly for APIs like Copilot + * that use 403 for TPM rate limits. */ + retryableStatuses?: number[]; + /** Optional callback for retry logging. Called before each retry wait. */ + onRetry?: (info: { + attempt: number; + maxRetries: number; + status: number; + delayMs: number; + retryAfterHeader?: string | null; + }) => void | Promise; + /** Request timeout in ms per attempt (default: no timeout). */ + timeoutMs?: number; +} + +const DEFAULT_RETRYABLE_STATUSES = [429, 502, 503, 504]; + +/** + * Fetch with automatic retry on rate-limit / transient server errors. + * + * - Respects `Retry-After` header (seconds or HTTP-date) when present. + * - Falls back to exponential backoff: baseDelay * 2^attempt, capped at maxDelay. + * - Returns the final Response (successful or last failed attempt). + * - Throws only on network/abort errors, never on HTTP status codes. + * + * Adapters that call external HTTP APIs (e.g. provider model-discovery or + * token-introspection endpoints) should use this instead of bare `fetch()` + * to handle transient rate-limit and server errors gracefully. + */ +export async function fetchWithRetry( + url: string | URL, + init: RequestInit, + options: FetchRetryOptions = {}, +): Promise { + const { + maxRetries = 3, + baseDelayMs = 2000, + maxDelayMs = 30_000, + retryableStatuses = DEFAULT_RETRYABLE_STATUSES, + onRetry, + timeoutMs, + } = options; + + let lastResponse: Response | undefined; + + for (let attempt = 0; attempt <= maxRetries; attempt++) { + let controller: AbortController | undefined; + let timer: ReturnType | undefined; + + if (timeoutMs && timeoutMs > 0) { + controller = new AbortController(); + timer = setTimeout(() => controller!.abort(), timeoutMs); + } + + const signals: AbortSignal[] = []; + if (controller?.signal) signals.push(controller.signal); + if (init.signal) signals.push(init.signal as AbortSignal); + const signal = + signals.length > 1 + ? AbortSignal.any(signals) + : signals[0] ?? undefined; + + try { + lastResponse = await fetch(url, { ...init, signal }); + } finally { + if (timer) clearTimeout(timer); + } + + if (lastResponse.ok) return lastResponse; + + if (!retryableStatuses.includes(lastResponse.status) || attempt >= maxRetries) { + return lastResponse; + } + + const retryAfter = lastResponse.headers.get("retry-after"); + let delayMs = baseDelayMs * Math.pow(2, attempt); + + if (retryAfter) { + const parsed = Number(retryAfter); + if (!Number.isNaN(parsed) && parsed > 0) { + delayMs = parsed * 1000; + } else { + const date = new Date(retryAfter).getTime(); + if (!Number.isNaN(date)) { + delayMs = Math.max(0, date - Date.now()); + } + } + } + + delayMs = Math.min(delayMs, maxDelayMs); + + try { await lastResponse.text(); } catch { /* drain body to free connection */ } + + if (onRetry) { + await onRetry({ + attempt: attempt + 1, + maxRetries, + status: lastResponse.status, + delayMs, + retryAfterHeader: retryAfter, + }); + } + + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + + return lastResponse!; +} From 2668b1fe4208422b482ba31c18fef4f9d88d2c55 Mon Sep 17 00:00:00 2001 From: Michel Tomas Date: Tue, 28 Apr 2026 18:46:53 +0200 Subject: [PATCH 2/5] fix(adapter-utils): treat Retry-After: 0 as retry-immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged `parsed > 0` as a P1 (Retry-After: 0 falls back to exponential backoff). The mechanism they described — `new Date("0")` returning Invalid Date in Node 20+ — does not reproduce: V8 parses `"0"` as year-2000 in local TZ on Node 20.20.2 and 22.22.1, so the date branch coincidentally returns 0 ms via `Math.max(0, past - now)`. The current code is therefore *accidentally correct* on Node 20+ in 2026, but relies on implementation-defined Date parsing per ES spec and would break under a future V8 change or a system clock set before year 2000. Switching `parsed > 0` to `parsed >= 0` makes the intent explicit, matches RFC 7231 §7.1.3 ("0" = "you may retry at once"), and skips the date branch entirely on the common case. Also pins the behavior with two test changes: - Strengthen the existing onRetry test to assert `delayMs: 0`. - New explicit test: Retry-After: 0 with baseDelayMs/maxDelayMs set to 60s — passes only if delayMs is actually 0, not exponential. --- .../adapter-utils/src/server-utils.test.ts | 24 +++++++++++++++++++ packages/adapter-utils/src/server-utils.ts | 2 +- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index e26557f9fe..f3c0109576 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -2687,6 +2687,8 @@ describe("buildPaperclipEnv", () => { const env = buildPaperclipEnv({ id: "agent-1", companyId: "company-1" }); expect(env.PAPERCLIP_API_URL).toBe("http://localhost:3200"); }); + }); +}); describe("fetchWithRetry", () => { let originalFetch: typeof fetch; @@ -2784,11 +2786,33 @@ describe("fetchWithRetry", () => { attempt: 1, maxRetries: 3, status: 429, + delayMs: 0, retryAfterHeader: "0", }), ); }); + it("treats Retry-After: 0 as 'retry immediately' (delayMs: 0)", async () => { + const fetchMock = vi + .fn() + .mockResolvedValueOnce(makeResponse(429, { "retry-after": "0" })) + .mockResolvedValueOnce(makeResponse(200)); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const onRetry = vi.fn(); + + const res = await fetchWithRetry( + "https://example.test", + {}, + { baseDelayMs: 60_000, maxDelayMs: 60_000, onRetry }, + ); + + expect(res.status).toBe(200); + expect(onRetry).toHaveBeenCalledWith( + expect.objectContaining({ delayMs: 0, retryAfterHeader: "0" }), + ); + }); + it("treats custom retryableStatuses as retryable (e.g. 403)", async () => { const fetchMock = vi .fn() diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index da9af1ba82..b7cebd4414 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -3620,7 +3620,7 @@ export async function fetchWithRetry( if (retryAfter) { const parsed = Number(retryAfter); - if (!Number.isNaN(parsed) && parsed > 0) { + if (!Number.isNaN(parsed) && parsed >= 0) { delayMs = parsed * 1000; } else { const date = new Date(retryAfter).getTime();