fix(sync): retry transient 5xx errors in fetchWithRetry

scripts/discourse-utils.mjs

@@ -50,19 +50,34 @@ export function atomicWrite(path, content) {
   renameSync(tmp, path);
 }
 
+// Transient server errors worth retrying. 500/502/503/504 are gateway or
+// momentary upstream failures; 429 is handled separately via Retry-After.
+const RETRYABLE_5XX = new Set([500, 502, 503, 504]);
+
 /**
- * Fetch wrapper that retries on HTTP 429 (rate-limited) responses.
- * Reads the `Retry-After` response header; falls back to 60 s when absent.
- * Caps the wait at 120 s to avoid stalling CI runs indefinitely.
+ * Fetch wrapper that retries on HTTP 429 (rate-limited) and transient 5xx
+ * responses (500, 502, 503, 504).
+ *
+ * - 429: reads the `Retry-After` header; falls back to 60 s, capped at 120 s.
+ * - 5xx: exponential backoff (2^attempt seconds), capped at 30 s.
+ *
  * Returns the final Response — caller inspects `res.ok` / `res.status`.
  */
 export async function fetchWithRetry(url, options = {}, maxRetries = 3) {
   for (let attempt = 0; attempt <= maxRetries; attempt++) {
     const res = await fetch(url, options);
-    if (res.status !== 429 || attempt === maxRetries) return res;
-    const header = res.headers.get("Retry-After");
-    const seconds = Math.min(parseInt(header ?? "60", 10) || 60, 120);
-    console.warn(`  Rate-limited (429). Waiting ${seconds}s before retry ${attempt + 1}/${maxRetries}…`);
+    const is429 = res.status === 429;
+    const is5xx = RETRYABLE_5XX.has(res.status);
+    if ((!is429 && !is5xx) || attempt === maxRetries) return res;
+    let seconds;
+    if (is429) {
+      const header = res.headers.get("Retry-After");
+      seconds = Math.min(parseInt(header ?? "60", 10) || 60, 120);
+      console.warn(`  Rate-limited (429). Waiting ${seconds}s before retry ${attempt + 1}/${maxRetries}…`);
+    } else {
+      seconds = Math.min(2 ** attempt, 30);
+      console.warn(`  Server error (${res.status}). Waiting ${seconds}s before retry ${attempt + 1}/${maxRetries}…`);
+    }
     await new Promise((r) => setTimeout(r, seconds * 1000));
   }
   return fetch(url, options); // unreachable; satisfies static analysis

src/test/scripts/discourse-utils.test.ts

@@ -102,14 +102,75 @@ describe("fetchWithRetry", () => {
     expect(fetch).toHaveBeenCalledTimes(1);
   });
 
-  it("returns non-200 non-429 responses without retrying", async () => {
+  it("returns 4xx responses without retrying", async () => {
     const res = new Response("not found", { status: 404 });
     vi.mocked(fetch).mockResolvedValue(res);
     const result = await fetchWithRetry("https://example.com");
     expect(result.status).toBe(404);
     expect(fetch).toHaveBeenCalledTimes(1);
   });
 
+  it("retries on 500 and returns success when retry succeeds", async () => {
+    vi.useFakeTimers();
+    const err500 = new Response("server error", { status: 500 });
+    const ok200 = new Response("ok", { status: 200 });
+    vi.mocked(fetch)
+      .mockResolvedValueOnce(err500)
+      .mockResolvedValueOnce(ok200);
+
+    const promise = fetchWithRetry("https://example.com", {}, 3);
+    await vi.runAllTimersAsync();
+    const result = await promise;
+    expect(result.status).toBe(200);
+    expect(fetch).toHaveBeenCalledTimes(2);
+  });
+
+  it("retries on 502 and returns success when retry succeeds", async () => {
+    vi.useFakeTimers();
+    const err502 = new Response("bad gateway", { status: 502 });
+    const ok200 = new Response("ok", { status: 200 });
+    vi.mocked(fetch)
+      .mockResolvedValueOnce(err502)
+      .mockResolvedValueOnce(ok200);
+
+    const promise = fetchWithRetry("https://example.com", {}, 3);
+    await vi.runAllTimersAsync();
+    const result = await promise;
+    expect(result.status).toBe(200);
+    expect(fetch).toHaveBeenCalledTimes(2);
+  });
+
+  it("returns the final 5xx after exhausting retries", async () => {
+    vi.useFakeTimers();
+    const err502 = new Response("bad gateway", { status: 502 });
+    vi.mocked(fetch).mockResolvedValue(err502);
+
+    const promise = fetchWithRetry("https://example.com", {}, 2);
+    await vi.runAllTimersAsync();
+    const result = await promise;
+    expect(result.status).toBe(502);
+    // maxRetries=2 means 3 total calls: attempt 0, 1, 2
+    expect(fetch).toHaveBeenCalledTimes(3);
+  });
+
+  it("uses exponential backoff for 5xx retries", async () => {
+    vi.useFakeTimers();
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+    const err500 = new Response("server error", { status: 500 });
+    const ok200 = new Response("ok", { status: 200 });
+    vi.mocked(fetch)
+      .mockResolvedValueOnce(err500)
+      .mockResolvedValueOnce(ok200);
+
+    const promise = fetchWithRetry("https://example.com", {}, 3);
+    await vi.runAllTimersAsync();
+    await promise;
+    // First 5xx retry: attempt=0, backoff = 2^0 = 1s
+    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Server error (500)"));
+    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("1s"));
+    warnSpy.mockRestore();
+  });
+
   it("retries on 429 and returns success when retry succeeds", async () => {
     vi.useFakeTimers();
     const rate429 = new Response("rate limited", {

src/test/scripts/refresh-discussions.test.ts

@@ -309,7 +309,11 @@ describe("fetchTopicPosts pagination", () => {
   // also tear down the localStorage stub that src/test/setup.ts installs for the
   // whole suite via vi.stubGlobal, breaking its own beforeEach.
   const realFetch = globalThis.fetch;
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
   afterEach(() => {
+    vi.useRealTimers();
     vi.stubGlobal("fetch", realFetch);
   });
 
@@ -329,7 +333,9 @@ describe("fetchTopicPosts pagination", () => {
   it("fails the topic when a chunk returns a non-ok status", async () => {
     stubDiscourse(45, () => ({ ok: false, status: 500, json: async () => ({}) }));
 
-    const result = await fetchTopicPosts("1", "https://community.offon.dev/t/a/1");
+    const promise = fetchTopicPosts("1", "https://community.offon.dev/t/a/1");
+    await vi.runAllTimersAsync();
+    const result = await promise;
     expect(result.ok).toBe(false);
     expect(result.reason).toContain("incomplete post list");
     expect(result.reason).toContain("HTTP 500");
@@ -360,15 +366,19 @@ describe("fetchTopicPosts pagination", () => {
         : { ok: false, status: 502, json: async () => ({}) },
     );
 
-    const result = await fetchTopicPosts("1", "https://community.offon.dev/t/a/1");
+    const promise = fetchTopicPosts("1", "https://community.offon.dev/t/a/1");
+    await vi.runAllTimersAsync();
+    const result = await promise;
     expect(result.ok).toBe(false);
     expect(result.posts).toBeUndefined();
   });
 
   it("names the missing post range so the gap is identifiable", async () => {
     stubDiscourse(45, () => ({ ok: false, status: 500, json: async () => ({}) }));
 
-    const result = await fetchTopicPosts("1", "https://community.offon.dev/t/a/1");
+    const promise = fetchTopicPosts("1", "https://community.offon.dev/t/a/1");
+    await vi.runAllTimersAsync();
+    const result = await promise;
     expect(result.reason).toMatch(/posts 21…40/);
   });