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
10 changes: 9 additions & 1 deletion src/review/content-lane/netuid-verification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ const RETRYABLE_STATUS = new Set([408, 425, 429, 500, 502, 503, 504]);

const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));

let netuidRetryBaseDelayMsOverride: number | null = null;

/** Test-only: collapses fetchWithRetry's exponential backoff to near-zero so a retry/exhaustion test
* doesn't pay real wall-clock time (the DEFAULT_BASE_DELAY_MS constant and production default are unchanged). */
export function setNetuidRetryBaseDelayMsForTest(value: number | null): void {
netuidRetryBaseDelayMsOverride = value;
}

/** Minimal fetch-with-retry (inlined from reviewbot core/fetch-retry.ts defaults). Retries on a
* thrown error or a retryable status, with exponential backoff + a per-attempt timeout. */
async function fetchWithRetry(
Expand All @@ -46,7 +54,7 @@ async function fetchWithRetry(
opts: { retries?: number; baseDelayMs?: number; timeoutMs?: number } = {},
): Promise<Response> {
const retries = opts.retries ?? DEFAULT_RETRIES;
const baseDelayMs = opts.baseDelayMs ?? DEFAULT_BASE_DELAY_MS;
const baseDelayMs = opts.baseDelayMs ?? netuidRetryBaseDelayMsOverride ?? DEFAULT_BASE_DELAY_MS;
const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS;
let lastError: unknown;
for (let attempt = 0; attempt <= retries; attempt += 1) {
Expand Down
14 changes: 13 additions & 1 deletion src/review/enrichment-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ function sharedSecretWasNormalized(
const REES_PING_NOT_READY_RETRIES = 2;
const REES_PING_NOT_READY_RETRY_DELAY_MS = 500;

let reesPingNotReadyRetryDelayMsOverride: number | null = null;

/** Test-only: collapses the real inter-retry wait so probeReesSecretAtStartup's retry tests don't pay
* REES_PING_NOT_READY_RETRIES * REES_PING_NOT_READY_RETRY_DELAY_MS of real wall-clock time. */
export function setReesPingNotReadyRetryDelayMsForTest(value: number | null): void {
reesPingNotReadyRetryDelayMsOverride = value;
}

function reesPingNotReadyRetryDelayMs(): number {
return reesPingNotReadyRetryDelayMsOverride ?? REES_PING_NOT_READY_RETRY_DELAY_MS;
}

async function fetchReesPingWithRetry(url: string, secret: string): Promise<Response> {
const request = () =>
fetch(url, {
Expand All @@ -85,7 +97,7 @@ async function fetchReesPingWithRetry(url: string, secret: string): Promise<Resp
});
let response = await request();
for (let attempt = 0; attempt < REES_PING_NOT_READY_RETRIES && response.status === 503; attempt += 1) {
await new Promise((resolve) => setTimeout(resolve, REES_PING_NOT_READY_RETRY_DELAY_MS));
await new Promise((resolve) => setTimeout(resolve, reesPingNotReadyRetryDelayMs()));
response = await request();
}
return response;
Expand Down
11 changes: 10 additions & 1 deletion src/selfhost/pg-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,26 @@ function isPgSqlStateConnectionError(err: unknown): boolean {
return hasErrorCode(err, PG_SQLSTATE_CONNECTION_CODES);
}

let pgRetryPoolQueryDelayMsOverride: number | null = null;

/** Test-only: collapses retryPoolQuery's per-attempt backoff to near-zero so a connection-error retry
* test doesn't pay real wall-clock time (the delayMs default and production behavior are unchanged). */
export function setPgRetryPoolQueryDelayMsForTest(value: number | null): void {
pgRetryPoolQueryDelayMsOverride = value;
}

/** Retry a pool query up to `retries` times on transient connection errors, with a short delay
* between attempts. The pool will establish a new connection automatically. */
async function retryPoolQuery<T>(fn: () => Promise<T>, retries = 3, delayMs = 500): Promise<T> {
const effectiveDelayMs = pgRetryPoolQueryDelayMsOverride ?? delayMs;
let lastErr: unknown;
for (let attempt = 0; attempt <= retries; attempt++) {
try {
return await fn();
} catch (err) {
lastErr = err;
if (!isPgConnectionError(err) || attempt === retries) throw err;
await new Promise((resolve) => setTimeout(resolve, delayMs * (attempt + 1)));
await new Promise((resolve) => setTimeout(resolve, effectiveDelayMs * (attempt + 1)));
}
}
throw lastErr;
Expand Down
4 changes: 4 additions & 0 deletions test/helpers/vitest-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
import { setReviewFilesEmptyRetryDelayMsForTest } from "../../src/github/backfill";
import { setGithubRateLimitRetrySleepCapMsForTest } from "../../src/github/client";
import { setMergeStateUnknownRetryDelayMsForTest } from "../../src/queue/ci-resolution";
import { setReesPingNotReadyRetryDelayMsForTest } from "../../src/review/enrichment-wire";
import { setNetuidRetryBaseDelayMsForTest } from "../../src/review/content-lane/netuid-verification";

setReviewFilesEmptyRetryDelayMsForTest(0);
setGithubRateLimitRetrySleepCapMsForTest(0);
setMergeStateUnknownRetryDelayMsForTest(0);
setReesPingNotReadyRetryDelayMsForTest(0);
setNetuidRetryBaseDelayMsForTest(0);
8 changes: 4 additions & 4 deletions test/unit/enrichment-wire.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,9 @@ describe("probeReesSecretAtStartup", () => {
globalThis.fetch = fetchSpy as unknown as typeof fetch;
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
await new Promise((resolve) => setTimeout(resolve, 1100));
expect(fetchSpy).toHaveBeenCalledTimes(3); // the first attempt + 2 retries, all still 503
// fetchReesPingWithRetry is fire-and-forget; poll instead of sleeping the retry budget's worst case
// (the test's own vitest-setup override collapses the real inter-retry delay to 0, so this settles fast).
await vi.waitFor(() => expect(fetchSpy).toHaveBeenCalledTimes(3)); // the first attempt + 2 retries, all still 503
const parsed = errSpy.mock.calls.map((c) => JSON.parse(c[0] as string));
expect(parsed.some((p) => p.event === "rees_ping_error" && p.status === 503)).toBe(true);
errSpy.mockRestore();
Expand All @@ -167,8 +168,7 @@ describe("probeReesSecretAtStartup", () => {
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
probeReesSecretAtStartup(env({ REES_URL: "https://rees.example", REES_SHARED_SECRET: "s3cret" }));
await new Promise((resolve) => setTimeout(resolve, 1100));
expect(fetchSpy).toHaveBeenCalledTimes(2);
await vi.waitFor(() => expect(fetchSpy).toHaveBeenCalledTimes(2));
expect(logSpy.mock.calls.some((c) => JSON.parse(c[0] as string).event === "rees_ping_ok")).toBe(true);
expect(errSpy).not.toHaveBeenCalled();
logSpy.mockRestore();
Expand Down
7 changes: 6 additions & 1 deletion test/unit/selfhost-pg-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Real-Postgres integration paths (migrations, pg-adapter translation) live in test/integration/selfhost-pg.test.ts.
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import type { Pool, QueryResult } from "pg";
import { createPgQueue } from "../../src/selfhost/pg-queue";
import { createPgQueue, setPgRetryPoolQueryDelayMsForTest } from "../../src/selfhost/pg-queue";
import { queueSnapshotFromBinding } from "../../src/selfhost/queue-common";
import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics";
import { RetryableJobError } from "../../src/queue/retryable";
Expand All @@ -15,6 +15,11 @@ import type { JobMessage } from "../../src/types";
// "unavailable" (null, never gates) here; individual host-load tests override the mock explicitly.
vi.mock("../../src/selfhost/host-pressure", () => ({ hostLoadAvg1PerCore: vi.fn(() => null) }));

// The PG connection-resilience tests below deliberately trigger retryPoolQuery's real ECONNRESET retry
// path; collapse its per-attempt backoff to near-zero so they don't pay real wall-clock time for it
// (the delayMs default and production behavior in src/selfhost/pg-queue.ts are unchanged).
setPgRetryPoolQueryDelayMsForTest(0);

const msg = (t: string): JobMessage => ({ type: t }) as unknown as JobMessage;
const webhook = (sender: { login: string; type: string }, eventName = "issue_comment", action = "edited"): JobMessage =>
({
Expand Down
Loading