Skip to content

Commit 9a84fcd

Browse files
authored
fix(discovery-index): treat body-signalled secondary rate limits as retryable (#10249)
isRateLimitStatus only inspected status/retry-after/x-ratelimit-remaining, so a GitHub secondary rate limit (403 with body-only signalling, no retry-after, remaining still non-zero) fell through as a permanent failure and silently truncated the fan-out's candidate set. Port the body-regex check from src/github/client.ts's isRateLimitedResponse, reading via clone().text() so callers can still read the response body on the retried-and-succeeded path. Co-authored-by: bitfathers94 <237535319+bitfathers94@users.noreply.github.com>
1 parent d4b477f commit 9a84fcd

2 files changed

Lines changed: 71 additions & 5 deletions

File tree

packages/discovery-index/src/github-client.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,22 @@ function defaultSleep(delayMs: number): Promise<void> {
5858
return new Promise((resolve) => setTimeout(resolve, delayMs));
5959
}
6060

61-
function isRateLimitStatus(response: Response): boolean {
61+
async function isRateLimitStatus(response: Response): Promise<boolean> {
6262
if (response.status === 429) return true;
6363
if (response.status !== 403) return false;
6464
if (response.headers.get("retry-after") != null) return true;
6565
const remaining = response.headers.get("x-ratelimit-remaining");
66-
return remaining != null && Number(remaining) === 0;
66+
if (remaining != null && Number(remaining) === 0) return true;
67+
try {
68+
return /secondary rate limit|\babuse\b|api rate limit exceeded/i.test(await response.clone().text());
69+
/* v8 ignore next 3 -- defensive: a cloned Response body that fails to read isn't reachable in practice */
70+
} catch {
71+
return false;
72+
}
6773
}
6874

69-
function isRetryableStatus(response: Response): boolean {
70-
return response.status >= 500 || isRateLimitStatus(response);
75+
async function isRetryableStatus(response: Response): Promise<boolean> {
76+
return response.status >= 500 || (await isRateLimitStatus(response));
7177
}
7278

7379
function retryDelayMs(response: Response, attempt: number, backoffMs: (attempt: number) => number): number {
@@ -169,7 +175,7 @@ export class GitHubClient {
169175
this.requestTimeoutMs && this.requestTimeoutMs > 0 ? { ...init, signal: AbortSignal.timeout(this.requestTimeoutMs) } : init,
170176
);
171177
this.recordRateLimit(response);
172-
if (!isRetryableStatus(response) || attempt >= this.maxAttempts) {
178+
if (!(await isRetryableStatus(response)) || attempt >= this.maxAttempts) {
173179
incr("discovery_index_github_requests_total", { outcome: response.ok ? "ok" : "failed" });
174180
return response;
175181
}

test/unit/discovery-index/github-client.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,66 @@ describe("discovery-index GitHubClient (#7164)", () => {
226226
expect(calls).toHaveLength(1);
227227
});
228228

229+
it("retries a body-signalled secondary rate limit with no Retry-After header and a non-zero x-ratelimit-remaining", async () => {
230+
const secondaryLimitBody = JSON.stringify({
231+
message: "You have exceeded a secondary rate limit. Please wait a few minutes before you try again.",
232+
});
233+
const { fetchImpl, calls } = makeFetchStub([
234+
new Response(secondaryLimitBody, { status: 403, headers: { "x-ratelimit-remaining": "4980" } }),
235+
new Response("[]", { status: 200 }),
236+
]);
237+
const client = new GitHubClient({ token: "tok", fetchImpl, sleepFn: vi.fn(), maxAttempts: 2 });
238+
const result = await client.fetchRepoIssues("owner/repo");
239+
expect(result).toEqual({ issues: [], warnings: [] });
240+
expect(calls).toHaveLength(2);
241+
});
242+
243+
it("retries a body-signalled secondary rate limit matching GitHub's older abuse-detection wording", async () => {
244+
const { fetchImpl, calls } = makeFetchStub([
245+
new Response(JSON.stringify({ message: "You have triggered an abuse detection mechanism." }), {
246+
status: 403,
247+
headers: { "x-ratelimit-remaining": "4980" },
248+
}),
249+
new Response("[]", { status: 200 }),
250+
]);
251+
const client = new GitHubClient({ token: "tok", fetchImpl, sleepFn: vi.fn(), maxAttempts: 2 });
252+
const { warnings } = await client.fetchRepoIssues("owner/repo");
253+
expect(warnings).toEqual([]);
254+
expect(calls).toHaveLength(2);
255+
});
256+
257+
it("treats a 403 with an unreadable cloned body as not rate limited", async () => {
258+
const unreadableResponse = {
259+
status: 403,
260+
ok: false,
261+
headers: new Headers({ "x-ratelimit-remaining": "4980" }),
262+
clone: () => ({ text: () => Promise.reject(new Error("body already consumed")) }),
263+
} as unknown as Response;
264+
const calls: string[] = [];
265+
const fetchImpl = (async (url: string | URL) => {
266+
calls.push(String(url));
267+
return unreadableResponse;
268+
}) as unknown as typeof fetch;
269+
const client = new GitHubClient({ token: "tok", fetchImpl, sleepFn: vi.fn(), maxAttempts: 3 });
270+
const { warnings } = await client.fetchRepoIssues("owner/repo");
271+
expect(calls).toHaveLength(1);
272+
expect(warnings[0]).toMatch(/403/);
273+
});
274+
275+
it("REGRESSION: a body-signalled secondary rate limit is retried, not surfaced as a truncated result", async () => {
276+
const secondaryLimitBody = JSON.stringify({
277+
message: "You have exceeded a secondary rate limit. Please wait a few minutes before you try again.",
278+
});
279+
const { fetchImpl } = makeFetchStub([
280+
new Response(secondaryLimitBody, { status: 403, headers: { "x-ratelimit-remaining": "4980" } }),
281+
new Response(JSON.stringify([{ number: 1, title: "A" }]), { status: 200 }),
282+
]);
283+
const client = new GitHubClient({ token: "tok", fetchImpl, sleepFn: vi.fn(), maxAttempts: 2 });
284+
const { issues, warnings } = await client.fetchRepoIssues("owner/repo");
285+
expect(issues).toHaveLength(1);
286+
expect(warnings).toEqual([]);
287+
});
288+
229289
it("applies a per-attempt request timeout signal when configured", async () => {
230290
const { fetchImpl, calls } = makeFetchStub([new Response("[]", { status: 200 })]);
231291
const client = new GitHubClient({ token: "tok", fetchImpl, sleepFn: vi.fn(), requestTimeoutMs: 5000 });

0 commit comments

Comments
 (0)