mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-03 19:04:05 +00:00
fix(retry): cap unsafe retry delays
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { MAX_SAFE_TIMEOUT_DELAY_MS } from "../utils/timer-delay.js";
|
||||
import { resolveRetryConfig, retryAsync } from "./retry.js";
|
||||
|
||||
const randomMocks = vi.hoisted(() => ({
|
||||
@@ -193,6 +194,60 @@ describe("retryAsync", () => {
|
||||
expect(fn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("falls back to the default attempt count for malformed numeric overloads", async () => {
|
||||
const fn = vi.fn().mockRejectedValue(new Error("boom"));
|
||||
await expect(runRetryNumberCase(fn, Number.NaN, 0)).rejects.toThrow("boom");
|
||||
expect(fn).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("caps numeric overload delays to the safe timer range", async () => {
|
||||
vi.clearAllTimers();
|
||||
vi.useFakeTimers();
|
||||
const timeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const fn = vi.fn().mockRejectedValueOnce(new Error("boom")).mockResolvedValueOnce("ok");
|
||||
try {
|
||||
const promise = retryAsync(fn, 2, 3_000_000_000);
|
||||
await vi.advanceTimersByTimeAsync(MAX_SAFE_TIMEOUT_DELAY_MS);
|
||||
await expect(promise).resolves.toBe("ok");
|
||||
expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_SAFE_TIMEOUT_DELAY_MS);
|
||||
} finally {
|
||||
timeoutSpy.mockRestore();
|
||||
vi.clearAllTimers();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps overflowed numeric overload backoff delays at the safe timer ceiling", async () => {
|
||||
vi.clearAllTimers();
|
||||
vi.useFakeTimers();
|
||||
const timeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const fn = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error("boom-1"))
|
||||
.mockRejectedValueOnce(new Error("boom-2"))
|
||||
.mockResolvedValueOnce("ok");
|
||||
try {
|
||||
const promise = retryAsync(fn, 3, Number.MAX_VALUE);
|
||||
await vi.advanceTimersByTimeAsync(MAX_SAFE_TIMEOUT_DELAY_MS);
|
||||
await vi.advanceTimersByTimeAsync(MAX_SAFE_TIMEOUT_DELAY_MS);
|
||||
await expect(promise).resolves.toBe("ok");
|
||||
expect(timeoutSpy).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.any(Function),
|
||||
MAX_SAFE_TIMEOUT_DELAY_MS,
|
||||
);
|
||||
expect(timeoutSpy).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.any(Function),
|
||||
MAX_SAFE_TIMEOUT_DELAY_MS,
|
||||
);
|
||||
} finally {
|
||||
timeoutSpy.mockRestore();
|
||||
vi.clearAllTimers();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "uses retryAfterMs when provided",
|
||||
@@ -261,6 +316,19 @@ describe("resolveRetryConfig", () => {
|
||||
},
|
||||
expected: { attempts: 3, minDelayMs: 300, maxDelayMs: 30000, jitter: 1 },
|
||||
},
|
||||
{
|
||||
name: "caps huge retry delays to the safe timer range",
|
||||
overrides: {
|
||||
minDelayMs: 3_000_000_000,
|
||||
maxDelayMs: 4_000_000_000,
|
||||
},
|
||||
expected: {
|
||||
attempts: 3,
|
||||
minDelayMs: MAX_SAFE_TIMEOUT_DELAY_MS,
|
||||
maxDelayMs: MAX_SAFE_TIMEOUT_DELAY_MS,
|
||||
jitter: 0,
|
||||
},
|
||||
},
|
||||
])("$name", ({ overrides, expected }) => {
|
||||
expect(resolveRetryConfig(undefined, overrides)).toEqual(expected);
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { asFiniteNumber } from "../shared/number-coercion.js";
|
||||
import { sleep } from "../utils.js";
|
||||
import { MAX_SAFE_TIMEOUT_DELAY_MS, resolveSafeTimeoutDelayMs } from "../utils/timer-delay.js";
|
||||
import { generateSecureFraction } from "./secure-random.js";
|
||||
|
||||
export type RetryConfig = {
|
||||
@@ -41,18 +42,32 @@ const clampNumber = (value: unknown, fallback: number, min?: number, max?: numbe
|
||||
return Math.min(Math.max(next, floor), ceiling);
|
||||
};
|
||||
|
||||
function resolveAttemptCount(value: unknown, fallback: number): number {
|
||||
const candidate = typeof value === "number" && Number.isFinite(value) ? value : fallback;
|
||||
return Math.max(1, Math.round(candidate));
|
||||
}
|
||||
|
||||
function resolveRetryDelayMs(value: number): number {
|
||||
if (value === Number.POSITIVE_INFINITY) {
|
||||
return MAX_SAFE_TIMEOUT_DELAY_MS;
|
||||
}
|
||||
return resolveSafeTimeoutDelayMs(value, { minMs: 0 });
|
||||
}
|
||||
|
||||
export function resolveRetryConfig(
|
||||
defaults: Required<RetryConfig> = DEFAULT_RETRY_CONFIG,
|
||||
overrides?: RetryConfig,
|
||||
): Required<RetryConfig> {
|
||||
const attempts = Math.max(1, Math.round(clampNumber(overrides?.attempts, defaults.attempts, 1)));
|
||||
const minDelayMs = Math.max(
|
||||
0,
|
||||
const attempts = resolveAttemptCount(
|
||||
clampNumber(overrides?.attempts, defaults.attempts, 1),
|
||||
defaults.attempts,
|
||||
);
|
||||
const minDelayMs = resolveRetryDelayMs(
|
||||
Math.round(clampNumber(overrides?.minDelayMs, defaults.minDelayMs, 0)),
|
||||
);
|
||||
const maxDelayMs = Math.max(
|
||||
minDelayMs,
|
||||
Math.round(clampNumber(overrides?.maxDelayMs, defaults.maxDelayMs, 0)),
|
||||
resolveRetryDelayMs(Math.round(clampNumber(overrides?.maxDelayMs, defaults.maxDelayMs, 0))),
|
||||
);
|
||||
const jitter = clampNumber(overrides?.jitter, defaults.jitter, 0, 1);
|
||||
return { attempts, minDelayMs, maxDelayMs, jitter };
|
||||
@@ -88,7 +103,7 @@ export async function retryAsync<T>(
|
||||
initialDelayMs = 300,
|
||||
): Promise<T> {
|
||||
if (typeof attemptsOrOptions === "number") {
|
||||
const attempts = Math.max(1, Math.round(attemptsOrOptions));
|
||||
const attempts = resolveAttemptCount(attemptsOrOptions, DEFAULT_RETRY_CONFIG.attempts);
|
||||
let lastErr: unknown;
|
||||
for (let i = 0; i < attempts; i += 1) {
|
||||
try {
|
||||
@@ -98,7 +113,7 @@ export async function retryAsync<T>(
|
||||
if (i === attempts - 1) {
|
||||
break;
|
||||
}
|
||||
const delay = initialDelayMs * 2 ** i;
|
||||
const delay = resolveRetryDelayMs(initialDelayMs * 2 ** i);
|
||||
await sleep(delay);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user