Skip to content

Commit 48abc5a

Browse files
committed
Address Copilot review: remove rate-limit duplication, fix header read order, add test
- throwApiError: read x-ratelimit-reset header before the try block so it is available even when the response body is malformed JSON - Team-repos loop: replace duplicated rate-limit detection block with a direct call to throwApiError (removes the duplication highlighted by Copilot) - api.test.ts: add test covering 403 rate-limit response in the team repos endpoint, verifying the clean error message with wait time is thrown
1 parent 9822372 commit 48abc5a

2 files changed

Lines changed: 37 additions & 32 deletions

File tree

src/api.test.ts

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ describe("fetchAllResults", () => {
308308
});
309309
});
310310

311-
// ─── fetchRepoTeams ──────────────────────────────────────────────────────────
311+
// ─── fetchRepoTeams ──────────────────────────────────────────────────────────
312312

313313
describe("fetchRepoTeams", () => {
314314
afterEach(() => {
@@ -458,6 +458,35 @@ describe("fetchRepoTeams", () => {
458458
expect(result.has("myorg/repo-0")).toBe(true);
459459
});
460460

461+
it("throws a clean rate-limit error when the team repo list returns 403 rate-limit", async () => {
462+
const resetTimestamp = Math.ceil((Date.now() + 300_000) / 1_000); // 5 minutes from now
463+
globalThis.fetch = (async (url: string | URL | Request) => {
464+
const urlStr = url.toString();
465+
if (urlStr.includes("/teams?")) {
466+
return new Response(JSON.stringify([{ slug: "frontend-web", name: "FE" }]), {
467+
status: 200,
468+
headers: { "content-type": "application/json" },
469+
});
470+
}
471+
// Team repos endpoint → rate-limited
472+
return new Response(
473+
JSON.stringify({ message: "API rate limit exceeded for user ID 1234.", status: "403" }),
474+
{
475+
status: 403,
476+
headers: {
477+
"content-type": "application/json",
478+
"x-ratelimit-remaining": "0",
479+
"x-ratelimit-reset": String(resetTimestamp),
480+
},
481+
},
482+
);
483+
}) as typeof fetch;
484+
485+
await expect(fetchRepoTeams("myorg", "tok", ["frontend"], false)).rejects.toThrow(
486+
/GitHub API rate limit exceeded\. Please retry in \d+ minute/,
487+
);
488+
});
489+
461490
it("prefix matching is case-insensitive", async () => {
462491
globalThis.fetch = (async (url: string | URL | Request) => {
463492
const urlStr = url.toString();

src/api.ts

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,12 @@ interface RawRepo {
4545
*/
4646
async function throwApiError(res: Response, context?: string): Promise<never> {
4747
let apiMsg = "";
48-
let resetHeader: string | null = null;
48+
// Read rate-limit headers before consuming the body so they are always
49+
// available even when the response body is malformed JSON.
50+
const resetHeader = res.headers.get("x-ratelimit-reset");
4951
try {
5052
const body = (await res.json()) as { message?: string };
5153
apiMsg = body.message ?? "";
52-
resetHeader = res.headers.get("x-ratelimit-reset");
5354
} catch {
5455
// Ignore JSON parse errors; fall through to generic message
5556
}
@@ -288,35 +289,10 @@ export async function fetchRepoTeams(
288289
);
289290
if (!res.ok) {
290291
// 404 is expected for nested/secret teams — skip silently.
291-
if (res.status === 404) {
292-
break;
293-
}
294-
// Rate-limit 403: throw a clean error like every other rate-limit hit.
295-
const bodyJson = await res.json().catch(() => ({}) as { message?: string });
296-
const apiMsg: string = (bodyJson as { message?: string }).message ?? "";
297-
if (
298-
res.status === 403 &&
299-
(res.headers.get("x-ratelimit-remaining") === "0" ||
300-
apiMsg.toLowerCase().includes("rate limit"))
301-
) {
302-
const resetHeader = res.headers.get("x-ratelimit-reset");
303-
let wait = "";
304-
if (resetHeader !== null) {
305-
const resetMs = parseInt(resetHeader, 10) * 1_000 - Date.now();
306-
if (resetMs > 0) wait = ` Please retry in ${formatRetryWait(resetMs)}.`;
307-
}
308-
throw new Error(`GitHub API rate limit exceeded.${wait}`);
309-
}
310-
// Other errors are unexpected: log a warning and skip this team.
311-
let bodyText = apiMsg || JSON.stringify(bodyJson);
312-
if (bodyText.length > 200) bodyText = bodyText.slice(0, 200) + "…";
313-
const message = bodyText ? `; body: ${bodyText}` : "";
314-
process.stderr.write(
315-
pc.dim(
316-
`Warning: could not fetch repos for team "${slug}" (HTTP ${res.status}${message})\n`,
317-
),
318-
);
319-
break;
292+
if (res.status === 404) break;
293+
// All other errors (rate-limit included) delegate to throwApiError
294+
// which detects rate-limits and produces a clean message.
295+
await throwApiError(res);
320296
}
321297
const repos = (await res.json()) as RawRepo[];
322298
for (const r of repos) {

0 commit comments

Comments
 (0)