diff --git a/packages/shared/src/__tests__/shared.test.ts b/packages/shared/src/__tests__/shared.test.ts index e21301a..909aa56 100644 --- a/packages/shared/src/__tests__/shared.test.ts +++ b/packages/shared/src/__tests__/shared.test.ts @@ -192,4 +192,47 @@ describe('fetchWithRetry', () => { }) ); }); + + it('retries on 429 Too Many Requests', async () => { + let callCount = 0; + vi.stubGlobal('fetch', vi.fn(async () => { + callCount++; + if (callCount === 1) { + return new Response('Rate limited', { status: 429, statusText: 'Too Many Requests' }); + } + return new Response('OK', { status: 200 }); + })); + + const result = await fetchWithRetry('https://example.com', { baseDelayMs: 1 }); + expect(result.ok).toBe(true); + expect(callCount).toBe(2); + }); + + it('does not retry on AbortError and returns it immediately', async () => { + let callCount = 0; + vi.stubGlobal('fetch', vi.fn(async () => { + callCount++; + const e = new Error('The operation was aborted'); + e.name = 'AbortError'; + throw e; + })); + + const result = await fetchWithRetry('https://example.com', { baseDelayMs: 1, maxRetries: 3 }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.name).toBe('AbortError'); + expect(callCount).toBe(1); + }); + + it('preserves the underlying error as cause after exhausting retries', async () => { + const underlying = new Error('ECONNRESET'); + vi.stubGlobal('fetch', vi.fn(async () => { + throw underlying; + })); + + const result = await fetchWithRetry('https://example.com', { baseDelayMs: 1, maxRetries: 2 }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.cause).toBe(underlying); + }); }); diff --git a/packages/shared/src/retry.ts b/packages/shared/src/retry.ts index c8f368a..71e9d52 100644 --- a/packages/shared/src/retry.ts +++ b/packages/shared/src/retry.ts @@ -1,4 +1,4 @@ -import { MAX_RETRIES, BASE_BACKOFF_MS } from './constants.js'; +import { MAX_RETRIES, BASE_BACKOFF_MS, MAX_BACKOFF_MS } from './constants.js'; import type { Logger } from './logger.js'; /** Result type mirroring @civic-source/types to avoid circular dependency */ @@ -18,6 +18,30 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +/** Statuses worth retrying: transient server errors and rate-limiting. */ +function isRetryableStatus(status: number): boolean { + return status === 429 || status >= 500; +} + +/** Exponential backoff for a 1-based attempt, capped at MAX_BACKOFF_MS. */ +function backoffDelayMs(baseDelayMs: number, attempt: number): number { + return Math.min(baseDelayMs * Math.pow(2, attempt - 1), MAX_BACKOFF_MS); +} + +/** + * Parse a `Retry-After` header (delta-seconds or HTTP-date) into milliseconds, + * or null if absent/unparseable. The caller still caps the result. + */ +function retryAfterMs(response: Response): number | null { + const header = response.headers.get('retry-after'); + if (header === null) return null; + const seconds = Number(header); + if (Number.isFinite(seconds)) return Math.max(0, seconds * 1000); + const dateMs = Date.parse(header); + if (!Number.isNaN(dateMs)) return Math.max(0, dateMs - Date.now()); + return null; +} + export interface FetchWithRetryOptions extends RequestInit { maxRetries?: number; baseDelayMs?: number; @@ -25,7 +49,11 @@ export interface FetchWithRetryOptions extends RequestInit { /** * Fetch with exponential backoff retry. - * Retries up to `maxRetries` times on network/server errors. + * + * Retries up to `maxRetries` attempts on network errors and on retryable HTTP + * statuses (429 and 5xx), honoring a `Retry-After` header when present. Backoff + * is capped at MAX_BACKOFF_MS. Aborts (`AbortError`) are NOT retried — a caller + * that cancels (timeout/abort signal) gets the abort back immediately. */ export async function fetchWithRetry( url: string, @@ -46,22 +74,34 @@ export async function fetchWithRetry( const response = await fetch(url, fetchOptions); if (response.ok) return ok(response); - if (response.status >= 500 && attempt < maxRetries) { - const delayMs = baseDelayMs * Math.pow(2, attempt - 1); - logger?.warn('Server error, retrying', { url, status: response.status, attempt, delayMs }); + if (isRetryableStatus(response.status) && attempt < maxRetries) { + const delayMs = Math.min( + Math.max(backoffDelayMs(baseDelayMs, attempt), retryAfterMs(response) ?? 0), + MAX_BACKOFF_MS + ); + logger?.warn('Retryable HTTP status, retrying', { + url, + status: response.status, + attempt, + delayMs, + }); await sleep(delayMs); continue; } return err(new Error(`HTTP ${response.status}: ${response.statusText}`)); } catch (error: unknown) { + // Do not retry deliberate cancellations. + if (error instanceof Error && error.name === 'AbortError') { + return err(error); + } if (attempt < maxRetries) { - const delayMs = baseDelayMs * Math.pow(2, attempt - 1); + const delayMs = backoffDelayMs(baseDelayMs, attempt); logger?.warn('Network error, retrying', { url, attempt, delayMs }); await sleep(delayMs); continue; } const message = error instanceof Error ? error.message : String(error); - return err(new Error(`Network error after ${maxRetries} attempts: ${message}`)); + return err(new Error(`Network error after ${maxRetries} attempts: ${message}`, { cause: error })); } } return err(new Error(`Failed after ${maxRetries} attempts`));