fix(shared): harden fetchWithRetry — backoff cap, 429, Retry-After, abort handling (#209)

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);
+  });
 });

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,14 +18,42 @@ function sleep(ms: number): Promise<void> {
   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;
 }
 
 /**
  * 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`));