From 4eb0e1d8ed7ce688125d14d29b377822dae57e32 Mon Sep 17 00:00:00 2001 From: Saffron <263493777+itsmiso-ai@users.noreply.github.com> Date: Thu, 3 Sep 2026 06:52:38 +0000 Subject: [PATCH] fix(github): retry transient 429/5xx in remaining GitHub API fetchers The shared fetchWithRetry wrapper in src/lib/github-auth.ts (2 retries, exponential backoff with jitter, Retry-After honoured on 429, 429+5xx retry set) was only applied to issue-reconciliation.ts and a subset of call sites. Wrap every remaining raw fetch in github-issues.ts (updateIssueLabels, addIssueComment, updateIssueBody, addIssueLabel, updateIssueTitleAndBody, removeIssueLabel, closeIssue), github-ci.ts (rerunWorkflowRun, dispatchWorkflow) and github-code-search.ts (searchRepositoryCode, fetchRepositoryFileText, fetchRepositoryFileContent, listRepositoryDirectory) so a transient 429 no longer aborts the operation on the first attempt. Add src/lib/github-retry.test.ts pinning: 429 with Retry-After is retried and succeeds on the second try, 503 is retried, 404 is not retried, and syncStatusLabels still attempts all 5 labels when one call hits a transient 429 mid-loop. Update the removeIssueLabel 500 test to expect the two retries before the error surfaces. Fixes #917 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com> --- src/lib/github-ci.ts | 4 +- src/lib/github-code-search.ts | 10 +-- src/lib/github-issues.ts | 14 ++--- src/lib/github-retry.test.ts | 113 ++++++++++++++++++++++++++++++++++ src/lib/github.test.ts | 4 +- 5 files changed, 130 insertions(+), 15 deletions(-) create mode 100644 src/lib/github-retry.test.ts diff --git a/src/lib/github-ci.ts b/src/lib/github-ci.ts index dd8b8e88..9e5ddd01 100644 --- a/src/lib/github-ci.ts +++ b/src/lib/github-ci.ts @@ -112,7 +112,7 @@ export async function fetchPackages(repoFullName: string): Promise { - const response = await fetch( + const response = await fetchWithRetry( `${GITHUB_API}/repos/${repoFullName}/actions/runs/${runId}/rerun`, { method: "POST", headers: await getHeadersAsync() } ); @@ -127,7 +127,7 @@ export async function triggerWorkflowDispatch( workflowId: number, ref: string ): Promise { - const response = await fetch( + const response = await fetchWithRetry( `${GITHUB_API}/repos/${repoFullName}/actions/workflows/${workflowId}/dispatches`, { method: "POST", diff --git a/src/lib/github-code-search.ts b/src/lib/github-code-search.ts index a02d4f29..1953a631 100644 --- a/src/lib/github-code-search.ts +++ b/src/lib/github-code-search.ts @@ -1,4 +1,4 @@ -import { GITHUB_API, getHeadersAsync, fetchPaginated } from "./github-auth"; +import { GITHUB_API, getHeadersAsync, fetchPaginated, fetchWithRetry } from "./github-auth"; export interface GithubRepo { full_name: string; @@ -9,7 +9,7 @@ export interface GithubRepo { } async function fetchRepoJson(repoFullName: string, errorPrefix: string): Promise> { - const response = await fetch(`${GITHUB_API}/repos/${repoFullName}`, { + const response = await fetchWithRetry(`${GITHUB_API}/repos/${repoFullName}`, { headers: await getHeadersAsync(), }); if (!response.ok) { @@ -51,7 +51,7 @@ export async function searchRepositoryCode( const perPage = Math.min(Math.max(1, limit), 100); const searchQuery = `${query} repo:${repoFullName}`; const url = `${GITHUB_API}/search/code?q=${encodeURIComponent(searchQuery)}&per_page=${perPage}`; - const response = await fetch(url, { headers: await getHeadersAsync() }); + const response = await fetchWithRetry(url, { headers: await getHeadersAsync() }); if (!response.ok) { const text = await response.text(); throw new Error(`Code search failed for ${repoFullName}: ${response.status} ${text}`); @@ -84,7 +84,7 @@ export async function fetchRepositoryFileText( ): Promise { const encodedPath = encodePathForContentsApi(path); const query = ref ? `?ref=${encodeURIComponent(ref)}` : ""; - const response = await fetch( + const response = await fetchWithRetry( `${GITHUB_API}/repos/${repoFullName}/contents/${encodedPath}${query}`, { headers: await getHeadersAsync() }, ); @@ -118,7 +118,7 @@ export async function listRepositoryDirectory( ): Promise { const encodedPath = path ? encodePathForContentsApi(path) : ""; const query = ref ? `?ref=${encodeURIComponent(ref)}` : ""; - const response = await fetch( + const response = await fetchWithRetry( `${GITHUB_API}/repos/${repoFullName}/contents/${encodedPath}${query}`, { headers: await getHeadersAsync() }, ); diff --git a/src/lib/github-issues.ts b/src/lib/github-issues.ts index e9ab6e1e..94a58c2a 100644 --- a/src/lib/github-issues.ts +++ b/src/lib/github-issues.ts @@ -43,7 +43,7 @@ export async function updateIssueLabels( const [owner, repo] = repoFullName.split("/"); const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}/labels`; - const response = await fetch(url, { + const response = await fetchWithRetry(url, { method: "PUT", headers: await getHeadersAsync(), body: JSON.stringify({ labels }), @@ -94,7 +94,7 @@ export async function addIssueComment( const [owner, repo] = repoFullName.split("/"); const apiPath = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}/comments`; - const response = await fetch(apiPath, { + const response = await fetchWithRetry(apiPath, { method: "POST", headers: await getHeadersAsync(), body: JSON.stringify({ body }), @@ -121,7 +121,7 @@ export async function updateIssueComment( const [owner, repo] = repoFullName.split("/"); const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/comments/${commentId}`; - const response = await fetch(url, { + const response = await fetchWithRetry(url, { method: "PATCH", headers: await getHeadersAsync(), body: JSON.stringify({ body }), @@ -141,7 +141,7 @@ export async function addIssueLabel( const [owner, repo] = repoFullName.split("/"); const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}/labels`; - const response = await fetch(url, { + const response = await fetchWithRetry(url, { method: "POST", headers: await getHeadersAsync(), body: JSON.stringify({ labels: [label] }), @@ -166,7 +166,7 @@ export async function updateIssueTitleAndBody( const [owner, repo] = repoFullName.split("/"); const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}`; - const response = await fetch(url, { + const response = await fetchWithRetry(url, { method: "PATCH", headers: await getHeadersAsync(), body: JSON.stringify(fields), @@ -186,7 +186,7 @@ export async function removeIssueLabel( const [owner, repo] = repoFullName.split("/"); const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}/labels/${encodeURIComponent(label)}`; - const response = await fetch(url, { + const response = await fetchWithRetry(url, { method: "DELETE", headers: await getHeadersAsync(), }); @@ -218,7 +218,7 @@ export async function closeIssue( const [owner, repo] = repoFullName.split("/"); const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}`; - const response = await fetch(url, { + const response = await fetchWithRetry(url, { method: "PATCH", headers: await getHeadersAsync(), body: JSON.stringify({ state: "closed" }), diff --git a/src/lib/github-retry.test.ts b/src/lib/github-retry.test.ts new file mode 100644 index 00000000..590daa85 --- /dev/null +++ b/src/lib/github-retry.test.ts @@ -0,0 +1,113 @@ +// @vitest-environment node +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { fetchWithRetry } from "./github-auth"; +import { syncStatusLabels } from "./github-issues"; + +process.env.GITHUB_TOKEN = "test-token-for-retry-tests"; + +function jsonResponse(status: number, body: unknown, headers: Record = {}): Response { + return { + ok: status >= 200 && status < 300, + status, + json: () => Promise.resolve(body), + text: () => Promise.resolve(typeof body === "string" ? body : JSON.stringify(body)), + headers: new Headers(headers), + } as Response; +} + +describe("fetchWithRetry (shared GitHub fetch retry wrapper)", () => { + let fetchMock: ReturnType; + + beforeEach(() => { + vi.useFakeTimers(); + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + it("retries a 429 with Retry-After and succeeds on the second try", async () => { + fetchMock + .mockResolvedValueOnce(jsonResponse(429, { message: "rate limited" }, { "Retry-After": "1" })) + .mockResolvedValueOnce(jsonResponse(200, { number: 7 })); + + const promise = fetchWithRetry("https://api.github.com/repos/o/r/issues/7", { headers: { Authorization: "Bearer test-token" } }); + await vi.advanceTimersByTimeAsync(2000); + const response = await promise; + + expect(response.ok).toBe(true); + expect(await response.json()).toEqual({ number: 7 }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("retries a 503 and succeeds on the second try", async () => { + fetchMock + .mockResolvedValueOnce(jsonResponse(503, { message: "server error" })) + .mockResolvedValueOnce(jsonResponse(200, { number: 8 })); + + const promise = fetchWithRetry("https://api.github.com/repos/o/r/issues/8", { headers: { Authorization: "Bearer test-token" } }); + await vi.advanceTimersByTimeAsync(5000); + const response = await promise; + + expect(response.ok).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("does not retry a 404 and throws immediately", async () => { + fetchMock.mockResolvedValue(jsonResponse(404, { message: "Not Found" })); + + const promise = fetchWithRetry("https://api.github.com/repos/o/r/issues/999", { headers: { Authorization: "Bearer test-token" } }); + await vi.advanceTimersByTimeAsync(60_000); + + const response = await promise; + expect(response.ok).toBe(false); + expect(response.status).toBe(404); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); +}); + +describe("syncStatusLabels with a transient 429 mid-loop", () => { + let fetchMock: ReturnType; + const labels = ["status/queued", "status/in-progress", "status/blocked", "status/review", "status/done"]; + + beforeEach(() => { + vi.useFakeTimers(); + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + it("still attempts all 5 labels when one call hits a transient 429", async () => { + // Every call succeeds except the 3rd, which 429s once then succeeds on retry. + fetchMock.mockImplementation(async (url: string) => { + const callIndex = fetchMock.mock.calls.length; // 1-based + if (callIndex === 3) { + return jsonResponse(429, { message: "rate limited" }, { "Retry-After": "1" }); + } + return jsonResponse(200, {}); + }); + + const promise = syncStatusLabels("o/r", 42, labels, []); + await vi.advanceTimersByTimeAsync(10_000); + await promise; + + // 5 label calls + 1 retry of the 429'd call. + expect(fetchMock).toHaveBeenCalledTimes(6); + const calledPaths = fetchMock.mock.calls.map((call) => String(call[0])); + for (const path of calledPaths) { + expect(path).toContain("/repos/o/r/issues/42/labels"); + } + // Each of the 5 labels was sent in a request body. + const sentLabels = fetchMock.mock.calls.map((call) => JSON.parse(String(call[1].body)).labels[0]); + for (const label of labels) { + expect(sentLabels).toContain(label); + } + }); +}); diff --git a/src/lib/github.test.ts b/src/lib/github.test.ts index 19cb695d..030a748e 100644 --- a/src/lib/github.test.ts +++ b/src/lib/github.test.ts @@ -627,9 +627,11 @@ describe("issue mutations", () => { }); it("removeIssueLabel still throws on non-404 errors", async () => { - fetchMock.mockResolvedValueOnce(httpError(500)); + // 500 is transient: the shared retry wrapper retries twice before surfacing the error. + fetchMock.mockResolvedValue(httpError(500)); await expect(removeIssueLabel("org/repo", 5, "x")).rejects.toThrow("GitHub API error: 500"); + expect(fetchMock).toHaveBeenCalledTimes(3); }); });