Imports: a GitHub rate limit on the repository lookup waits instead of failing the import (#95)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AizKXSsengXZRegJxRATkL
Canberk Demirci authored · GitHub committed signed1 parent 5702919commit 90b603a1cc7883f742f44cea961cfccca7b182daBrowse files
3 files changed+38−3
modifiedapps/diffinery/src/imports/service.ts+10−2View file
| @@ -730,14 +730,21 @@ | ||
| 730 | 730 | |
| 731 | 731 | // ---------------------------------------------------------------- workflow: one repository |
| 732 | 732 | |
| 733 | - /** Create the Diffinery repository (or reuse the one this import created on an earlier attempt). */ | |
| 734 | - async stepRepository(id: string): Promise<void> { | |
| 733 | + /** | |
| 734 | + * Create the Diffinery repository (or reuse the one this import created on an earlier attempt). | |
| 735 | + * A rate limit comes back as `wait` so the workflow sleeps it off and calls this again. | |
| 736 | + */ | |
| 737 | + async stepRepository(id: string): Promise<{ wait?: number }> { | |
| 735 | 738 | const ctx = await this.context(id); |
| 736 | 739 | await this.setStep(id, "repository", { status: "running", startedAt: now() }); |
| 737 | 740 | let gr: GhRepo; |
| 738 | 741 | try { |
| 739 | 742 | gr = await ctx.gh.get<GhRepo>(`repos/${ctx.row.source}`); |
| 740 | 743 | } catch (e) { |
| 744 | + if (e instanceof RateLimited) { | |
| 745 | + await this.setStep(id, "repository", { message: e.message }); | |
| 746 | + return { wait: e.waitSeconds }; | |
| 747 | + } | |
| 741 | 748 | if (e instanceof GitHubError && e.status < 500) throw fatal(e.status === 401 ? "GitHub rejected the token (expired or revoked?)" : `GitHub repository ${ctx.row.source} was not found or the token cannot read it`); |
| 742 | 749 | throw e; |
| 743 | 750 | } |
| @@ -782,6 +789,7 @@ | ||
| 782 | 789 | await this.dir.setTopics(repo.id, ctx.summary.source.topics); |
| 783 | 790 | await this.saveSummary(id, ctx.summary); |
| 784 | 791 | await this.setStep(id, "repository", { status: "done", message: `${repo.owner_login}/${repo.name}`, finishedAt: now() }); |
| 792 | + return {}; | |
| 785 | 793 | } |
| 786 | 794 | |
| 787 | 795 | /** Start (or restart) the Git transfer. Returns whether an external agent has to claim it. */ |
modifiedapps/diffinery/src/imports/workflow.ts+10−1View file
| @@ -13,6 +13,8 @@ | ||
| 13 | 13 | const GIT_TRIES = 2; |
| 14 | 14 | /** An overall cap on how many times one paged step may be called, so a hostile or broken source cannot run the workflow forever. */ |
| 15 | 15 | const MAX_STEP_CALLS = 2000; |
| 16 | +/** How many rate-limit sleeps (each at most an hour) the repository lookup may take. */ | |
| 17 | +const MAX_RATE_LIMIT_WAITS = 48; | |
| 16 | 18 | |
| 17 | 19 | /** |
| 18 | 20 | * Durable execution of one GitHub import (instance id `<importId>-<attempt>`). |
| @@ -39,7 +41,14 @@ | ||
| 39 | 41 | } |
| 40 | 42 | |
| 41 | 43 | private async repository(id: string, step: WorkflowStep, svc: () => ImportService): Promise<void> { |
| 42 | - await step.do("repository", RETRY, () => svc().stepRepository(id)); | |
| 44 | + // GitHub's rate limit is slept off, never a failure (the first call keeps its original step name). | |
| 45 | + for (let i = 0; ; i++) { | |
| 46 | + if (i >= MAX_RATE_LIMIT_WAITS) throw new Error("GitHub kept rate limiting the repository lookup"); | |
| 47 | + const r = await step.do(i === 0 ? "repository" : `repository ${i}`, RETRY, () => svc().stepRepository(id)); | |
| 48 | + // `?.`: an instance that ran this step before the change replays it with no result. | |
| 49 | + if (!r?.wait) break; | |
| 50 | + await step.sleep(`repository rate limit ${i}`, `${r.wait} seconds`); | |
| 51 | + } | |
| 43 | 52 | |
| 44 | 53 | for (let attempt = 1; ; attempt++) { |
| 45 | 54 | await step.do(`git start ${attempt}`, RETRY, () => svc().startGit(id)); |
modifiedapps/diffinery/test/unit/imports.test.ts+18−0View file
| @@ -11,6 +11,7 @@ | ||
| 11 | 11 | import type { Env } from "../../src/env.js"; |
| 12 | 12 | import type { Account } from "../../src/lib/directory.js"; |
| 13 | 13 | import { ImportService } from "../../src/imports/service.js"; |
| 14 | +import { RateLimited } from "../../src/imports/github.js"; | |
| 14 | 15 | |
| 15 | 16 | // ---------------------------------------------------------------- request and option validation |
| 16 | 17 | |
| @@ -356,6 +357,23 @@ | ||
| 356 | 357 | }); |
| 357 | 358 | }); |
| 358 | 359 | |
| 360 | +// ---------------------------------------------------------------- rate limits | |
| 361 | + | |
| 362 | +describe("a GitHub rate limit pauses an import, it never fails it", () => { | |
| 363 | + it("looking up the source repository waits out the limit and leaves the step running", async () => { | |
| 364 | + const svc = service(); | |
| 365 | + const steps: Array<Record<string, unknown>> = []; | |
| 366 | + const gh = { get: async () => { throw new RateLimited(42); } }; | |
| 367 | + Object.assign(svc, { | |
| 368 | + context: async () => ({ row: { id: "imp", source: "octo/app" }, gh, steps: {}, summary: {} }), | |
| 369 | + setStep: async (_id: string, _name: string, patch: Record<string, unknown>) => void steps.push(patch), | |
| 370 | + }); | |
| 371 | + expect(await svc.stepRepository("imp")).toEqual({ wait: 42 }); | |
| 372 | + expect(steps.at(-1)).toEqual({ message: "GitHub rate limit reached; continuing in 42 seconds" }); | |
| 373 | + expect(steps.some((p) => p.status === "failed" || p.status === "done")).toBe(false); | |
| 374 | + }); | |
| 375 | +}); | |
| 376 | + | |
| 359 | 377 | // ---------------------------------------------------------------- texts that point at the web UI |
| 360 | 378 | |
| 361 | 379 | describe("import texts name what the web UI shows", () => { |