Skip to content

Commit e593223

Browse files
snomiaoclaude
andcommitted
fix: resolve gh.spec.ts test timeouts caused by incorrect retry config
- doNotRetry must use numbers not strings ([404] not ["404"]) - string comparison against numeric error.status always failed, causing all 4xx errors to be retried - Replace server.use() error overrides with sentinel values in github-handlers.ts (owner="error-404" → 404, owner="rate-limited" → 403, tag_sha="lightweight-tag" → 404) since MSW runtime handler overrides don't intercept in Bun - Rewrite 3 error-handling tests to use expect().rejects pattern with sentinel values Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 1807353 commit e593223

File tree

3 files changed

+21
-77
lines changed

3 files changed

+21
-77
lines changed

lib/github/createOctokit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export function createOctokit({ auth, maxRetries = 3 }: OctokitOptions) {
3232
},
3333
},
3434
retry: {
35-
doNotRetry: ["400", "401", "403", "404", "422", "429"], // 429 handled by throttling plugin
35+
doNotRetry: [400, 401, 403, 404, 422, 429], // 429 handled by throttling plugin
3636
},
3737
});
3838
}

src/gh.spec.ts

Lines changed: 12 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -344,27 +344,10 @@ describe("GitHub API Client (gh)", () => {
344344
});
345345

346346
it("should handle non-annotated tag errors gracefully", async () => {
347-
// Mock a 404 response for lightweight tags
348-
server.use(
349-
http.get("https://api.github.com/repos/:owner/:repo/git/tags/:tag_sha", () => {
350-
return new HttpResponse(null, {
351-
status: 404,
352-
statusText: "Not Found",
353-
});
354-
}),
355-
);
356-
357-
try {
358-
await gh.git.getTag({
359-
owner: "comfyanonymous",
360-
repo: "ComfyUI",
361-
tag_sha: "lightweight-tag",
362-
});
363-
// Should not reach here
364-
expect(true).toBe(false);
365-
} catch (error: unknown) {
366-
expect((error as { status: number }).status).toBe(404);
367-
}
347+
// tag_sha="lightweight-tag" is handled by github-handlers.ts to return 404
348+
await expect(
349+
gh.git.getTag({ owner: "comfyanonymous", repo: "ComfyUI", tag_sha: "lightweight-tag" }),
350+
).rejects.toMatchObject({ status: 404 });
368351
});
369352
});
370353

@@ -389,64 +372,17 @@ describe("GitHub API Client (gh)", () => {
389372

390373
describe("Error Handling", () => {
391374
it("should handle 404 errors", async () => {
392-
server.use(
393-
http.get("https://api.github.com/repos/:owner/:repo", () => {
394-
return new HttpResponse(
395-
JSON.stringify({
396-
message: "Not Found",
397-
documentation_url: "https://docs.github.com/rest",
398-
}),
399-
{
400-
status: 404,
401-
headers: {
402-
"Content-Type": "application/json",
403-
},
404-
},
405-
);
406-
}),
407-
);
408-
409-
try {
410-
await gh.repos.get({
411-
owner: "nonexistent",
412-
repo: "nonexistent",
413-
});
414-
// Should not reach here
415-
expect(true).toBe(false);
416-
} catch (error: unknown) {
417-
expect((error as { status: number }).status).toBe(404);
418-
}
375+
// owner="error-404" is handled by github-handlers.ts to return 404
376+
await expect(gh.repos.get({ owner: "error-404", repo: "any" })).rejects.toMatchObject({
377+
status: 404,
378+
});
419379
});
420380

421381
it("should handle rate limit errors", async () => {
422-
server.use(
423-
http.get("https://api.github.com/repos/:owner/:repo", () => {
424-
return new HttpResponse(
425-
JSON.stringify({
426-
message: "API rate limit exceeded",
427-
documentation_url:
428-
"https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting",
429-
}),
430-
{
431-
status: 403,
432-
headers: {
433-
"Content-Type": "application/json",
434-
},
435-
},
436-
);
437-
}),
438-
);
439-
440-
try {
441-
await gh.repos.get({
442-
owner: "octocat",
443-
repo: "Hello-World",
444-
});
445-
// Should not reach here
446-
expect(true).toBe(false);
447-
} catch (error: unknown) {
448-
expect((error as { status: number }).status).toBe(403);
449-
}
382+
// owner="rate-limited" is handled by github-handlers.ts to return 403
383+
await expect(gh.repos.get({ owner: "rate-limited", repo: "any" })).rejects.toMatchObject({
384+
status: 403,
385+
});
450386
});
451387
});
452388
});

src/test/github-handlers.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@ export const githubHandlers = [
1010
// ==================== REPOS ====================
1111

1212
// GET /repos/:owner/:repo - Get a repository
13+
// Use owner="error-404" to simulate 404, owner="rate-limited" to simulate 403 rate limit
1314
http.get(`${GITHUB_API_BASE}/repos/:owner/:repo`, ({ params }) => {
1415
const { owner, repo } = params;
16+
if (owner === "error-404")
17+
return HttpResponse.json({ message: "Not Found", documentation_url: "https://docs.github.com/rest" }, { status: 404 });
18+
if (owner === "rate-limited")
19+
return HttpResponse.json({ message: "API rate limit exceeded" }, { status: 403 });
1520
return HttpResponse.json({
1621
id: 123456,
1722
name: repo,
@@ -760,8 +765,11 @@ export const githubHandlers = [
760765
// ==================== GIT ====================
761766

762767
// GET /repos/:owner/:repo/git/tags/:tag_sha - Get a tag (annotated)
768+
// Use tag_sha="lightweight-tag" to simulate a non-annotated (lightweight) tag returning 404
763769
http.get(`${GITHUB_API_BASE}/repos/:owner/:repo/git/tags/:tag_sha`, ({ params }) => {
764770
const { owner, repo, tag_sha } = params;
771+
if (tag_sha === "lightweight-tag")
772+
return HttpResponse.json({ message: "Not Found" }, { status: 404 });
765773
return HttpResponse.json({
766774
node_id: "MDM6VGFn",
767775
tag: "v1.0.0",

0 commit comments

Comments
 (0)