From fd61b79f86025d731948c72c96a60da928469c01 Mon Sep 17 00:00:00 2001 From: real-venus Date: Sun, 12 Jul 2026 11:27:41 -0700 Subject: [PATCH] feat(miner): retry a transient 5xx in the discovery fanout A network blip while fetching a repo's policy docs or target issues was silently swallowed into a warning, dropping that repo's results for the entire discover run with no retry. Wrap the fanout's single shared fetch helper (githubGetJson) with the same fetchWithRetry discipline the CI and gate-verdict pollers use (#4829): a transient 5xx response is retried with bounded exponential backoff before the repo is dropped, so a brief blip recovers instead of losing results. A 4xx/404 is returned immediately and a thrown network error still propagates to each caller's try/catch (unchanged). sleepFn is threaded through normalizeOptions so tests inject an instant retry; a persistent 5xx still warns after the retries are exhausted. Closes #4830 --- .../lib/opportunity-fanout.d.ts | 4 +++ .../lib/opportunity-fanout.js | 23 ++++++++++------ test/unit/miner-opportunity-fanout.test.ts | 27 ++++++++++++++++++- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/packages/gittensory-miner/lib/opportunity-fanout.d.ts b/packages/gittensory-miner/lib/opportunity-fanout.d.ts index dc1289d03a..6f4d408694 100644 --- a/packages/gittensory-miner/lib/opportunity-fanout.d.ts +++ b/packages/gittensory-miner/lib/opportunity-fanout.d.ts @@ -38,6 +38,7 @@ export function fetchCandidateIssuesWithSummary( apiBaseUrl?: string; concurrency?: number; perPage?: number; + sleepFn?: (ms: number) => Promise; }, ): Promise; @@ -48,6 +49,7 @@ export function fetchCandidateIssues( apiBaseUrl?: string; concurrency?: number; perPage?: number; + sleepFn?: (ms: number) => Promise; }, ): Promise; @@ -58,6 +60,7 @@ export function searchCandidateIssuesWithSummary( apiBaseUrl?: string; concurrency?: number; perPage?: number; + sleepFn?: (ms: number) => Promise; }, ): Promise; @@ -68,5 +71,6 @@ export function searchCandidateIssues( apiBaseUrl?: string; concurrency?: number; perPage?: number; + sleepFn?: (ms: number) => Promise; }, ): Promise; diff --git a/packages/gittensory-miner/lib/opportunity-fanout.js b/packages/gittensory-miner/lib/opportunity-fanout.js index a8e56c7ca5..915e18b598 100644 --- a/packages/gittensory-miner/lib/opportunity-fanout.js +++ b/packages/gittensory-miner/lib/opportunity-fanout.js @@ -1,5 +1,6 @@ import { Buffer } from "node:buffer"; import { resolveAiPolicyVerdict } from "@jsonbored/gittensory-engine"; +import { fetchWithRetry } from "./http-retry.js"; const defaultApiBaseUrl = "https://api.github.com"; const defaultConcurrency = 5; @@ -101,11 +102,15 @@ function recordRateLimit(summary, response) { } } -async function githubGetJson(url, githubToken, summary) { - const response = await fetch(url, { - method: "GET", - headers: githubHeaders(githubToken), - }); +async function githubGetJson(url, githubToken, summary, options) { + // Retry a transient 5xx from GitHub before dropping this target's results for the whole run (#4830) — the same + // discipline as the CI/gate-verdict pollers. A thrown network error still propagates to each caller's try/catch. + const response = await fetchWithRetry( + fetch, + url, + { method: "GET", headers: githubHeaders(githubToken) }, + { sleepFn: options?.sleepFn }, + ); recordRateLimit(summary, response); const payload = await response.json().catch(() => null); return { response, payload }; @@ -130,7 +135,7 @@ async function fetchRepoDoc(target, path, githubToken, options, summary, warning repoPath(target, `/contents/${encodeURIComponent(path)}`), ); try { - const { response, payload } = await githubGetJson(url, githubToken, summary); + const { response, payload } = await githubGetJson(url, githubToken, summary, options); if (response.status === 404) return null; if (!response.ok) { warnings.push(warning(target, `policy:${path}`, `GitHub returned ${response.status}`)); @@ -212,7 +217,7 @@ async function fetchTargetIssues(target, githubToken, options, summary, warnings `?state=open&per_page=${options.perPage}`, ); try { - const { response, payload } = await githubGetJson(url, githubToken, summary); + const { response, payload } = await githubGetJson(url, githubToken, summary, options); if (!response.ok) { warnings.push(warning(target, "issues", `GitHub returned ${response.status}`)); return []; @@ -242,7 +247,7 @@ async function fetchSearchIssues(searchQuery, githubToken, options, summary, war `?q=${encodeURIComponent(qualifiedQuery)}&per_page=${options.perPage}`, ); try { - const { response, payload } = await githubGetJson(url, githubToken, summary); + const { response, payload } = await githubGetJson(url, githubToken, summary, options); if (!response.ok) { warnings.push({ repoFullName: "*", @@ -292,6 +297,8 @@ function normalizeOptions(options = {}) { : defaultApiBaseUrl, concurrency: normalizeLimit(options.concurrency, defaultConcurrency, 1, 10), perPage: normalizeLimit(options.perPage, defaultPerPage, 1, 100), + // Passed through to the per-fetch retry so tests can inject an instant sleep; undefined uses the real backoff. + sleepFn: typeof options.sleepFn === "function" ? options.sleepFn : undefined, }; } diff --git a/test/unit/miner-opportunity-fanout.test.ts b/test/unit/miner-opportunity-fanout.test.ts index 2cb7fb4c3f..3a8db62489 100644 --- a/test/unit/miner-opportunity-fanout.test.ts +++ b/test/unit/miner-opportunity-fanout.test.ts @@ -181,7 +181,7 @@ describe("fetchCandidateIssues (#2307)", () => { { owner: "acme", repo: "up" }, ], "token", - { apiBaseUrl: API }, + { apiBaseUrl: API, sleepFn: () => Promise.resolve() }, // instant retry: a persistent 503 still warns ); expect(result.issues.map((entry) => entry.issueNumber)).toEqual([11]); @@ -301,6 +301,7 @@ describe("fetchCandidateIssues (#2307)", () => { const result = await searchCandidateIssuesWithSummary("label:feature", "token", { apiBaseUrl: API, + sleepFn: () => Promise.resolve(), // instant retry: a persistent 502 still warns }); expect(result.issues).toEqual([]); @@ -308,4 +309,28 @@ describe("fetchCandidateIssues (#2307)", () => { { repoFullName: "*", stage: "search", message: "GitHub returned 502" }, ]); }); + + it("retries a transient 5xx and keeps the target's issues instead of dropping them (#4830)", async () => { + let issuesAttempts = 0; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = String(input); + if (url.endsWith("/contents/AI-USAGE.md")) return jsonResponse({}, { status: 404 }); + if (url.endsWith("/contents/CONTRIBUTING.md")) return contentResponse("Contributions welcome."); + if (url.includes("/repos/acme/blip/issues?")) { + issuesAttempts += 1; + if (issuesAttempts === 1) return jsonResponse({ message: "server error" }, { status: 503 }); // a blip + return jsonResponse([issue(7)]); + } + return jsonResponse({}, { status: 404 }); + }); + + const result = await fetchCandidateIssuesWithSummary([{ owner: "acme", repo: "blip" }], "token", { + apiBaseUrl: API, + sleepFn: () => Promise.resolve(), + }); + + expect(issuesAttempts).toBe(2); // the 503 was retried, then succeeded + expect(result.issues.map((entry) => entry.issueNumber)).toEqual([7]); // results kept, not dropped + expect(result.warnings).toEqual([]); // no warning — the transient blip recovered + }); });