fix: harden shared http helper

2026-04-29 21:09:10 -04:00
parent f5339cadf1
commit f95b974c7e
2 changed files with 120 additions and 14 deletions
--- a/packages/core/src/utils/http.ts
+++ b/packages/core/src/utils/http.ts
@@ -71,6 +71,43 @@ function calculateBackoffDelay(
  return Math.min(exponentialDelay + jitterDelay, 30000); // Cap at 30 seconds
 }

+const MAX_RATE_LIMIT_WAIT_MS = 30_000;
+const MAX_DELTA_RESET_SECONDS = 86_400;
+
+function mergeHeaders(
+  defaultHeaders: Record<string, string>,
+  customHeaders?: Record<string, string>,
+): Record<string, string> {
+  const merged: Record<string, string> = {};
+
+  for (const [key, value] of Object.entries(defaultHeaders)) {
+    merged[key.toLowerCase()] = value;
+  }
+
+  for (const [key, value] of Object.entries(customHeaders ?? {})) {
+    merged[key.toLowerCase()] = value;
+  }
+
+  return merged;
+}
+
+function calculateRateLimitWaitMs(
+  resetHeader: string | null,
+  fallbackWaitMs: number,
+): number {
+  if (!resetHeader) return fallbackWaitMs;
+
+  const resetValue = Number(resetHeader);
+  if (!Number.isFinite(resetValue)) return fallbackWaitMs;
+
+  const waitMs =
+    resetValue <= MAX_DELTA_RESET_SECONDS
+      ? resetValue * 1000
+      : resetValue * 1000 - Date.now();
+
+  return Math.min(Math.max(0, waitMs), MAX_RATE_LIMIT_WAIT_MS);
+}
+
 /** Result type when includeResponseUrl is true */
 export interface FetchHtmlResult {
  html: HTMLString;
@@ -141,13 +178,17 @@ export async function fetchHtml(
      const controller = new AbortController();
      const timeoutId = setTimeout(() => controller.abort(), timeoutMs);

-      const res = await fetch(url, {
-        method: "GET",
-        headers: { ...defaultHeaders, ...opts?.headers },
-        signal: controller.signal,
-      });
-
-      clearTimeout(timeoutId);
+      const res = await (async () => {
+        try {
+          return await fetch(url, {
+            method: "GET",
+            headers: mergeHeaders(defaultHeaders, opts?.headers),
+            signal: controller.signal,
+          });
+        } finally {
+          clearTimeout(timeoutId);
+        }
+      })();

      const rateLimitRemaining = res.headers.get("X-RateLimit-Remaining");
      const rateLimitReset = res.headers.get("X-RateLimit-Reset");
@@ -159,13 +200,14 @@ export async function fetchHtml(
          const resetSeconds = rateLimitReset
            ? Number(rateLimitReset)
            : Number.NaN;
-          const waitMs = Number.isFinite(resetSeconds)
-            ? Math.max(0, resetSeconds * 1000)
-            : calculateBackoffDelay(
-                attempt,
-                retryBaseMs,
-                opts?.jitter ?? Math.random,
-              );
+          const waitMs = calculateRateLimitWaitMs(
+            rateLimitReset,
+            calculateBackoffDelay(
+              attempt,
+              retryBaseMs,
+              opts?.jitter ?? Math.random,
+            ),
+          );

          if (attempt < maxRetries) {
            await delay(waitMs);
--- a/packages/core/test/http.test.ts
+++ b/packages/core/test/http.test.ts
@@ -57,4 +57,68 @@ describe("fetchHtml", () => {
    expect(result.html).toBe("<html></html>");
    expect(result.responseUrl).toBe("https://example.test/final");
  });
+
+  test("rate limit epoch reset uses bounded wait", async () => {
+    process.env.NODE_ENV = "production";
+    const scheduledDelays: number[] = [];
+    const farFutureEpochSeconds = Math.floor(Date.now() / 1000) + 315_360_000;
+    let calls = 0;
+
+    global.fetch = mock(() => {
+      calls += 1;
+      return Promise.resolve({
+        ok: calls > 1,
+        status: calls > 1 ? 200 : 429,
+        url: "https://example.test",
+        headers: {
+          get: (name: string) =>
+            name === "X-RateLimit-Reset" ? String(farFutureEpochSeconds) : null,
+        },
+        text: () => Promise.resolve("<html></html>"),
+      });
+    }) as unknown as typeof fetch;
+    globalThis.setTimeout = mock((handler: TimerHandler, timeout?: number) => {
+      scheduledDelays.push(Number(timeout));
+      if (timeout !== 1_234_567 && typeof handler === "function") {
+        handler();
+      }
+      return 0 as unknown as ReturnType<typeof setTimeout>;
+    }) as unknown as typeof setTimeout;
+    globalThis.clearTimeout = mock(() => {}) as unknown as typeof clearTimeout;
+
+    await fetchHtml("https://example.test", 0, {
+      maxRetries: 1,
+      timeoutMs: 1_234_567,
+    });
+
+    expect(scheduledDelays).toContain(30_000);
+    expect(scheduledDelays).not.toContain(farFutureEpochSeconds * 1000);
+  });
+
+  test("custom Accept header overrides default accept without duplicate casing", async () => {
+    process.env.NODE_ENV = "test";
+    const customAccept = "text/plain";
+    let requestHeaders: HeadersInit | undefined;
+
+    global.fetch = mock((_url: string | URL | Request, init?: RequestInit) => {
+      requestHeaders = init?.headers;
+      return Promise.resolve({
+        ok: true,
+        status: 200,
+        url: "https://example.test",
+        headers: { get: () => null },
+        text: () => Promise.resolve("<html></html>"),
+      });
+    }) as unknown as typeof fetch;
+
+    await fetchHtml("https://example.test", 0, {
+      headers: { Accept: customAccept },
+    });
+
+    expect(requestHeaders).toBeDefined();
+    expect((requestHeaders as Record<string, string>).accept).toBe(
+      customAccept,
+    );
+    expect((requestHeaders as Record<string, string>).Accept).toBeUndefined();
+  });
 });