Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/gittensory-miner/lib/ci-poller.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { fetchWithRetry } from "./http-retry.js";

const defaultApiBaseUrl = "https://api.github.com";
const defaultMinIntervalMs = 60_000;
const defaultMaxIntervalMs = 5 * 60_000;
Expand Down Expand Up @@ -82,10 +84,14 @@ function githubError(response, payload) {
}

async function githubGetJsonResponse(url, options) {
const response = await options.fetchFn(url, {
method: "GET",
headers: githubHeaders(options.githubToken),
});
// Retry transient network errors / 5xx around this single call (#4829), distinct from the poller's own
// pending-retry loop; the poller's injected sleepFn keeps tests instant.
const response = await fetchWithRetry(
options.fetchFn,
url,
{ method: "GET", headers: githubHeaders(options.githubToken) },
{ sleepFn: options.sleepFn },
);
const payload = await response.json().catch(() => null);
if (!response.ok) {
throw githubError(response, payload);
Expand Down
4 changes: 3 additions & 1 deletion packages/gittensory-miner/lib/gate-verdict-poller.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// gate verdict are two different signals a caller can record independently.
//
// Fully testable via injected `fetchFn`/`sleepFn` (mirrors `ci-poller.js`) — no real network in tests.
import { fetchWithRetry } from "./http-retry.js";

/** The typed gate verdicts, decided ones first, `pending` (not-yet-decided) last. */
export const GATE_VERDICTS = Object.freeze(["merge", "close", "hold", "pending"]);
Expand Down Expand Up @@ -86,7 +87,8 @@ export async function pollGateVerdict(url, options = {}) {

let latest = { verdict: "pending", disposition: null, attempts: 0, body: null };
for (let attempt = 0; attempt < maxAttempts; attempt += 1) {
const response = await fetchFn(url, { headers });
// Retry transient network errors / 5xx around this single call (#4829), distinct from the pending-retry loop.
const response = await fetchWithRetry(fetchFn, url, { headers }, { sleepFn });
if (!response || !response.ok) throw new Error(`gate_verdict_http_${response ? response.status : "error"}`);
const body = await response.json();
const disposition = readGateDisposition(body);
Expand Down
12 changes: 12 additions & 0 deletions packages/gittensory-miner/lib/http-retry.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export function defaultRetryBackoffMs(attempt: number): number;

export function fetchWithRetry<Response extends { status: number }>(
fetchFn: (url: unknown, init?: unknown) => Promise<Response>,
url: unknown,
init?: unknown,
options?: {
maxAttempts?: number;
sleepFn?: (ms: number) => Promise<unknown>;
backoffMs?: (attempt: number) => number;
},
): Promise<Response>;
52 changes: 52 additions & 0 deletions packages/gittensory-miner/lib/http-retry.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Bounded retry-with-backoff around a single HTTP call (#4829). The miner's pollers (ci-poller, gate-verdict-
// poller) previously let a single brief 5xx from GitHub kill the whole poll loop, because their own attempt loop
// only re-polls while a conclusion is genuinely "pending", never after a server error. This wraps ONE fetch so a
// transient SERVER error (a 5xx RESPONSE) is retried a bounded number of times, DISTINCT from that pending-
// polling, sleeping an exponential backoff between attempts and giving up after `maxAttempts`. A 2xx/3xx/4xx
// response is returned immediately, and a THROWN error (a network-level failure) propagates unchanged rather than
// being retried — the pollers' existing failure-mode contract (#4281) deliberately bubbles those to the caller.
// Pure control flow over injected `fetchFn`/`sleepFn`/`backoffMs` — no real network or timers in tests.

const DEFAULT_MAX_ATTEMPTS = 3;
const DEFAULT_BASE_BACKOFF_MS = 500;
const MAX_BACKOFF_MS = 10_000;

/** Clamp `maxAttempts` to a positive integer, flooring BEFORE the positivity test so a fractional value below 1
* falls back to the default rather than becoming a 0 that would skip every attempt. */
function normalizeMaxAttempts(raw) {
const numeric = Math.floor(Number(raw));
return Number.isFinite(numeric) && numeric >= 1 ? numeric : DEFAULT_MAX_ATTEMPTS;
}

/** Exponential backoff from a base delay, capped: attempt 1 → base, 2 → 2×base, 3 → 4×base, … ≤ MAX_BACKOFF_MS. */
export function defaultRetryBackoffMs(attempt) {
return Math.min(MAX_BACKOFF_MS, DEFAULT_BASE_BACKOFF_MS * 2 ** (Math.max(1, attempt) - 1));
}

const defaultSleep = (delayMs) => new Promise((resolve) => setTimeout(resolve, delayMs));

/**
* Perform `fetchFn(url, init)` with bounded retry on a transient 5xx response. A 5xx is retried (sleeping
* `backoffMs(attempt)` between attempts) up to `maxAttempts`; a 2xx/3xx/4xx response is returned immediately, and
* after the last attempt a lingering 5xx is returned as-is (the caller's own error handling still runs). A THROWN
* error is NOT retried — it propagates to the caller (the pollers' #4281 failure-mode contract).
*
* @param {(url: any, init?: any) => Promise<any>} fetchFn
* @param {any} url
* @param {any} [init]
* @param {{ maxAttempts?: number, sleepFn?: (ms: number) => Promise<unknown>, backoffMs?: (attempt: number) => number }} [options]
* @returns {Promise<any>} the fetch response
*/
export async function fetchWithRetry(fetchFn, url, init, options = {}) {
const maxAttempts = normalizeMaxAttempts(options.maxAttempts);
const sleepFn = typeof options.sleepFn === "function" ? options.sleepFn : defaultSleep;
const backoffMs = typeof options.backoffMs === "function" ? options.backoffMs : defaultRetryBackoffMs;
for (let attempt = 1; ; attempt += 1) {
// A thrown error is intentionally NOT caught here — it propagates to the caller unchanged.
const response = await fetchFn(url, init);
// Retry only transient SERVER errors (5xx). Return 2xx/3xx/4xx immediately; on the final attempt a lingering
// 5xx is returned as-is.
if (response.status < 500 || attempt >= maxAttempts) return response;
await sleepFn(backoffMs(attempt));
}
}
22 changes: 22 additions & 0 deletions test/unit/miner-ci-poller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,28 @@ describe("miner CI check-run poller (#2323)", () => {
).toBe(true);
});

it("retries a transient 5xx from GitHub during the poll and completes (#4829)", async () => {
let checkRunsAttempts = 0;
const fetchFn = vi.fn(async (input: RequestInfo | URL) => {
const url = String(input);
if (url.endsWith("/repos/acme/widgets/pulls/42")) return prResponse("head-sha");
if (url.includes("/check-runs")) {
checkRunsAttempts += 1;
if (checkRunsAttempts === 1) return jsonResponse({}, { status: 503 }); // a brief transient server error
return checksResponse([checkRun("validate", "completed", "success")]);
}
return jsonResponse({}, { status: 404 });
});
const result = await pollCheckRuns("acme/widgets", 42, {
apiBaseUrl: API,
githubToken: "github-token",
fetchFn,
sleepFn: () => Promise.resolve(), // no real backoff delay in the test
});
expect(checkRunsAttempts).toBe(2); // the 503 was retried, then succeeded
expect(result.conclusion).toBe("success"); // the poll completed despite the transient 5xx
});

it("rejects untrusted apiBaseUrl values before any token-bearing request", async () => {
const fetchFn = vi.fn();
for (const apiBaseUrl of [
Expand Down
92 changes: 92 additions & 0 deletions test/unit/miner-http-retry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { describe, expect, it } from "vitest";
import { defaultRetryBackoffMs, fetchWithRetry } from "../../packages/gittensory-miner/lib/http-retry.js";

const noSleep = () => Promise.resolve();

/** A fetchFn driven by a scripted list of behaviours: a number ⇒ a response with that status, "throw" ⇒ reject. */
function scriptedFetch(script: Array<number | "throw">) {
let call = 0;
const calls: unknown[] = [];
const fn = async (url: unknown) => {
calls.push(url);
const step = script[Math.min(call, script.length - 1)] ?? 200;
call += 1;
if (step === "throw") throw new Error("network down");
return { status: step };
};
return { fn, get calls() { return calls; } };
}

describe("fetchWithRetry (#4829)", () => {
it("returns a 2xx response on the first attempt with no retry", async () => {
const f = scriptedFetch([200]);
const res = await fetchWithRetry(f.fn, "u", undefined, { sleepFn: noSleep });
expect(res.status).toBe(200);
expect(f.calls).toHaveLength(1);
});

it("returns a 4xx immediately without retrying (client errors are not transient)", async () => {
const f = scriptedFetch([404, 200]);
const res = await fetchWithRetry(f.fn, "u", undefined, { sleepFn: noSleep });
expect(res.status).toBe(404);
expect(f.calls).toHaveLength(1);
});

it("retries a transient 5xx and returns the eventual success", async () => {
const f = scriptedFetch([503, 500, 200]);
const res = await fetchWithRetry(f.fn, "u", undefined, { maxAttempts: 5, sleepFn: noSleep });
expect(res.status).toBe(200);
expect(f.calls).toHaveLength(3);
});

it("does NOT retry a thrown network error — it propagates immediately (poller #4281 contract)", async () => {
const f = scriptedFetch(["throw", 200]);
await expect(fetchWithRetry(f.fn, "u", undefined, { sleepFn: noSleep })).rejects.toThrow("network down");
expect(f.calls).toHaveLength(1); // no retry on a thrown error
});

it("returns the last 5xx response once attempts are exhausted (caller handles it)", async () => {
const f = scriptedFetch([500]);
const res = await fetchWithRetry(f.fn, "u", undefined, { maxAttempts: 3, sleepFn: noSleep });
expect(res.status).toBe(500);
expect(f.calls).toHaveLength(3);
});

it("makes exactly one attempt when maxAttempts is 1 (returns the 5xx, no retry)", async () => {
const f = scriptedFetch([500, 200]);
const res = await fetchWithRetry(f.fn, "u", undefined, { maxAttempts: 1, sleepFn: noSleep });
expect(res.status).toBe(500);
expect(f.calls).toHaveLength(1);
});

it("uses the built-in sleep + backoff when none are injected (0ms backoff keeps the test fast)", async () => {
// No sleepFn and no backoffMs ⇒ exercises the default sleep (real setTimeout) with a 0ms delay, via a 5xx retry.
const f = scriptedFetch([500, 200]);
const res = await fetchWithRetry(f.fn, "u", undefined, { maxAttempts: 2, backoffMs: () => 0 });
expect(res.status).toBe(200);
expect(f.calls).toHaveLength(2);
});

it("sleeps the backoff between attempts and floors a bad maxAttempts to the default", async () => {
const delays: number[] = [];
const sleepFn = (ms: number) => {
delays.push(ms);
return Promise.resolve();
};
const f = scriptedFetch([500]); // always 5xx ⇒ retries until the (default 3) attempts are used up
// maxAttempts "0.5" floors to 0 ⇒ below 1 ⇒ falls back to the default of 3, so 2 sleeps happen.
await fetchWithRetry(f.fn, "u", undefined, { maxAttempts: 0.5 as unknown as number, sleepFn, backoffMs: (a) => a * 10 });
expect(f.calls).toHaveLength(3);
expect(delays).toEqual([10, 20]);
});
});

describe("defaultRetryBackoffMs (#4829)", () => {
it("grows exponentially from the base and caps", () => {
expect(defaultRetryBackoffMs(1)).toBe(500);
expect(defaultRetryBackoffMs(2)).toBe(1000);
expect(defaultRetryBackoffMs(3)).toBe(2000);
expect(defaultRetryBackoffMs(100)).toBe(10_000); // capped
expect(defaultRetryBackoffMs(0)).toBe(500); // attempt clamped to >= 1
});
});