From 4e876f6e127e2c3c8fecaf7a8418bb214282ea1f Mon Sep 17 00:00:00 2001 From: Psychotoxical <171614930+Psychotoxical@users.noreply.github.com> Date: Fri, 17 Jul 2026 02:16:02 +0200 Subject: [PATCH] fix(ci): tolerate transient GitHub API errors in ci-ok aggregate (#1317) --- .github/scripts/ci-ok-aggregate.mjs | 57 +++++++++++++++--- .github/scripts/ci-ok-aggregate.test.mjs | 74 ++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/.github/scripts/ci-ok-aggregate.mjs b/.github/scripts/ci-ok-aggregate.mjs index e3c9519e..f9dbc763 100644 --- a/.github/scripts/ci-ok-aggregate.mjs +++ b/.github/scripts/ci-ok-aggregate.mjs @@ -26,6 +26,33 @@ const POLL_MS = 30_000; const TIMEOUT_MS = 90 * 60 * 1000; const OK_CONCLUSIONS = new Set(['success', 'neutral', 'skipped']); +/** + * GitHub API hiccups (5xx, secondary rate limits, dropped connections) must + * not fail the gate — the answer is to poll again, not to go red while the + * real jobs are green. 4xx config errors (401/404 …) still throw. + */ +export function isTransientApiError(err) { + const status = typeof err?.status === 'number' ? err.status : 0; + return status === 0 || status === 429 || status >= 500; +} + +export async function withTransientRetry(label, fn, core, attempts = 5, delayMs = POLL_MS) { + for (let attempt = 1; ; attempt++) { + try { + return await fn(); + } catch (err) { + if (!isTransientApiError(err) || attempt >= attempts) { + throw err; + } + // err.message can be a whole HTML error page — log only the status. + core.info( + `${label}: transient API error (status=${err?.status ?? 'network'}), retry ${attempt}/${attempts - 1} in ${delayMs / 1000}s`, + ); + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + } +} + export function pathTriggersFrontend(file) { return FRONTEND_PATH_RE.test(file); } @@ -150,7 +177,11 @@ export async function runCiOkAggregate(github, context, core) { const { owner, repo } = context.repo; const sha = resolveTargetSha(context); const excludeRunId = String(context.runId); - const changedFiles = await listChangedFiles(github, context); + const changedFiles = await withTransientRetry( + 'listChangedFiles', + () => listChangedFiles(github, context), + core, + ); const required = requiredJobNames(changedFiles); core.info(`ci-ok @ ${sha}; ${changedFiles.length} changed file(s)`); @@ -162,12 +193,24 @@ export async function runCiOkAggregate(github, context, core) { const deadline = Date.now() + TIMEOUT_MS; while (Date.now() < deadline) { - const checksAll = await github.paginate(github.rest.checks.listForRef, { - owner, - repo, - ref: sha, - per_page: 100, - }); + let checksAll; + try { + checksAll = await github.paginate(github.rest.checks.listForRef, { + owner, + repo, + ref: sha, + per_page: 100, + }); + } catch (err) { + if (!isTransientApiError(err)) { + throw err; + } + // Same as an inconclusive poll: wait out the hiccup, the 90-minute + // deadline stays the backstop. + core.info(`checks.listForRef: transient API error (status=${err?.status ?? 'network'}) — retrying next poll`); + await new Promise((resolve) => setTimeout(resolve, POLL_MS)); + continue; + } const newestByName = newestChecksByName(checksAll, excludeRunId); const { pending, failures, done } = evaluateRequiredJobs(required, newestByName); diff --git a/.github/scripts/ci-ok-aggregate.test.mjs b/.github/scripts/ci-ok-aggregate.test.mjs index ef86194f..d8e782e9 100644 --- a/.github/scripts/ci-ok-aggregate.test.mjs +++ b/.github/scripts/ci-ok-aggregate.test.mjs @@ -3,10 +3,12 @@ import test from 'node:test'; import { evaluateRequiredJobs, + isTransientApiError, newestChecksByName, pathTriggersFrontend, pathTriggersRust, requiredJobNames, + withTransientRetry, } from './ci-ok-aggregate.mjs'; test('pathTriggersFrontend matches frontend workflow paths', () => { @@ -52,3 +54,75 @@ test('evaluateRequiredJobs passes when all required jobs succeeded', () => { const result = evaluateRequiredJobs(['eslint', 'vitest run'], newestChecksByName(checks)); assert.equal(result.done, true); }); + +test('isTransientApiError treats 5xx, 429 and network errors as transient', () => { + assert.equal(isTransientApiError({ status: 503 }), true); + assert.equal(isTransientApiError({ status: 500 }), true); + assert.equal(isTransientApiError({ status: 429 }), true); + assert.equal(isTransientApiError(new Error('socket hang up')), true); + assert.equal(isTransientApiError({ status: 404 }), false); + assert.equal(isTransientApiError({ status: 401 }), false); +}); + +const silentCore = { info: () => {} }; + +test('withTransientRetry retries transient errors and returns the late success', async () => { + let calls = 0; + const result = await withTransientRetry( + 'test', + async () => { + calls += 1; + if (calls < 3) { + const err = new Error('unavailable'); + err.status = 503; + throw err; + } + return 'ok'; + }, + silentCore, + 5, + 1, + ); + assert.equal(result, 'ok'); + assert.equal(calls, 3); +}); + +test('withTransientRetry rethrows non-transient errors immediately', async () => { + let calls = 0; + await assert.rejects( + withTransientRetry( + 'test', + async () => { + calls += 1; + const err = new Error('not found'); + err.status = 404; + throw err; + }, + silentCore, + 5, + 1, + ), + /not found/, + ); + assert.equal(calls, 1); +}); + +test('withTransientRetry gives up after the attempt budget', async () => { + let calls = 0; + await assert.rejects( + withTransientRetry( + 'test', + async () => { + calls += 1; + const err = new Error('unavailable'); + err.status = 503; + throw err; + }, + silentCore, + 3, + 1, + ), + /unavailable/, + ); + assert.equal(calls, 3); +});