From be320f72670c406b4efa66cf9c53fa76ad4fcc5d Mon Sep 17 00:00:00 2001 From: WYSIATI Date: Sat, 21 Mar 2026 20:32:41 +0800 Subject: [PATCH 1/3] fix: add retry, timeout, and rate-limit handling to GitHub API scripts Extract a shared `githubRequest` utility in `scripts/github-request.ts` that wraps `fetch()` with: - Exponential backoff retry (3 attempts) for transient errors (ECONNRESET, ECONNREFUSED, EPIPE, ETIMEDOUT, etc.) - Retry on retryable HTTP status codes (408, 429, 500, 502, 503, 504) - Respect for `Retry-After` headers from GitHub's rate limiter - Per-request timeout (30s default) via AbortController - 204 No Content handling - Jitter to avoid thundering herd on concurrent retries Migrate all 4 scripts to use the shared utility: - auto-close-duplicates.ts - backfill-duplicate-comments.ts - lifecycle-comment.ts - sweep.ts Previously, a single ECONNRESET or GitHub 502 would crash these CI scripts with no recovery. This is especially problematic for the sweep and auto-close scripts which make hundreds of sequential API calls during batch processing. --- scripts/auto-close-duplicates.ts | 42 ++--- scripts/backfill-duplicate-comments.ts | 38 ++--- scripts/github-request.ts | 211 +++++++++++++++++++++++++ scripts/lifecycle-comment.ts | 22 +-- scripts/sweep.ts | 37 ++--- 5 files changed, 252 insertions(+), 98 deletions(-) create mode 100644 scripts/github-request.ts diff --git a/scripts/auto-close-duplicates.ts b/scripts/auto-close-duplicates.ts index 2ad3bd3112..9e7fda677b 100644 --- a/scripts/auto-close-duplicates.ts +++ b/scripts/auto-close-duplicates.ts @@ -1,5 +1,7 @@ #!/usr/bin/env bun +import { githubRequest } from "./github-request.ts"; + declare global { var process: { env: Record; @@ -25,27 +27,6 @@ interface GitHubReaction { content: string; } -async function githubRequest(endpoint: string, token: string, method: string = 'GET', body?: any): Promise { - const response = await fetch(`https://api.github.com${endpoint}`, { - method, - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github.v3+json", - "User-Agent": "auto-close-duplicates-script", - ...(body && { "Content-Type": "application/json" }), - }, - ...(body && { body: JSON.stringify(body) }), - }); - - if (!response.ok) { - throw new Error( - `GitHub API request failed: ${response.status} ${response.statusText}` - ); - } - - return response.json(); -} - function extractDuplicateIssueNumber(commentBody: string): number | null { // Try to match #123 format first let match = commentBody.match(/#(\d+)/); @@ -73,24 +54,28 @@ async function closeIssueAsDuplicate( await githubRequest( `/repos/${owner}/${repo}/issues/${issueNumber}`, token, - 'PATCH', { - state: 'closed', - state_reason: 'duplicate', - labels: ['duplicate'] + method: 'PATCH', + body: { + state: 'closed', + state_reason: 'duplicate', + labels: ['duplicate'] + }, } ); await githubRequest( `/repos/${owner}/${repo}/issues/${issueNumber}/comments`, token, - 'POST', { - body: `This issue has been automatically closed as a duplicate of #${duplicateOfNumber}. + method: 'POST', + body: { + body: `This issue has been automatically closed as a duplicate of #${duplicateOfNumber}. If this is incorrect, please re-open this issue or create a new one. 🤖 Generated with [Claude Code](https://claude.ai/code)` + }, } ); @@ -272,6 +257,3 @@ async function autoCloseDuplicates(): Promise { } autoCloseDuplicates().catch(console.error); - -// Make it a module -export {}; diff --git a/scripts/backfill-duplicate-comments.ts b/scripts/backfill-duplicate-comments.ts index f79ab43160..6108a0bc17 100644 --- a/scripts/backfill-duplicate-comments.ts +++ b/scripts/backfill-duplicate-comments.ts @@ -1,5 +1,7 @@ #!/usr/bin/env bun +import { githubRequest } from "./github-request.ts"; + declare global { var process: { env: Record; @@ -23,27 +25,6 @@ interface GitHubComment { user: { type: string; id: number }; } -async function githubRequest(endpoint: string, token: string, method: string = 'GET', body?: any): Promise { - const response = await fetch(`https://api.github.com${endpoint}`, { - method, - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github.v3+json", - "User-Agent": "backfill-duplicate-comments-script", - ...(body && { "Content-Type": "application/json" }), - }, - ...(body && { body: JSON.stringify(body) }), - }); - - if (!response.ok) { - throw new Error( - `GitHub API request failed: ${response.status} ${response.statusText}` - ); - } - - return response.json(); -} - async function triggerDedupeWorkflow( owner: string, repo: string, @@ -59,12 +40,14 @@ async function triggerDedupeWorkflow( await githubRequest( `/repos/${owner}/${repo}/actions/workflows/claude-dedupe-issues.yml/dispatches`, token, - 'POST', { - ref: 'main', - inputs: { - issue_number: issueNumber.toString() - } + method: 'POST', + body: { + ref: 'main', + inputs: { + issue_number: issueNumber.toString() + } + }, } ); } @@ -208,6 +191,3 @@ Environment Variables: } backfillDuplicateComments().catch(console.error); - -// Make it a module -export {}; \ No newline at end of file diff --git a/scripts/github-request.ts b/scripts/github-request.ts new file mode 100644 index 0000000000..ad41b4023f --- /dev/null +++ b/scripts/github-request.ts @@ -0,0 +1,211 @@ +/** + * Shared GitHub API request utility with retry, timeout, and rate-limit handling. + * + * All scripts in this directory make unauthenticated or token-authenticated + * calls to the GitHub REST API. Transient failures (ECONNRESET, 502, 429, …) + * are common in CI and long-running batch jobs. This module wraps `fetch()` + * with exponential-backoff retry so every caller gets resilience for free. + */ + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface GitHubRequestOptions { + /** HTTP method (default: GET) */ + method?: string; + /** JSON body — will be serialised automatically */ + body?: unknown; + /** Per-request timeout in ms (default: 30 000) */ + timeoutMs?: number; + /** Maximum number of retry attempts (default: 3) */ + maxRetries?: number; + /** Custom User-Agent header (default: "claude-code-scripts") */ + userAgent?: string; +} + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +const DEFAULT_TIMEOUT_MS = 30_000; +const DEFAULT_MAX_RETRIES = 3; +const INITIAL_RETRY_DELAY_MS = 500; +const MAX_RETRY_DELAY_MS = 32_000; + +/** Error codes that are safe to retry — the request never reached the server + * or was interrupted mid-flight by a network event. */ +const TRANSIENT_ERROR_CODES = new Set([ + "ECONNRESET", + "ECONNREFUSED", + "EPIPE", + "ETIMEDOUT", + "ENOTFOUND", + "EAI_AGAIN", + "ENETUNREACH", + "EHOSTUNREACH", + "UND_ERR_SOCKET", + "UND_ERR_CONNECT_TIMEOUT", +]); + +/** HTTP status codes that are safe to retry. */ +const RETRYABLE_STATUS_CODES = new Set([ + 408, // Request Timeout + 429, // Too Many Requests (rate-limited) + 500, // Internal Server Error + 502, // Bad Gateway + 503, // Service Unavailable + 504, // Gateway Timeout +]); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function isTransientError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + + const code = (error as NodeJS.ErrnoException).code ?? ""; + if (TRANSIENT_ERROR_CODES.has(code)) return true; + + // Bun-specific socket close message + if (error.message.includes("socket connection was closed unexpectedly")) + return true; + + // AbortError from our own timeout — safe to retry + if (error.name === "AbortError" || error.name === "TimeoutError") return true; + + return false; +} + +function calculateDelay(attempt: number): number { + const base = Math.min( + INITIAL_RETRY_DELAY_MS * Math.pow(2, attempt), + MAX_RETRY_DELAY_MS, + ); + // Add ±25 % jitter to avoid thundering herd + const jitter = base * (0.75 + Math.random() * 0.5); + return Math.round(jitter); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * Parse the `Retry-After` header (seconds or HTTP-date) into milliseconds. + * Returns `null` when the header is missing or un-parseable. + */ +function parseRetryAfter(headers: Headers): number | null { + const value = headers.get("retry-after"); + if (!value) return null; + + // Numeric seconds + const seconds = Number(value); + if (!Number.isNaN(seconds) && seconds > 0 && seconds <= 300) { + return seconds * 1000; + } + + // HTTP-date + const date = Date.parse(value); + if (!Number.isNaN(date)) { + const ms = date - Date.now(); + return ms > 0 && ms <= 300_000 ? ms : null; + } + + return null; +} + +// --------------------------------------------------------------------------- +// Main export +// --------------------------------------------------------------------------- + +/** + * Make a GitHub REST API request with automatic retry on transient failures. + * + * @param endpoint Path starting with `/` — appended to `https://api.github.com`. + * @param token GitHub personal-access or installation token. + * @param options Optional overrides for method, body, timeout, retries, etc. + * @returns Parsed JSON response body (typed as `T`). + * @throws After all retries are exhausted, or on non-retryable errors. + */ +export async function githubRequest( + endpoint: string, + token: string, + options: GitHubRequestOptions = {}, +): Promise { + const { + method = "GET", + body, + timeoutMs = DEFAULT_TIMEOUT_MS, + maxRetries = DEFAULT_MAX_RETRIES, + userAgent = "claude-code-scripts", + } = options; + + const url = `https://api.github.com${endpoint}`; + + for (let attempt = 0; attempt <= maxRetries; attempt++) { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + + try { + const response = await fetch(url, { + method, + signal: controller.signal, + headers: { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github.v3+json", + "User-Agent": userAgent, + ...(body !== undefined && { "Content-Type": "application/json" }), + }, + ...(body !== undefined && { body: JSON.stringify(body) }), + }); + + // ── Non-retryable success or client error ───────────────────── + if (response.ok) { + if (response.status === 204) { + return {} as T; + } + return (await response.json()) as T; + } + + // ── Retryable HTTP status ───────────────────────────────────── + if (RETRYABLE_STATUS_CODES.has(response.status) && attempt < maxRetries) { + const retryAfter = parseRetryAfter(response.headers); + const delay = retryAfter ?? calculateDelay(attempt); + const text = await response.text().catch(() => ""); + console.warn( + `[RETRY] GitHub API ${response.status} on ${method} ${endpoint} — ` + + `retrying in ${Math.round(delay / 1000)}s (attempt ${attempt + 1}/${maxRetries})` + + (text ? `: ${text.slice(0, 200)}` : ""), + ); + await sleep(delay); + continue; + } + + // ── Non-retryable HTTP error ────────────────────────────────── + const text = await response.text().catch(() => response.statusText); + throw new Error(`GitHub API ${response.status}: ${text}`); + } catch (error: unknown) { + // ── Retryable network / timeout error ───────────────────────── + if (isTransientError(error) && attempt < maxRetries) { + const delay = calculateDelay(attempt); + const code = + (error as NodeJS.ErrnoException).code ?? (error as Error).name; + console.warn( + `[RETRY] ${code} on ${method} ${endpoint} — ` + + `retrying in ${Math.round(delay / 1000)}s (attempt ${attempt + 1}/${maxRetries})`, + ); + await sleep(delay); + continue; + } + + throw error; + } finally { + clearTimeout(timer); + } + } + + // Should never reach here, but satisfy TypeScript + throw new Error(`GitHub API request failed after ${maxRetries} retries`); +} diff --git a/scripts/lifecycle-comment.ts b/scripts/lifecycle-comment.ts index 3edbae7c5e..706a96d59c 100644 --- a/scripts/lifecycle-comment.ts +++ b/scripts/lifecycle-comment.ts @@ -4,6 +4,7 @@ // giving the author a heads-up and a chance to respond before auto-close. import { lifecycle } from "./issue-lifecycle.ts"; +import { githubRequest } from "./github-request.ts"; const DRY_RUN = process.argv.includes("--dry-run"); const token = process.env.GITHUB_TOKEN; @@ -31,23 +32,10 @@ if (DRY_RUN) { process.exit(0); } -const response = await fetch( - `https://api.github.com/repos/${repo}/issues/${issueNumber}/comments`, - { - method: "POST", - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github.v3+json", - "Content-Type": "application/json", - "User-Agent": "lifecycle-comment", - }, - body: JSON.stringify({ body }), - } +await githubRequest( + `/repos/${repo}/issues/${issueNumber}/comments`, + token!, + { method: "POST", body: { body } }, ); -if (!response.ok) { - const text = await response.text(); - throw new Error(`GitHub API ${response.status}: ${text}`); -} - console.log(`Commented on #${issueNumber} for label "${label}"`); diff --git a/scripts/sweep.ts b/scripts/sweep.ts index 4709290be3..163d506b94 100644 --- a/scripts/sweep.ts +++ b/scripts/sweep.ts @@ -1,6 +1,7 @@ #!/usr/bin/env bun import { lifecycle, STALE_UPVOTE_THRESHOLD } from "./issue-lifecycle.ts"; +import { githubRequest as _githubRequest } from "./github-request.ts"; // -- @@ -10,34 +11,26 @@ const DRY_RUN = process.argv.includes("--dry-run"); const CLOSE_MESSAGE = (reason: string) => `Closing for now — ${reason}. Please [open a new issue](${NEW_ISSUE}) if this is still relevant.`; -// -- +const _token = (() => { + const t = process.env.GITHUB_TOKEN; + if (!t) throw new Error("GITHUB_TOKEN required"); + return t; +})(); +/** Convenience wrapper: token is read once at startup, 404→empty object. */ async function githubRequest( endpoint: string, method = "GET", - body?: unknown + body?: unknown, ): Promise { - const token = process.env.GITHUB_TOKEN; - if (!token) throw new Error("GITHUB_TOKEN required"); - - const response = await fetch(`https://api.github.com${endpoint}`, { - method, - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github.v3+json", - "User-Agent": "sweep", - ...(body && { "Content-Type": "application/json" }), - }, - ...(body && { body: JSON.stringify(body) }), - }); - - if (!response.ok) { - if (response.status === 404) return {} as T; - const text = await response.text(); - throw new Error(`GitHub API ${response.status}: ${text}`); + try { + return await _githubRequest(endpoint, _token, { method, body }); + } catch (error) { + if (error instanceof Error && error.message.startsWith("GitHub API 404:")) { + return {} as T; + } + throw error; } - - return response.json(); } // -- From b9fdb075ff439777b35b93e534472e0728016b10 Mon Sep 17 00:00:00 2001 From: WYSIATI Date: Sat, 21 Mar 2026 20:38:15 +0800 Subject: [PATCH 2/3] test: add unit tests for github-request retry utility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 24 tests covering: - Basic GET/POST functionality and 204 No Content handling - Non-retryable HTTP errors (401, 403, 422) — no retry, immediate throw - Transient network error retry (ECONNRESET, EPIPE, ETIMEDOUT, ECONNREFUSED, Bun socket close) - Retryable HTTP status codes (429, 500, 502, 503) - Retry exhaustion — throws after max attempts - maxRetries=0 disables all retry - Mixed error scenarios (ECONNRESET → 502 → success) - Request timeout via AbortController - Custom User-Agent header --- scripts/github-request.test.ts | 416 +++++++++++++++++++++++++++++++++ 1 file changed, 416 insertions(+) create mode 100644 scripts/github-request.test.ts diff --git a/scripts/github-request.test.ts b/scripts/github-request.test.ts new file mode 100644 index 0000000000..9a2c084a84 --- /dev/null +++ b/scripts/github-request.test.ts @@ -0,0 +1,416 @@ +import { describe, test, expect, mock, beforeEach, afterEach } from "bun:test"; +import { githubRequest } from "./github-request.ts"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** Build a minimal Response-like object for the mock. */ +function jsonResponse(body: unknown, status = 200, headers: Record = {}): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json", ...headers }, + }); +} + +function textResponse(body: string, status: number, headers: Record = {}): Response { + return new Response(body, { status, headers }); +} + +function networkError(code: string, message = "Connection failed"): Error { + const err = new Error(message) as NodeJS.ErrnoException; + err.code = code; + return err; +} + +// --------------------------------------------------------------------------- +// Mock setup +// --------------------------------------------------------------------------- + +const originalFetch = globalThis.fetch; +let fetchMock: ReturnType; + +beforeEach(() => { + fetchMock = mock(() => Promise.resolve(jsonResponse({ ok: true }))); + globalThis.fetch = fetchMock as unknown as typeof fetch; +}); + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("githubRequest", () => { + // ── Basic functionality ─────────────────────────────────────────────── + + test("makes GET request by default", async () => { + const result = await githubRequest("/repos/test/repo", "test-token"); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(1); + + const [url, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe("https://api.github.com/repos/test/repo"); + expect(opts.method).toBe("GET"); + expect((opts.headers as Record)["Authorization"]).toBe("Bearer test-token"); + }); + + test("sends POST with JSON body", async () => { + await githubRequest("/repos/test/repo/issues", "token", { + method: "POST", + body: { title: "test" }, + }); + + const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(opts.method).toBe("POST"); + expect(JSON.parse(opts.body as string)).toEqual({ title: "test" }); + expect((opts.headers as Record)["Content-Type"]).toBe("application/json"); + }); + + test("handles 204 No Content", async () => { + fetchMock = mock(() => Promise.resolve(new Response(null, { status: 204 }))); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo/labels", "token", { + method: "POST", + body: { labels: ["bug"] }, + }); + expect(result).toEqual({}); + }); + + // ── Error handling ──────────────────────────────────────────────────── + + test("throws on non-retryable HTTP error (422)", async () => { + fetchMock = mock(() => + Promise.resolve(textResponse("Validation failed", 422)), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token"), + ).rejects.toThrow("GitHub API 422"); + + expect(fetchMock).toHaveBeenCalledTimes(1); // no retries + }); + + test("throws on 401 Unauthorized without retry", async () => { + fetchMock = mock(() => + Promise.resolve(textResponse("Bad credentials", 401)), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token"), + ).rejects.toThrow("GitHub API 401"); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("throws on 403 Forbidden without retry", async () => { + fetchMock = mock(() => + Promise.resolve(textResponse("Forbidden", 403)), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token"), + ).rejects.toThrow("GitHub API 403"); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + // ── Retry on transient network errors ───────────────────────────────── + + test("retries on ECONNRESET", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount <= 2) return Promise.reject(networkError("ECONNRESET")); + return Promise.resolve(jsonResponse({ recovered: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 3, + }); + + expect(result).toEqual({ recovered: true }); + expect(fetchMock).toHaveBeenCalledTimes(3); // 2 failures + 1 success + }); + + test("retries on EPIPE", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) return Promise.reject(networkError("EPIPE")); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("retries on ETIMEDOUT", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) return Promise.reject(networkError("ETIMEDOUT")); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("retries on ECONNREFUSED", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) return Promise.reject(networkError("ECONNREFUSED")); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("retries on Bun socket close message", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) + return Promise.reject( + new Error("The socket connection was closed unexpectedly"), + ); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("exhausts retries and throws on persistent ECONNRESET", async () => { + fetchMock = mock(() => Promise.reject(networkError("ECONNRESET"))); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 2 }), + ).rejects.toThrow(); + + expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries + }); + + test("does not retry non-transient errors", async () => { + fetchMock = mock(() => + Promise.reject(new Error("Something unexpected")), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 3 }), + ).rejects.toThrow("Something unexpected"); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + // ── Retry on retryable HTTP status codes ────────────────────────────── + + test("retries on 502 Bad Gateway", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) + return Promise.resolve(textResponse("Bad Gateway", 502)); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("retries on 500 Internal Server Error", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) + return Promise.resolve(textResponse("Internal Server Error", 500)); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("retries on 503 Service Unavailable", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) + return Promise.resolve(textResponse("Unavailable", 503)); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + }); + + test("retries on 429 rate limit", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) + return Promise.resolve( + textResponse("Rate limited", 429, { "retry-after": "1" }), + ); + return Promise.resolve(jsonResponse({ ok: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("exhausts retries on persistent 502", async () => { + fetchMock = mock(() => + Promise.resolve(textResponse("Bad Gateway", 502)), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 2 }), + ).rejects.toThrow("GitHub API 502"); + + expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries + }); + + // ── maxRetries=0 disables retry ─────────────────────────────────────── + + test("maxRetries=0 disables retry for network errors", async () => { + fetchMock = mock(() => Promise.reject(networkError("ECONNRESET"))); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 0 }), + ).rejects.toThrow(); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("maxRetries=0 disables retry for HTTP errors", async () => { + fetchMock = mock(() => + Promise.resolve(textResponse("Bad Gateway", 502)), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 0 }), + ).rejects.toThrow("GitHub API 502"); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + // ── Mixed error scenarios ───────────────────────────────────────────── + + test("retries through mixed ECONNRESET then 502 then success", async () => { + let callCount = 0; + fetchMock = mock(() => { + callCount++; + if (callCount === 1) return Promise.reject(networkError("ECONNRESET")); + if (callCount === 2) + return Promise.resolve(textResponse("Bad Gateway", 502)); + return Promise.resolve(jsonResponse({ finally: "success" })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 3, + }); + expect(result).toEqual({ finally: "success" }); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + // ── Timeout ─────────────────────────────────────────────────────────── + + test("aborts on timeout and retries", async () => { + let callCount = 0; + fetchMock = mock((_url: string, opts: RequestInit) => { + callCount++; + if (callCount === 1) { + // Simulate a slow response — the AbortSignal should fire before this resolves + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => resolve(jsonResponse({ slow: true })), + 5000, + ); + opts.signal?.addEventListener("abort", () => { + clearTimeout(timer); + const err = new Error("The operation was aborted"); + err.name = "AbortError"; + reject(err); + }); + }); + } + return Promise.resolve(jsonResponse({ fast: true })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await githubRequest("/repos/test/repo", "token", { + timeoutMs: 50, // very short timeout to trigger abort + maxRetries: 2, + }); + + expect(result).toEqual({ fast: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + // ── Custom User-Agent ───────────────────────────────────────────────── + + test("uses custom userAgent", async () => { + await githubRequest("/repos/test/repo", "token", { + userAgent: "my-script", + }); + + const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect((opts.headers as Record)["User-Agent"]).toBe("my-script"); + }); + + test("uses default userAgent when not specified", async () => { + await githubRequest("/repos/test/repo", "token"); + + const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect((opts.headers as Record)["User-Agent"]).toBe("claude-code-scripts"); + }); +}); From 0d3725ed58a8e43324190d58d11e856f6fa7e1b5 Mon Sep 17 00:00:00 2001 From: WYSIATI Date: Sat, 21 Mar 2026 20:55:22 +0800 Subject: [PATCH 3/3] refactor: restructure tests and export internal helpers for testability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test architecture improvements: - Export isTransientError, calculateDelay, parseRetryAfter for direct unit testing — follows common practice of testing internal logic independently from the integration layer - Split tests into unit tests (internal helpers) and integration tests (githubRequest end-to-end) - Nest describe blocks by concern: basic requests, non-retryable errors, transient network errors, retryable HTTP codes, maxRetries=0, mixed scenarios, timeout - Use test.each() for parameterized tests — reduces boilerplate for error code and status code coverage - Move beforeEach/afterEach inside the githubRequest describe block - Extract mockFetch() and failThenSucceed() helpers to eliminate repeated globalThis.fetch reassignment Test count: 24 → 52 (added unit tests for internal helpers) --- scripts/github-request.test.ts | 659 +++++++++++++++++---------------- scripts/github-request.ts | 8 +- 2 files changed, 349 insertions(+), 318 deletions(-) diff --git a/scripts/github-request.test.ts b/scripts/github-request.test.ts index 9a2c084a84..78fd62a2d2 100644 --- a/scripts/github-request.test.ts +++ b/scripts/github-request.test.ts @@ -1,19 +1,31 @@ import { describe, test, expect, mock, beforeEach, afterEach } from "bun:test"; -import { githubRequest } from "./github-request.ts"; +import { + githubRequest, + isTransientError, + calculateDelay, + parseRetryAfter, +} from "./github-request.ts"; // --------------------------------------------------------------------------- -// Helpers +// Test helpers // --------------------------------------------------------------------------- -/** Build a minimal Response-like object for the mock. */ -function jsonResponse(body: unknown, status = 200, headers: Record = {}): Response { +function jsonResponse( + body: unknown, + status = 200, + headers: Record = {}, +): Response { return new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json", ...headers }, }); } -function textResponse(body: string, status: number, headers: Record = {}): Response { +function textResponse( + body: string, + status: number, + headers: Record = {}, +): Response { return new Response(body, { status, headers }); } @@ -24,393 +36,412 @@ function networkError(code: string, message = "Connection failed"): Error { } // --------------------------------------------------------------------------- -// Mock setup +// Unit tests — internal helpers // --------------------------------------------------------------------------- -const originalFetch = globalThis.fetch; -let fetchMock: ReturnType; - -beforeEach(() => { - fetchMock = mock(() => Promise.resolve(jsonResponse({ ok: true }))); - globalThis.fetch = fetchMock as unknown as typeof fetch; -}); - -afterEach(() => { - globalThis.fetch = originalFetch; -}); - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe("githubRequest", () => { - // ── Basic functionality ─────────────────────────────────────────────── - - test("makes GET request by default", async () => { - const result = await githubRequest("/repos/test/repo", "test-token"); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(1); +describe("isTransientError", () => { + test.each([ + "ECONNRESET", + "ECONNREFUSED", + "EPIPE", + "ETIMEDOUT", + "ENOTFOUND", + "EAI_AGAIN", + "ENETUNREACH", + "EHOSTUNREACH", + "UND_ERR_SOCKET", + "UND_ERR_CONNECT_TIMEOUT", + ])("returns true for %s", (code) => { + expect(isTransientError(networkError(code))).toBe(true); + }); - const [url, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; - expect(url).toBe("https://api.github.com/repos/test/repo"); - expect(opts.method).toBe("GET"); - expect((opts.headers as Record)["Authorization"]).toBe("Bearer test-token"); + test("returns true for Bun socket close message", () => { + expect( + isTransientError( + new Error("The socket connection was closed unexpectedly"), + ), + ).toBe(true); }); - test("sends POST with JSON body", async () => { - await githubRequest("/repos/test/repo/issues", "token", { - method: "POST", - body: { title: "test" }, - }); + test("returns true for AbortError", () => { + const err = new Error("aborted"); + err.name = "AbortError"; + expect(isTransientError(err)).toBe(true); + }); - const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; - expect(opts.method).toBe("POST"); - expect(JSON.parse(opts.body as string)).toEqual({ title: "test" }); - expect((opts.headers as Record)["Content-Type"]).toBe("application/json"); + test("returns true for TimeoutError", () => { + const err = new Error("timed out"); + err.name = "TimeoutError"; + expect(isTransientError(err)).toBe(true); }); - test("handles 204 No Content", async () => { - fetchMock = mock(() => Promise.resolve(new Response(null, { status: 204 }))); - globalThis.fetch = fetchMock as unknown as typeof fetch; + test("returns false for generic Error", () => { + expect(isTransientError(new Error("Something unexpected"))).toBe(false); + }); - const result = await githubRequest("/repos/test/repo/labels", "token", { - method: "POST", - body: { labels: ["bug"] }, - }); - expect(result).toEqual({}); + test("returns false for non-Error values", () => { + expect(isTransientError("string error")).toBe(false); + expect(isTransientError(null)).toBe(false); + expect(isTransientError(undefined)).toBe(false); + expect(isTransientError(42)).toBe(false); }); +}); - // ── Error handling ──────────────────────────────────────────────────── +describe("calculateDelay", () => { + test("returns a value in the jitter range for attempt 0", () => { + const delay = calculateDelay(0); + // base = 500ms, jitter range = [375, 625] + expect(delay).toBeGreaterThanOrEqual(375); + expect(delay).toBeLessThanOrEqual(625); + }); - test("throws on non-retryable HTTP error (422)", async () => { - fetchMock = mock(() => - Promise.resolve(textResponse("Validation failed", 422)), - ); - globalThis.fetch = fetchMock as unknown as typeof fetch; + test("increases exponentially", () => { + // attempt 0 base = 500, attempt 3 base = 4000 + // Collect several samples to verify the trend + const lowSamples = Array.from({ length: 10 }, () => calculateDelay(0)); + const highSamples = Array.from({ length: 10 }, () => calculateDelay(3)); + const lowAvg = lowSamples.reduce((a, b) => a + b, 0) / lowSamples.length; + const highAvg = + highSamples.reduce((a, b) => a + b, 0) / highSamples.length; + expect(highAvg).toBeGreaterThan(lowAvg * 3); + }); - await expect( - githubRequest("/repos/test/repo", "token"), - ).rejects.toThrow("GitHub API 422"); + test("caps at MAX_RETRY_DELAY_MS (32s)", () => { + const delay = calculateDelay(20); // 500 * 2^20 would be huge without cap + // max base = 32000, jitter range = [24000, 40000] + expect(delay).toBeLessThanOrEqual(40_000); + }); +}); - expect(fetchMock).toHaveBeenCalledTimes(1); // no retries +describe("parseRetryAfter", () => { + test("parses numeric seconds", () => { + const headers = new Headers({ "retry-after": "5" }); + expect(parseRetryAfter(headers)).toBe(5000); }); - test("throws on 401 Unauthorized without retry", async () => { - fetchMock = mock(() => - Promise.resolve(textResponse("Bad credentials", 401)), - ); - globalThis.fetch = fetchMock as unknown as typeof fetch; + test("returns null for missing header", () => { + expect(parseRetryAfter(new Headers())).toBeNull(); + }); - await expect( - githubRequest("/repos/test/repo", "token"), - ).rejects.toThrow("GitHub API 401"); + test("returns null for zero", () => { + const headers = new Headers({ "retry-after": "0" }); + expect(parseRetryAfter(headers)).toBeNull(); + }); - expect(fetchMock).toHaveBeenCalledTimes(1); + test("returns null for negative values", () => { + const headers = new Headers({ "retry-after": "-10" }); + expect(parseRetryAfter(headers)).toBeNull(); }); - test("throws on 403 Forbidden without retry", async () => { - fetchMock = mock(() => - Promise.resolve(textResponse("Forbidden", 403)), - ); - globalThis.fetch = fetchMock as unknown as typeof fetch; + test("caps at 300 seconds", () => { + const headers = new Headers({ "retry-after": "301" }); + expect(parseRetryAfter(headers)).toBeNull(); + }); - await expect( - githubRequest("/repos/test/repo", "token"), - ).rejects.toThrow("GitHub API 403"); + test("accepts values up to 300 seconds", () => { + const headers = new Headers({ "retry-after": "300" }); + expect(parseRetryAfter(headers)).toBe(300_000); + }); - expect(fetchMock).toHaveBeenCalledTimes(1); + test("returns null for unparseable value", () => { + const headers = new Headers({ "retry-after": "not-a-number" }); + expect(parseRetryAfter(headers)).toBeNull(); }); +}); - // ── Retry on transient network errors ───────────────────────────────── +// --------------------------------------------------------------------------- +// Integration tests — githubRequest +// --------------------------------------------------------------------------- - test("retries on ECONNRESET", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount <= 2) return Promise.reject(networkError("ECONNRESET")); - return Promise.resolve(jsonResponse({ recovered: true })); - }); +describe("githubRequest", () => { + const originalFetch = globalThis.fetch; + let fetchMock: ReturnType; + + /** Replace `globalThis.fetch` with a fresh mock before each test. */ + function mockFetch( + impl: (...args: unknown[]) => Promise, + ): void { + fetchMock = mock(impl); globalThis.fetch = fetchMock as unknown as typeof fetch; + } - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 3, - }); + beforeEach(() => { + mockFetch(() => Promise.resolve(jsonResponse({ ok: true }))); + }); - expect(result).toEqual({ recovered: true }); - expect(fetchMock).toHaveBeenCalledTimes(3); // 2 failures + 1 success + afterEach(() => { + globalThis.fetch = originalFetch; }); - test("retries on EPIPE", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) return Promise.reject(networkError("EPIPE")); - return Promise.resolve(jsonResponse({ ok: true })); - }); - globalThis.fetch = fetchMock as unknown as typeof fetch; + // ── Basic request functionality ─────────────────────────────────────── - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, - }); + describe("basic requests", () => { + test("makes GET request with auth header", async () => { + const result = await githubRequest("/repos/test/repo", "test-token"); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(1); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); + const [url, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe("https://api.github.com/repos/test/repo"); + expect(opts.method).toBe("GET"); + expect((opts.headers as Record)["Authorization"]).toBe( + "Bearer test-token", + ); + }); - test("retries on ETIMEDOUT", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) return Promise.reject(networkError("ETIMEDOUT")); - return Promise.resolve(jsonResponse({ ok: true })); + test("sends POST with JSON body and Content-Type", async () => { + await githubRequest("/repos/test/repo/issues", "token", { + method: "POST", + body: { title: "test" }, + }); + + const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(opts.method).toBe("POST"); + expect(JSON.parse(opts.body as string)).toEqual({ title: "test" }); + expect((opts.headers as Record)["Content-Type"]).toBe( + "application/json", + ); }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + test("handles 204 No Content without crashing on json()", async () => { + mockFetch(() => + Promise.resolve(new Response(null, { status: 204 })), + ); + const result = await githubRequest("/repos/test/repo/labels", "token", { + method: "POST", + body: { labels: ["bug"] }, + }); + expect(result).toEqual({}); }); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); - test("retries on ECONNREFUSED", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) return Promise.reject(networkError("ECONNREFUSED")); - return Promise.resolve(jsonResponse({ ok: true })); + test("uses custom userAgent", async () => { + await githubRequest("/repos/test/repo", "token", { + userAgent: "my-script", + }); + const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect((opts.headers as Record)["User-Agent"]).toBe( + "my-script", + ); }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + test("uses default userAgent when not specified", async () => { + await githubRequest("/repos/test/repo", "token"); + const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect((opts.headers as Record)["User-Agent"]).toBe( + "claude-code-scripts", + ); }); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); }); - test("retries on Bun socket close message", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) - return Promise.reject( - new Error("The socket connection was closed unexpectedly"), - ); - return Promise.resolve(jsonResponse({ ok: true })); + // ── Non-retryable errors ────────────────────────────────────────────── + + describe("non-retryable HTTP errors", () => { + test.each([ + [401, "Bad credentials"], + [403, "Forbidden"], + [422, "Validation failed"], + ])("throws immediately on %i without retry", async (status, body) => { + mockFetch(() => Promise.resolve(textResponse(body, status))); + await expect( + githubRequest("/repos/test/repo", "token"), + ).rejects.toThrow(`GitHub API ${status}`); + expect(fetchMock).toHaveBeenCalledTimes(1); }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + test("does not retry non-transient network errors", async () => { + mockFetch(() => + Promise.reject(new Error("Something unexpected")), + ); + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 3 }), + ).rejects.toThrow("Something unexpected"); + expect(fetchMock).toHaveBeenCalledTimes(1); }); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); }); - test("exhausts retries and throws on persistent ECONNRESET", async () => { - fetchMock = mock(() => Promise.reject(networkError("ECONNRESET"))); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - await expect( - githubRequest("/repos/test/repo", "token", { maxRetries: 2 }), - ).rejects.toThrow(); - - expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries - }); + // ── Retry on transient network errors ───────────────────────────────── - test("does not retry non-transient errors", async () => { - fetchMock = mock(() => - Promise.reject(new Error("Something unexpected")), + describe("retry on transient network errors", () => { + /** + * Helper: fetch fails `failCount` times with the given error, then succeeds. + */ + function failThenSucceed( + makeError: () => Error, + failCount: number, + ): void { + let calls = 0; + mockFetch(() => { + calls++; + if (calls <= failCount) return Promise.reject(makeError()); + return Promise.resolve(jsonResponse({ recovered: true })); + }); + } + + test.each(["ECONNRESET", "EPIPE", "ETIMEDOUT", "ECONNREFUSED"])( + "retries and recovers from %s", + async (code) => { + failThenSucceed(() => networkError(code), 1); + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ recovered: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }, ); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - await expect( - githubRequest("/repos/test/repo", "token", { maxRetries: 3 }), - ).rejects.toThrow("Something unexpected"); - expect(fetchMock).toHaveBeenCalledTimes(1); - }); - - // ── Retry on retryable HTTP status codes ────────────────────────────── - - test("retries on 502 Bad Gateway", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) - return Promise.resolve(textResponse("Bad Gateway", 502)); - return Promise.resolve(jsonResponse({ ok: true })); - }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + test("retries and recovers from Bun socket close", async () => { + failThenSucceed( + () => new Error("The socket connection was closed unexpectedly"), + 1, + ); + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ recovered: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); - test("retries on 500 Internal Server Error", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) - return Promise.resolve(textResponse("Internal Server Error", 500)); - return Promise.resolve(jsonResponse({ ok: true })); + test("recovers after multiple consecutive failures", async () => { + failThenSucceed(() => networkError("ECONNRESET"), 2); + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 3, + }); + expect(result).toEqual({ recovered: true }); + expect(fetchMock).toHaveBeenCalledTimes(3); }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + test("throws after exhausting all retries", async () => { + mockFetch(() => Promise.reject(networkError("ECONNRESET"))); + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 2 }), + ).rejects.toThrow(); + expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries }); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); }); - test("retries on 503 Service Unavailable", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) - return Promise.resolve(textResponse("Unavailable", 503)); - return Promise.resolve(jsonResponse({ ok: true })); - }); - globalThis.fetch = fetchMock as unknown as typeof fetch; + // ── Retry on retryable HTTP status codes ────────────────────────────── - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + describe("retry on retryable HTTP status codes", () => { + function httpFailThenSucceed(status: number, body: string): void { + let calls = 0; + mockFetch(() => { + calls++; + if (calls === 1) return Promise.resolve(textResponse(body, status)); + return Promise.resolve(jsonResponse({ ok: true })); + }); + } + + test.each([ + [429, "Rate limited"], + [500, "Internal Server Error"], + [502, "Bad Gateway"], + [503, "Service Unavailable"], + [504, "Gateway Timeout"], + ])("retries and recovers from %i", async (status, body) => { + httpFailThenSucceed(status, body); + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - expect(result).toEqual({ ok: true }); - }); - test("retries on 429 rate limit", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) - return Promise.resolve( - textResponse("Rate limited", 429, { "retry-after": "1" }), - ); - return Promise.resolve(jsonResponse({ ok: true })); + test("respects Retry-After header on 429", async () => { + let calls = 0; + mockFetch(() => { + calls++; + if (calls === 1) + return Promise.resolve( + textResponse("Rate limited", 429, { "retry-after": "1" }), + ); + return Promise.resolve(jsonResponse({ ok: true })); + }); + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 2, + }); + expect(result).toEqual({ ok: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 2, + test("throws after exhausting retries on persistent 502", async () => { + mockFetch(() => Promise.resolve(textResponse("Bad Gateway", 502))); + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 2 }), + ).rejects.toThrow("GitHub API 502"); + expect(fetchMock).toHaveBeenCalledTimes(3); }); - expect(result).toEqual({ ok: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); - - test("exhausts retries on persistent 502", async () => { - fetchMock = mock(() => - Promise.resolve(textResponse("Bad Gateway", 502)), - ); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - await expect( - githubRequest("/repos/test/repo", "token", { maxRetries: 2 }), - ).rejects.toThrow("GitHub API 502"); - - expect(fetchMock).toHaveBeenCalledTimes(3); // initial + 2 retries }); // ── maxRetries=0 disables retry ─────────────────────────────────────── - test("maxRetries=0 disables retry for network errors", async () => { - fetchMock = mock(() => Promise.reject(networkError("ECONNRESET"))); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - await expect( - githubRequest("/repos/test/repo", "token", { maxRetries: 0 }), - ).rejects.toThrow(); - - expect(fetchMock).toHaveBeenCalledTimes(1); - }); - - test("maxRetries=0 disables retry for HTTP errors", async () => { - fetchMock = mock(() => - Promise.resolve(textResponse("Bad Gateway", 502)), - ); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - await expect( - githubRequest("/repos/test/repo", "token", { maxRetries: 0 }), - ).rejects.toThrow("GitHub API 502"); + describe("maxRetries=0", () => { + test("no retry on network error", async () => { + mockFetch(() => Promise.reject(networkError("ECONNRESET"))); + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 0 }), + ).rejects.toThrow(); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); - expect(fetchMock).toHaveBeenCalledTimes(1); + test("no retry on retryable HTTP status", async () => { + mockFetch(() => Promise.resolve(textResponse("Bad Gateway", 502))); + await expect( + githubRequest("/repos/test/repo", "token", { maxRetries: 0 }), + ).rejects.toThrow("GitHub API 502"); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); }); // ── Mixed error scenarios ───────────────────────────────────────────── - test("retries through mixed ECONNRESET then 502 then success", async () => { - let callCount = 0; - fetchMock = mock(() => { - callCount++; - if (callCount === 1) return Promise.reject(networkError("ECONNRESET")); - if (callCount === 2) - return Promise.resolve(textResponse("Bad Gateway", 502)); - return Promise.resolve(jsonResponse({ finally: "success" })); - }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - const result = await githubRequest("/repos/test/repo", "token", { - maxRetries: 3, + describe("mixed error scenarios", () => { + test("recovers through ECONNRESET → 502 → success", async () => { + let calls = 0; + mockFetch(() => { + calls++; + if (calls === 1) return Promise.reject(networkError("ECONNRESET")); + if (calls === 2) + return Promise.resolve(textResponse("Bad Gateway", 502)); + return Promise.resolve(jsonResponse({ finally: "success" })); + }); + const result = await githubRequest("/repos/test/repo", "token", { + maxRetries: 3, + }); + expect(result).toEqual({ finally: "success" }); + expect(fetchMock).toHaveBeenCalledTimes(3); }); - expect(result).toEqual({ finally: "success" }); - expect(fetchMock).toHaveBeenCalledTimes(3); }); // ── Timeout ─────────────────────────────────────────────────────────── - test("aborts on timeout and retries", async () => { - let callCount = 0; - fetchMock = mock((_url: string, opts: RequestInit) => { - callCount++; - if (callCount === 1) { - // Simulate a slow response — the AbortSignal should fire before this resolves - return new Promise((resolve, reject) => { - const timer = setTimeout( - () => resolve(jsonResponse({ slow: true })), - 5000, - ); - opts.signal?.addEventListener("abort", () => { - clearTimeout(timer); - const err = new Error("The operation was aborted"); - err.name = "AbortError"; - reject(err); + describe("timeout", () => { + test("aborts slow requests and retries", async () => { + let calls = 0; + mockFetch((_url: unknown, opts: RequestInit) => { + calls++; + if (calls === 1) { + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => resolve(jsonResponse({ slow: true })), + 5000, + ); + opts.signal?.addEventListener("abort", () => { + clearTimeout(timer); + const err = new Error("The operation was aborted"); + err.name = "AbortError"; + reject(err); + }); }); - }); - } - return Promise.resolve(jsonResponse({ fast: true })); - }); - globalThis.fetch = fetchMock as unknown as typeof fetch; - - const result = await githubRequest("/repos/test/repo", "token", { - timeoutMs: 50, // very short timeout to trigger abort - maxRetries: 2, + } + return Promise.resolve(jsonResponse({ fast: true })); + }); + + const result = await githubRequest("/repos/test/repo", "token", { + timeoutMs: 50, + maxRetries: 2, + }); + expect(result).toEqual({ fast: true }); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - - expect(result).toEqual({ fast: true }); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); - - // ── Custom User-Agent ───────────────────────────────────────────────── - - test("uses custom userAgent", async () => { - await githubRequest("/repos/test/repo", "token", { - userAgent: "my-script", - }); - - const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; - expect((opts.headers as Record)["User-Agent"]).toBe("my-script"); - }); - - test("uses default userAgent when not specified", async () => { - await githubRequest("/repos/test/repo", "token"); - - const [, opts] = fetchMock.mock.calls[0] as [string, RequestInit]; - expect((opts.headers as Record)["User-Agent"]).toBe("claude-code-scripts"); }); }); diff --git a/scripts/github-request.ts b/scripts/github-request.ts index ad41b4023f..4dab0aac53 100644 --- a/scripts/github-request.ts +++ b/scripts/github-request.ts @@ -59,10 +59,10 @@ const RETRYABLE_STATUS_CODES = new Set([ ]); // --------------------------------------------------------------------------- -// Helpers +// Helpers (exported for testing) // --------------------------------------------------------------------------- -function isTransientError(error: unknown): boolean { +export function isTransientError(error: unknown): boolean { if (!(error instanceof Error)) return false; const code = (error as NodeJS.ErrnoException).code ?? ""; @@ -78,7 +78,7 @@ function isTransientError(error: unknown): boolean { return false; } -function calculateDelay(attempt: number): number { +export function calculateDelay(attempt: number): number { const base = Math.min( INITIAL_RETRY_DELAY_MS * Math.pow(2, attempt), MAX_RETRY_DELAY_MS, @@ -96,7 +96,7 @@ function sleep(ms: number): Promise { * Parse the `Retry-After` header (seconds or HTTP-date) into milliseconds. * Returns `null` when the header is missing or un-parseable. */ -function parseRetryAfter(headers: Headers): number | null { +export function parseRetryAfter(headers: Headers): number | null { const value = headers.get("retry-after"); if (!value) return null;