Imports and live test: ride out GitHub outages, and check schedules even when the import check fails (#99)
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 8d60426commit e256545eb0d8ed8e91605bd420a2f6dd9abf3625Browse files
3 files changed+73−5
modified.github/workflows/live-test.yml+13−3View file
| @@ -532,9 +532,18 @@ | ||
| 532 | 532 | api() { curl -fsS -X "$1" "$DIFFINERY/api/v3$2" -H "Authorization: Bearer $TOKEN" -H "content-type: application/json" ${3:+--data "$3"}; } |
| 533 | 533 | name="$REPO-import" |
| 534 | 534 | # Token-free, so GitHub allows 60 requests an hour per address: a rate limit skips this check with a warning. |
| 535 | - code=$(curl -sS -o import.json -w '%{http_code}' -X POST "$DIFFINERY/api/v3/diffinery/imports" -H "Authorization: Bearer $TOKEN" -H "content-type: application/json" \ | |
| 536 | - --data "{\"source\":\"$SOURCE\",\"owner\":\"$OWNER\",\"name\":\"$name\",\"visibility\":\"private\"}") | |
| 535 | + # Starting the import reads the repository from GitHub; a GitHub outage (502 "could not reach GitHub") | |
| 536 | + # is retried, and skips this check with a warning if it outlasts the retries. | |
| 537 | + for try in 1 2 3 4; do | |
| 538 | + code=$(curl -sS -o import.json -w '%{http_code}' -X POST "$DIFFINERY/api/v3/diffinery/imports" -H "Authorization: Bearer $TOKEN" -H "content-type: application/json" \ | |
| 539 | + --data "{\"source\":\"$SOURCE\",\"owner\":\"$OWNER\",\"name\":\"$name\",\"visibility\":\"private\"}" || true) | |
| 540 | + unreachable=$(jq -r '.message // "" | startswith("could not reach GitHub")' import.json 2>/dev/null || echo false) | |
| 541 | + { [ "$code" = 502 ] && [ "$unreachable" = true ]; } || [ "$code" = 000 ] || break | |
| 542 | + echo "starting the import returned $code ($(jq -r .message import.json 2>/dev/null)); trying again in 30 seconds" | |
| 543 | + sleep 30 | |
| 544 | + done | |
| 537 | 545 | if [ "$code" = 429 ]; then echo "::warning::GitHub's rate limit stopped the import check: $(jq -r .message import.json)"; exit 0; fi |
| 546 | + if [ "$code" = 502 ] && [ "$unreachable" = true ]; then echo "::warning::GitHub could not be reached, so the import check is skipped: $(jq -r .message import.json)"; exit 0; fi | |
| 538 | 547 | [ "$code" = 201 ] || { echo "::error::starting the import returned $code: $(cat import.json)"; exit 1; } |
| 539 | 548 | echo "IMPORT_REPO=$name" >> "$GITHUB_ENV" |
| 540 | 549 | id=$(jq -r .id import.json) |
| @@ -580,7 +589,8 @@ | ||
| 580 | 589 | [ "$runs" = 0 ] || { echo "::error::the import started $runs workflow runs"; exit 1; } |
| 581 | 590 | |
| 582 | 591 | - name: Scheduled workflow |
| 583 | - if: steps.token.outputs.skip != 'true' | |
| 592 | + # Runs even when the import check failed: the two are independent. | |
| 593 | + if: ${{ !cancelled() && steps.token.outputs.skip != 'true' }} | |
| 584 | 594 | run: | |
| 585 | 595 | set -euo pipefail |
| 586 | 596 | # The cron trigger fires every 5 minutes, so allow two windows. |
modifiedapps/diffinery/src/imports/github.ts+24−1View file
| @@ -50,6 +50,8 @@ | ||
| 50 | 50 | apiUrl: string, |
| 51 | 51 | private readonly token: string | null, |
| 52 | 52 | private readonly fetcher: typeof fetch = (input, init) => fetch(input, init), |
| 53 | + /** Pauses before retrying a read that met a dropped connection or a 502/503/504 (one retry per entry). */ | |
| 54 | + private readonly retryDelaysMs: readonly number[] = [500, 2000], | |
| 53 | 55 | ) { |
| 54 | 56 | this.base = new URL(apiUrl.replace(/\/+$/, "") + "/"); |
| 55 | 57 | } |
| @@ -66,6 +68,27 @@ | ||
| 66 | 68 | return u.pathname.slice(this.base.pathname.length) + u.search; |
| 67 | 69 | } |
| 68 | 70 | |
| 71 | + /** | |
| 72 | + * One request. Reads are retried a couple of times when GitHub or the path to it drops the | |
| 73 | + * connection or answers 502/503/504, which pass in seconds; a timeout is not retried. | |
| 74 | + */ | |
| 75 | + private async fetchRetrying(url: URL, method: string, init: RequestInit): Promise<Response> { | |
| 76 | + for (let attempt = 0; ; attempt++) { | |
| 77 | + const last = method !== "GET" || attempt >= this.retryDelaysMs.length; | |
| 78 | + let res: Response; | |
| 79 | + try { | |
| 80 | + res = await this.fetcher(url.toString(), { ...init, signal: AbortSignal.timeout(30_000) }); | |
| 81 | + } catch (e) { | |
| 82 | + if (last || (e instanceof Error && e.name === "TimeoutError")) throw e; | |
| 83 | + await new Promise((r) => setTimeout(r, this.retryDelaysMs[attempt])); | |
| 84 | + continue; | |
| 85 | + } | |
| 86 | + if (last || ![502, 503, 504].includes(res.status)) return res; | |
| 87 | + await res.body?.cancel().catch(() => undefined); | |
| 88 | + await new Promise((r) => setTimeout(r, this.retryDelaysMs[attempt])); | |
| 89 | + } | |
| 90 | + } | |
| 91 | + | |
| 69 | 92 | private async request(path: string, init: { method?: string; body?: unknown } = {}): Promise<Response> { |
| 70 | 93 | let url = this.resolve(path); |
| 71 | 94 | for (let hop = 0; hop < 4; hop++) { |
| @@ -81,7 +104,7 @@ | ||
| 81 | 104 | body = JSON.stringify(init.body); |
| 82 | 105 | headers["content-type"] = "application/json"; |
| 83 | 106 | } |
| 84 | - const res = await this.fetcher(url.toString(), { method, body, headers, redirect: "manual", signal: AbortSignal.timeout(30_000) }); | |
| 107 | + const res = await this.fetchRetrying(url, method, { method, body, headers, redirect: "manual" }); | |
| 85 | 108 | if ([301, 302, 307, 308].includes(res.status) && method === "GET") { |
| 86 | 109 | const loc = res.headers.get("location"); |
| 87 | 110 | if (!loc) throw new GitHubError(502, "GitHub redirected without a location"); |
modifiedapps/diffinery/test/unit/imports.test.ts+36−1View file
| @@ -11,7 +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 | +import { GitHubClient, GitHubError, RateLimited } from "../../src/imports/github.js"; | |
| 15 | 15 | |
| 16 | 16 | // ---------------------------------------------------------------- request and option validation |
| 17 | 17 | |
| @@ -374,6 +374,41 @@ | ||
| 374 | 374 | }); |
| 375 | 375 | }); |
| 376 | 376 | |
| 377 | +describe("transient GitHub failures are retried, real answers are not", () => { | |
| 378 | + const client = (answers: Array<number | Error>) => { | |
| 379 | + const calls: string[] = []; | |
| 380 | + const fetcher = (async (url: string) => { | |
| 381 | + calls.push(url); | |
| 382 | + const a = answers.shift() ?? 200; | |
| 383 | + if (a instanceof Error) throw a; | |
| 384 | + return new Response(JSON.stringify({ full_name: "octo/app" }), { status: a, headers: { "content-type": "application/json" } }); | |
| 385 | + }) as unknown as typeof fetch; | |
| 386 | + return { gh: new GitHubClient("https://api.github.com", null, fetcher, [0, 0]), calls }; | |
| 387 | + }; | |
| 388 | + | |
| 389 | + it("retries a dropped connection and a 503, then answers", async () => { | |
| 390 | + const { gh, calls } = client([new TypeError("Network connection lost."), 503]); | |
| 391 | + expect(await gh.get("repos/octo/app")).toEqual({ full_name: "octo/app" }); | |
| 392 | + expect(calls).toHaveLength(3); | |
| 393 | + }); | |
| 394 | + | |
| 395 | + it("gives up after its retries with GitHub's error", async () => { | |
| 396 | + const { gh, calls } = client([502, 503, 504]); | |
| 397 | + await expect(gh.get("repos/octo/app")).rejects.toBeInstanceOf(GitHubError); | |
| 398 | + expect(calls).toHaveLength(3); | |
| 399 | + }); | |
| 400 | + | |
| 401 | + it("does not retry a 404 or a timeout", async () => { | |
| 402 | + const a = client([404]); | |
| 403 | + await expect(a.gh.get("repos/octo/app")).rejects.toMatchObject({ status: 404 }); | |
| 404 | + expect(a.calls).toHaveLength(1); | |
| 405 | + const timeout = Object.assign(new Error("The operation was aborted due to timeout"), { name: "TimeoutError" }); | |
| 406 | + const b = client([timeout]); | |
| 407 | + await expect(b.gh.get("repos/octo/app")).rejects.toThrow(/timeout/); | |
| 408 | + expect(b.calls).toHaveLength(1); | |
| 409 | + }); | |
| 410 | +}); | |
| 411 | + | |
| 377 | 412 | // ---------------------------------------------------------------- texts that point at the web UI |
| 378 | 413 | |
| 379 | 414 | describe("import texts name what the web UI shows", () => { |