Merge PR #1337: honor Retry-After header in design variants 429 handler

design/src/variants.ts

@@ -31,30 +31,37 @@ const STYLE_VARIATIONS = [
 
 /**
  * Generate a single variant with retry on 429.
+ *
+ * Exported for testability. Pass `fetchFn` to inject a stubbed fetch in tests;
+ * production code uses the global fetch by default.
  */
-async function generateVariant(
+export async function generateVariant(
   apiKey: string,
   prompt: string,
   outputPath: string,
   size: string,
   quality: string,
+  fetchFn: typeof globalThis.fetch = globalThis.fetch,
 ): Promise<{ path: string; success: boolean; error?: string }> {
   const maxRetries = 3;
+  const MAX_RETRY_AFTER_MS = 60_000; // cap honored Retry-After to bound stalls
   let lastError = "";
+  let skipLeadingDelay = false;
 
   for (let attempt = 0; attempt <= maxRetries; attempt++) {
-    if (attempt > 0) {
+    if (attempt > 0 && !skipLeadingDelay) {
       // Exponential backoff: 2s, 4s, 8s
       const delay = Math.pow(2, attempt) * 1000;
       console.error(`  Rate limited, retrying in ${delay / 1000}s...`);
       await new Promise(r => setTimeout(r, delay));
     }
+    skipLeadingDelay = false;
 
     const controller = new AbortController();
     const timeout = setTimeout(() => controller.abort(), 120_000);
 
     try {
-      const response = await fetch("https://api.openai.com/v1/responses", {
+      const response = await fetchFn("https://api.openai.com/v1/responses", {
         method: "POST",
         headers: {
           "Authorization": `Bearer ${apiKey}`,
@@ -72,6 +79,29 @@ async function generateVariant(
 
       if (response.status === 429) {
         lastError = "Rate limited (429)";
+        const retryAfter = response.headers.get("retry-after");
+        if (retryAfter) {
+          const trimmed = retryAfter.trim();
+          let waitMs: number | null = null;
+          if (/^\d+$/.test(trimmed)) {
+            // delta-seconds (RFC 7231)
+            waitMs = Math.min(Number.parseInt(trimmed, 10) * 1000, MAX_RETRY_AFTER_MS);
+          } else {
+            // HTTP-date (RFC 7231)
+            const dateMs = Date.parse(trimmed);
+            if (!Number.isNaN(dateMs)) {
+              waitMs = Math.min(Math.max(0, dateMs - Date.now()), MAX_RETRY_AFTER_MS);
+            }
+          }
+          if (waitMs !== null) {
+            if (waitMs > 0) {
+              await new Promise(resolve => setTimeout(resolve, waitMs));
+            }
+            // Honored Retry-After (incl. 0 / past date "retry now") — skip the
+            // next iteration's leading exponential sleep so we don't double-wait.
+            skipLeadingDelay = true;
+          }
+        }
         continue;
       }
 

design/test/variants-retry-after.test.ts

@@ -0,0 +1,133 @@
+import { describe, test, expect, beforeEach, afterEach } from "bun:test";
+import fs from "fs";
+import os from "os";
+import path from "path";
+import { generateVariant } from "../src/variants";
+
+// 1x1 transparent PNG, base64 — valid bytes that fs.writeFileSync can write.
+const TINY_PNG_BASE64 =
+  "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=";
+
+function successResponse(): Response {
+  return new Response(
+    JSON.stringify({
+      output: [{ type: "image_generation_call", result: TINY_PNG_BASE64 }],
+    }),
+    { status: 200, headers: { "Content-Type": "application/json" } },
+  );
+}
+
+function rateLimited(retryAfter?: string): Response {
+  const headers: Record<string, string> = {};
+  if (retryAfter !== undefined) headers["Retry-After"] = retryAfter;
+  return new Response("rate limited", { status: 429, headers });
+}
+
+interface CallRecord {
+  ts: number;
+}
+
+function makeStubFetch(
+  responses: Response[],
+  calls: CallRecord[],
+): typeof globalThis.fetch {
+  let idx = 0;
+  return (async (_input: any, _init?: any) => {
+    calls.push({ ts: Date.now() });
+    const response = responses[idx];
+    if (!response) throw new Error(`stub fetch: no response for call ${idx + 1}`);
+    idx++;
+    return response;
+  }) as typeof globalThis.fetch;
+}
+
+describe("generateVariant Retry-After handling", () => {
+  let tmpDir: string;
+  let outputPath: string;
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "variants-retry-after-"));
+    outputPath = path.join(tmpDir, "variant.png");
+  });
+
+  afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  test("delta-seconds: honors Retry-After: 1 with no extra leading exponential", async () => {
+    const calls: CallRecord[] = [];
+    const fetchFn = makeStubFetch([rateLimited("1"), successResponse()], calls);
+
+    const result = await generateVariant(
+      "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn,
+    );
+
+    expect(result.success).toBe(true);
+    expect(calls.length).toBe(2);
+    const gap = calls[1].ts - calls[0].ts;
+    // Honored ~1s; should NOT add the 2s leading exponential on top
+    expect(gap).toBeGreaterThanOrEqual(900);
+    expect(gap).toBeLessThan(1700);
+  });
+
+  test("HTTP-date: honors a future date with no extra leading exponential", async () => {
+    const calls: CallRecord[] = [];
+    const future = new Date(Date.now() + 3000).toUTCString();
+    const fetchFn = makeStubFetch([rateLimited(future), successResponse()], calls);
+
+    const result = await generateVariant(
+      "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn,
+    );
+
+    expect(result.success).toBe(true);
+    expect(calls.length).toBe(2);
+    const gap = calls[1].ts - calls[0].ts;
+    expect(gap).toBeGreaterThanOrEqual(2500);
+    expect(gap).toBeLessThan(4500);
+  });
+
+  test("invalid Retry-After (alphanumeric): falls through to exponential", async () => {
+    const calls: CallRecord[] = [];
+    const fetchFn = makeStubFetch([rateLimited("2abc"), successResponse()], calls);
+
+    const result = await generateVariant(
+      "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn,
+    );
+
+    expect(result.success).toBe(true);
+    expect(calls.length).toBe(2);
+    const gap = calls[1].ts - calls[0].ts;
+    // Falls through to existing 2s exponential leading delay
+    expect(gap).toBeGreaterThanOrEqual(1800);
+    expect(gap).toBeLessThan(3000);
+  });
+
+  test("no Retry-After header: falls through to exponential", async () => {
+    const calls: CallRecord[] = [];
+    const fetchFn = makeStubFetch([rateLimited(), successResponse()], calls);
+
+    const result = await generateVariant(
+      "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn,
+    );
+
+    expect(result.success).toBe(true);
+    expect(calls.length).toBe(2);
+    const gap = calls[1].ts - calls[0].ts;
+    expect(gap).toBeGreaterThanOrEqual(1800);
+    expect(gap).toBeLessThan(3000);
+  });
+
+  test("Retry-After: 0 retries immediately, skips leading exponential", async () => {
+    const calls: CallRecord[] = [];
+    const fetchFn = makeStubFetch([rateLimited("0"), successResponse()], calls);
+
+    const result = await generateVariant(
+      "fake-key", "prompt", outputPath, "1024x1024", "high", fetchFn,
+    );
+
+    expect(result.success).toBe(true);
+    expect(calls.length).toBe(2);
+    const gap = calls[1].ts - calls[0].ts;
+    expect(gap).toBeLessThan(500);
+  });
+});