mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 14:55:43 +00:00
fix(ci): tolerate transient GitHub API errors in ci-ok aggregate (#1317)
This commit is contained in:
@@ -26,6 +26,33 @@ const POLL_MS = 30_000;
|
|||||||
const TIMEOUT_MS = 90 * 60 * 1000;
|
const TIMEOUT_MS = 90 * 60 * 1000;
|
||||||
const OK_CONCLUSIONS = new Set(['success', 'neutral', 'skipped']);
|
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) {
|
export function pathTriggersFrontend(file) {
|
||||||
return FRONTEND_PATH_RE.test(file);
|
return FRONTEND_PATH_RE.test(file);
|
||||||
}
|
}
|
||||||
@@ -150,7 +177,11 @@ export async function runCiOkAggregate(github, context, core) {
|
|||||||
const { owner, repo } = context.repo;
|
const { owner, repo } = context.repo;
|
||||||
const sha = resolveTargetSha(context);
|
const sha = resolveTargetSha(context);
|
||||||
const excludeRunId = String(context.runId);
|
const excludeRunId = String(context.runId);
|
||||||
const changedFiles = await listChangedFiles(github, context);
|
const changedFiles = await withTransientRetry(
|
||||||
|
'listChangedFiles',
|
||||||
|
() => listChangedFiles(github, context),
|
||||||
|
core,
|
||||||
|
);
|
||||||
const required = requiredJobNames(changedFiles);
|
const required = requiredJobNames(changedFiles);
|
||||||
|
|
||||||
core.info(`ci-ok @ ${sha}; ${changedFiles.length} changed file(s)`);
|
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;
|
const deadline = Date.now() + TIMEOUT_MS;
|
||||||
while (Date.now() < deadline) {
|
while (Date.now() < deadline) {
|
||||||
const checksAll = await github.paginate(github.rest.checks.listForRef, {
|
let checksAll;
|
||||||
owner,
|
try {
|
||||||
repo,
|
checksAll = await github.paginate(github.rest.checks.listForRef, {
|
||||||
ref: sha,
|
owner,
|
||||||
per_page: 100,
|
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 newestByName = newestChecksByName(checksAll, excludeRunId);
|
||||||
const { pending, failures, done } = evaluateRequiredJobs(required, newestByName);
|
const { pending, failures, done } = evaluateRequiredJobs(required, newestByName);
|
||||||
|
|
||||||
|
|||||||
@@ -3,10 +3,12 @@ import test from 'node:test';
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
evaluateRequiredJobs,
|
evaluateRequiredJobs,
|
||||||
|
isTransientApiError,
|
||||||
newestChecksByName,
|
newestChecksByName,
|
||||||
pathTriggersFrontend,
|
pathTriggersFrontend,
|
||||||
pathTriggersRust,
|
pathTriggersRust,
|
||||||
requiredJobNames,
|
requiredJobNames,
|
||||||
|
withTransientRetry,
|
||||||
} from './ci-ok-aggregate.mjs';
|
} from './ci-ok-aggregate.mjs';
|
||||||
|
|
||||||
test('pathTriggersFrontend matches frontend workflow paths', () => {
|
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));
|
const result = evaluateRequiredJobs(['eslint', 'vitest run'], newestChecksByName(checks));
|
||||||
assert.equal(result.done, true);
|
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);
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user