fix(client): respect Retry-After before retrying

src/core/client.ts

@@ -7,6 +7,24 @@ const DEFAULT_MAX_RETRIES = 5;
 const DEFAULT_BASE_DELAY = 50;
 const DEFAULT_TIMEOUT = 30_000;
 const DEFAULT_USER_AGENT = "registries/0.1.0";
+const MAX_TIMER_DELAY = 2_147_483_647;
+
+function parseRetryAfterValue(header: string | null | undefined): number | undefined {
+  if (!header) return undefined;
+  const trimmed = header.trim();
+  if (!trimmed) return undefined;
+
+  if (/^\d+$/.test(trimmed)) {
+    const seconds = Number(trimmed);
+    return Number.isNaN(seconds) ? undefined : seconds;
+  }
+
+  const timestamp = /[a-z]/i.test(trimmed) ? Date.parse(trimmed) : NaN;
+  if (Number.isNaN(timestamp)) return undefined;
+
+  const seconds = Math.ceil((timestamp - Date.now()) / 1000);
+  return Math.max(seconds, 0);
+}
 
 /**
  * Parse a `Retry-After` header into seconds.
@@ -18,19 +36,19 @@ const DEFAULT_USER_AGENT = "registries/0.1.0";
  * Returns 60 when the header is absent, empty, or unparseable.
  */
 export function parseRetryAfter(header: string | null | undefined): number {
-  if (!header) return 60;
-  const trimmed = header.trim();
-  if (!trimmed) return 60;
+  return parseRetryAfterValue(header) ?? 60;
+}
 
-  if (/^\d+$/.test(trimmed)) return Number(trimmed);
+/** Apply a valid, timer-safe `Retry-After` value or reject an unschedulable delay. */
+export function retryDelayFor(header: string | null | undefined, fallbackDelay: number): number {
+  const retryAfter = parseRetryAfterValue(header);
+  if (retryAfter === undefined) return fallbackDelay;
 
-  const timestamp = /[a-z]/i.test(trimmed) ? Date.parse(trimmed) : NaN;
-  if (!Number.isNaN(timestamp)) {
-    const seconds = Math.ceil((timestamp - Date.now()) / 1000);
-    return Math.max(seconds, 0);
+  const retryAfterDelay = retryAfter * 1000;
+  if (!Number.isFinite(retryAfterDelay) || retryAfterDelay > MAX_TIMER_DELAY) {
+    throw new RateLimitError(retryAfter);
   }
-
-  return 60;
+  return Math.max(fallbackDelay, retryAfterDelay);
 }
 
 /** HTTP client with retry, backoff, rate limiting, and timeout. */
@@ -58,8 +76,16 @@ export class Client {
         const remaining = typeof context.options.retry === "number" ? context.options.retry : 0;
         const attempt = maxRetries - remaining;
         const delay = baseDelay * Math.pow(2, attempt - 1);
-        const jitter = delay * Math.random() * 0.1;
-        return delay + jitter;
+        const jitteredDelay = delay + delay * Math.random() * 0.1;
+        const retryAfter = context.response?.headers.get("Retry-After");
+        try {
+          return retryDelayFor(retryAfter, jitteredDelay);
+        } catch (error) {
+          if (error instanceof RateLimitError && context.response?.status !== 429) {
+            throw new HTTPError(context.response?.status ?? 0, String(context.request), "");
+          }
+          throw error;
+        }
       },
       retryStatusCodes: [408, 409, 425, 429, 500, 502, 503, 504],
       timeout: this.timeout,

test/e2e/client.test.ts

@@ -0,0 +1,58 @@
+import { createServer } from "node:http";
+import { Client } from "../../src/core/client.ts";
+import { HTTPError } from "../../src/core/errors.ts";
+
+describe("Client", () => {
+  it("honors Retry-After without misclassifying server errors", async () => {
+    let requests = 0;
+    let responseStatus: number | null = 429;
+    let retryAfter = "1";
+    const server = createServer((_request, response) => {
+      requests++;
+      if (responseStatus !== null) {
+        const status = responseStatus;
+        responseStatus = null;
+        response.writeHead(status, { Connection: "close", "Retry-After": retryAfter });
+        response.end();
+        return;
+      }
+
+      response.writeHead(200, { Connection: "close", "Content-Type": "application/json" });
+      response.end('{"ok":true}');
+    });
+
+    await new Promise<void>((resolve, reject) => {
+      const onError = (error: Error) => reject(error);
+      server.once("error", onError);
+      server.listen(0, "127.0.0.1", () => {
+        server.off("error", onError);
+        resolve();
+      });
+    });
+
+    try {
+      const address = server.address();
+      if (!address || typeof address === "string") throw new Error("Expected TCP server address");
+      const url = `http://127.0.0.1:${address.port}`;
+
+      const startedAt = performance.now();
+      await new Client({ maxRetries: 1, baseDelay: 10 }).getJSON(url);
+      expect(performance.now() - startedAt).toBeGreaterThanOrEqual(900);
+      expect(requests).toBe(2);
+
+      responseStatus = 503;
+      // 2_147_484 seconds converts to 2_147_484_000 ms, just above Node's timer ceiling.
+      retryAfter = "2147484";
+      const failure = new Client({ maxRetries: 1, baseDelay: 10 }).getJSON(url);
+      await expect(failure).rejects.toMatchObject({
+        name: HTTPError.name,
+        statusCode: 503,
+      });
+      expect(requests).toBe(3);
+    } finally {
+      await new Promise<void>((resolve, reject) => {
+        server.close((error) => (error ? reject(error) : resolve()));
+      });
+    }
+  });
+});

test/unit/client.test.ts

@@ -1,4 +1,5 @@
-import { parseRetryAfter } from "../../src/core/client.ts";
+import { parseRetryAfter, retryDelayFor } from "../../src/core/client.ts";
+import { RateLimitError } from "../../src/core/errors.ts";
 
 describe("parseRetryAfter", () => {
   it("parses numeric seconds", () => {
@@ -60,3 +61,19 @@ describe("parseRetryAfter", () => {
     expect(parseRetryAfter("   ")).toBe(60);
   });
 });
+
+describe("retryDelayFor", () => {
+  it("falls back for invalid values", () => {
+    expect(retryDelayFor("not-a-delay", 75)).toBe(75);
+  });
+
+  it("keeps a longer local backoff", () => {
+    expect(retryDelayFor("0", 75)).toBe(75);
+  });
+
+  it("rejects values above the timer limit", () => {
+    // Node timers accept at most 2_147_483_647 ms, so these straddle that ceiling in seconds.
+    expect(retryDelayFor("2147483", 10)).toBe(2_147_483_000);
+    expect(() => retryDelayFor("2147484", 10)).toThrow(RateLimitError);
+  });
+});