From 33300cdfca6d102c611aac78c032c5718468a20d Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:25:09 +0000 Subject: [PATCH] fix(miner): retry transient 5xx/rate-limit in contribution-profile getJson MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extractContributionProfile's getJson fetched a repo's label taxonomy and CONTRIBUTING.md by calling fetchImpl directly with no retry, so a single transient GitHub 5xx or rate-limit response (429 / secondary-403) degraded the signal to absent/unknown. Because discover-cli persists whatever extract() returns into the contribution-profile cache, that one blip got cached as the repo's eligibility signal until the entry expired, silently weakening label filtering for a repo that was reachable moments before or after. Route getJson through fetchWithRetry (./http-retry.js) — the same helper opportunity-fanout.js's sibling githubGetJson already uses — so a retryable status rides out its bounded attempts with exponential-backoff-or-Retry-After delay before falling back. The per-attempt REQUEST_TIMEOUT_MS is preserved via fetchWithRetry's timeoutMs (a fresh AbortSignal.timeout per attempt), the never-throws / fail-open contract is unchanged once retries are exhausted, and a sleepFn seam is threaded through so the retry is testable without real timers. Closes #7090 --- .../lib/contribution-profile-extract.d.ts | 2 + .../lib/contribution-profile-extract.js | 28 +++- .../unit/contribution-profile-extract.test.ts | 155 ++++++++++++++++++ 3 files changed, 176 insertions(+), 9 deletions(-) diff --git a/packages/loopover-miner/lib/contribution-profile-extract.d.ts b/packages/loopover-miner/lib/contribution-profile-extract.d.ts index b48067f309..6f0ea199bb 100644 --- a/packages/loopover-miner/lib/contribution-profile-extract.d.ts +++ b/packages/loopover-miner/lib/contribution-profile-extract.d.ts @@ -13,5 +13,7 @@ export function extractContributionProfile( apiBaseUrl?: string; /** ISO timestamp for the profile's generatedAt; defaults to now. Injected so tests stay deterministic. */ generatedAt?: string; + /** Sleep seam for the transient-5xx/rate-limit retry (via fetchWithRetry). Injected so tests use no real timers. */ + sleepFn?: (ms: number) => Promise; }, ): Promise; diff --git a/packages/loopover-miner/lib/contribution-profile-extract.js b/packages/loopover-miner/lib/contribution-profile-extract.js index ca256c7de4..6f8aee6fcb 100644 --- a/packages/loopover-miner/lib/contribution-profile-extract.js +++ b/packages/loopover-miner/lib/contribution-profile-extract.js @@ -9,6 +9,7 @@ import { emptyContributionProfile, weakestConfidence, } from "./contribution-profile.js"; +import { fetchWithRetry } from "./http-retry.js"; const DEFAULT_API_BASE_URL = "https://api.github.com"; const GITHUB_API_VERSION = "2022-11-28"; @@ -75,15 +76,20 @@ function githubHeaders(githubToken) { return headers; } -/** Bounded, never-throwing JSON GET. Returns null on any transport/HTTP/parse failure. */ -async function getJson(url, headers, fetchImpl) { +/** Bounded, never-throwing JSON GET. Rides out a transient GitHub 5xx or rate-limit response (429 / secondary-403) + * via `fetchWithRetry` — the same discipline opportunity-fanout.js's sibling `githubGetJson` already uses — before + * falling back to its fail-open contract: returns null on a non-retryable/exhausted HTTP, transport, or parse + * failure. `timeoutMs` gives each attempt its own fresh `AbortSignal.timeout` (preserving the per-request bound), + * and `sleepFn` is the injectable no-real-timers seam every other `fetchWithRetry` call site exposes. */ +async function getJson(url, headers, fetchImpl, sleepFn) { let response; try { - response = await fetchImpl(url, { - method: "GET", - headers, - signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), - }); + response = await fetchWithRetry( + fetchImpl, + url, + { method: "GET", headers }, + { sleepFn, timeoutMs: REQUEST_TIMEOUT_MS }, + ); } catch { return null; } @@ -146,12 +152,13 @@ function decodeContents(payload) { } /** Fetch CONTRIBUTING.md, probing the repo root then `.github/` (#6794: 6/10 at root, 2/10 under `.github/`). */ -async function fetchContributing(base, target, headers, fetchImpl) { +async function fetchContributing(base, target, headers, fetchImpl, sleepFn) { for (const path of ["CONTRIBUTING.md", ".github/CONTRIBUTING.md"]) { const payload = await getJson( `${base}/repos/${target.owner}/${target.repo}/contents/${path}`, headers, fetchImpl, + sleepFn, ); const text = decodeContents(payload); if (text !== null) return text; @@ -183,7 +190,7 @@ function extractPrBody(contributing) { * Extract a best-effort ContributionProfile for a repo from what it actually publishes. * * @param {string} repoFullName owner/repo - * @param {{ fetchImpl?: typeof fetch, githubToken?: string, apiBaseUrl?: string, generatedAt?: string }} [options] + * @param {{ fetchImpl?: typeof fetch, githubToken?: string, apiBaseUrl?: string, generatedAt?: string, sleepFn?: (ms: number) => Promise }} [options] * @returns {Promise} */ export async function extractContributionProfile(repoFullName, options = {}) { @@ -209,10 +216,12 @@ export async function extractContributionProfile(repoFullName, options = {}) { options.githubToken ?? process.env.GITHUB_TOKEN, ); + const sleepFn = options.sleepFn; const labelsPayload = await getJson( `${base}/repos/${target.owner}/${target.repo}/labels?per_page=100`, headers, fetchImpl, + sleepFn, ); const labels = Array.isArray(labelsPayload) ? labelsPayload : []; const contributing = await fetchContributing( @@ -220,6 +229,7 @@ export async function extractContributionProfile(repoFullName, options = {}) { target, headers, fetchImpl, + sleepFn, ); const eligibilityLabels = classifyLabels( diff --git a/test/unit/contribution-profile-extract.test.ts b/test/unit/contribution-profile-extract.test.ts index 55d97c54c5..fcf966b565 100644 --- a/test/unit/contribution-profile-extract.test.ts +++ b/test/unit/contribution-profile-extract.test.ts @@ -240,11 +240,166 @@ describe("extractContributionProfile (#6796)", () => { const profile = await extractContributionProfile("acme/widgets", { fetchImpl: asFetch(fetchImpl), generatedAt: AT, + sleepFn: async () => {}, }); expect(profile.eligibilityLabels.confidence).toBe("absent"); expect(profile.completeness).toBe("absent"); }); + it("retries a transient 5xx on the labels fetch and yields the same profile as an immediate success (#7090)", async () => { + // A single 5xx blip on the first attempt must NOT degrade the label signal — the retry rides it out and the + // resulting profile is identical to one where the labels fetch succeeded immediately. + const sleeps: number[] = []; + let labelsCalls = 0; + const fetchImpl = vi.fn(async (url: string) => { + const u = String(url); + if (u.includes("/labels")) { + labelsCalls += 1; + if (labelsCalls === 1) + return { + ok: false, + status: 500, + json: async () => ({}), + } as unknown as Response; + return { + ok: true, + status: 200, + json: async () => [ + { name: "help wanted", description: "Extra attention is needed" }, + ], + } as unknown as Response; + } + return { + ok: false, + status: 404, + json: async () => ({}), + } as unknown as Response; + }); + const profile = await extractContributionProfile("acme/widgets", { + fetchImpl: asFetch(fetchImpl), + generatedAt: AT, + sleepFn: async (ms: number) => { + sleeps.push(ms); + }, + }); + expect(labelsCalls).toBe(2); + expect(sleeps).toHaveLength(1); + expect(profile.eligibilityLabels.confidence).toBe("explicit"); + expect(profile.eligibilityLabels.value).toEqual([ + { field: "name", contains: "help wanted" }, + ]); + }); + + it("retries a transient 5xx on the CONTRIBUTING.md fetch and reads the linked-issue rule as if it never blipped (#7090)", async () => { + const sleeps: number[] = []; + let docCalls = 0; + const body = bigContributing("Reference an issue with Closes #7."); + const fetchImpl = vi.fn(async (url: string) => { + const u = String(url); + if (u.includes("/labels")) + return { + ok: true, + status: 200, + json: async () => [], + } as unknown as Response; + if (u.includes("/contents/CONTRIBUTING.md")) { + docCalls += 1; + if (docCalls === 1) + return { + ok: false, + status: 503, + json: async () => ({}), + } as unknown as Response; + return { + ok: true, + status: 200, + json: async () => ({ + encoding: "base64", + content: Buffer.from(body).toString("base64"), + }), + } as unknown as Response; + } + return { + ok: false, + status: 404, + json: async () => ({}), + } as unknown as Response; + }); + const profile = await extractContributionProfile("acme/widgets", { + fetchImpl: asFetch(fetchImpl), + generatedAt: AT, + sleepFn: async (ms: number) => { + sleeps.push(ms); + }, + }); + expect(docCalls).toBe(2); + expect(sleeps).toHaveLength(1); + expect(profile.prBody).toEqual({ + value: { requiresLinkedIssue: true }, + confidence: "explicit", + provenance: [{ source: "contributing_md", detail: "CONTRIBUTING.md" }], + }); + }); + + it("still degrades labels to absent once the 5xx retries are genuinely exhausted, without throwing (#7090)", async () => { + const sleeps: number[] = []; + let labelsCalls = 0; + const fetchImpl = vi.fn(async (url: string) => { + if (String(url).includes("/labels")) { + labelsCalls += 1; + return { + ok: false, + status: 502, + json: async () => ({}), + } as unknown as Response; + } + return { + ok: false, + status: 404, + json: async () => ({}), + } as unknown as Response; + }); + const profile = await extractContributionProfile("acme/widgets", { + fetchImpl: asFetch(fetchImpl), + generatedAt: AT, + sleepFn: async (ms: number) => { + sleeps.push(ms); + }, + }); + // DEFAULT_MAX_ATTEMPTS attempts, 2 sleeps between them — then fail open exactly as before. + expect(labelsCalls).toBe(3); + expect(sleeps).toHaveLength(2); + expect(profile.eligibilityLabels.confidence).toBe("absent"); + expect(profile.completeness).toBe("absent"); + }); + + it("still degrades the doc to absent once its 5xx retries are exhausted, preserving the fail-open contract (#7090)", async () => { + let docCalls = 0; + const fetchImpl = vi.fn(async (url: string) => { + const u = String(url); + if (u.includes("/labels")) + return { + ok: true, + status: 200, + json: async () => [], + } as unknown as Response; + docCalls += 1; + return { + ok: false, + status: 500, + json: async () => ({}), + } as unknown as Response; + }); + const profile = await extractContributionProfile("acme/widgets", { + fetchImpl: asFetch(fetchImpl), + generatedAt: AT, + sleepFn: async () => {}, + }); + // Both the root and `.github/` probes are each retried to exhaustion (3 attempts × 2 paths). + expect(docCalls).toBe(6); + expect(profile.prBody.confidence).toBe("absent"); + }); + it("degrades to absent when the transport throws, without propagating the error", async () => { const fetchImpl = vi.fn(async () => { throw new Error("network down");