compaction & branch summarization follow retry policy
fixes #6647 compaction (auto & manual) and branch summarization retry on transient failures. use the same retry policy from settings. emit events for the tui to show indication of retries
This commit is contained in:
David Brailovsky
@@ -85,6 +85,116 @@ const RETRYABLE_PROVIDER_ERROR_PATTERN = buildProviderErrorPattern([
|
||||
"ResourceExhausted",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Retry policy: bounded attempts with exponential backoff (`baseDelayMs * 2^(attempt-1)`).
|
||||
* Matches `settings.retry` (`enabled`, `maxRetries`, `baseDelayMs`) in coding-agent; kept
|
||||
* here so the classifier and the policy-driven retry loop live together and stay reusable
|
||||
* by the SDK and other callers.
|
||||
*/
|
||||
export interface RetryPolicy {
|
||||
enabled: boolean;
|
||||
/** Max retry attempts (0 = no retries). The initial call never counts as a retry. */
|
||||
maxRetries: number;
|
||||
/** Base delay in ms. Per-attempt delay is `baseDelayMs * 2^(attempt-1)` before jitter. */
|
||||
baseDelayMs: number;
|
||||
}
|
||||
|
||||
/** Optional callbacks emitted by {@link retryAssistantCall} around each retry. */
|
||||
export interface RetryCallbacks {
|
||||
/** Emitted before the backoff sleep of each retry attempt (1-indexed). */
|
||||
onRetry?: (attempt: number, maxAttempts: number, delayMs: number, errorMessage: string) => void;
|
||||
/** Emitted after the backoff sleep, immediately before the retried call starts. */
|
||||
onRetryAttemptStart?: () => void;
|
||||
/** Emitted once when the loop ends: success if a later call returned a non-error message. */
|
||||
onRetryEnd?: (success: boolean, attempt: number, finalError?: string) => void;
|
||||
}
|
||||
|
||||
class RetrySleepAbortError extends Error {
|
||||
constructor() {
|
||||
super("Aborted");
|
||||
}
|
||||
}
|
||||
|
||||
function sleep(ms: number, signal?: AbortSignal): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
if (signal?.aborted) {
|
||||
reject(new RetrySleepAbortError());
|
||||
return;
|
||||
}
|
||||
const timeout = setTimeout(resolve, ms);
|
||||
signal?.addEventListener(
|
||||
"abort",
|
||||
() => {
|
||||
clearTimeout(timeout);
|
||||
reject(new RetrySleepAbortError());
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a single assistant-producing call with bounded retry on transient errors.
|
||||
*
|
||||
* Behavior:
|
||||
* - A non-error response (success or aborted) is returned immediately; aborts are
|
||||
* never retried. Aborts during the backoff sleep are normalized to an aborted
|
||||
* `AssistantMessage` too, so callers do not need to care when cancellation happened.
|
||||
* - A non-retryable error (per {@link isRetryableAssistantError}, including quota/
|
||||
* billing exhaustion) is returned immediately so deterministic errors fail fast.
|
||||
* - Otherwise retries up to `maxRetries` times with exponential backoff, emitting
|
||||
* `onRetry` before each sleep, `onRetryAttemptStart` after each sleep before the
|
||||
* retried call starts, and `onRetryEnd` once at the end (whether the loop ends in
|
||||
* success, exhausted retries, or an aborted backoff).
|
||||
*
|
||||
* When `policy` is undefined or disabled, the first response is returned unchanged
|
||||
* (equivalent to calling `produce()` directly).
|
||||
*/
|
||||
export async function retryAssistantCall(
|
||||
produce: () => Promise<AssistantMessage>,
|
||||
policy: RetryPolicy | undefined,
|
||||
signal: AbortSignal | undefined,
|
||||
callbacks?: RetryCallbacks,
|
||||
): Promise<AssistantMessage> {
|
||||
const maxAttempts = policy?.enabled ? policy.maxRetries : 0;
|
||||
|
||||
let attempt = 0;
|
||||
let lastRetry: { attempt: number; errorMessage: string } | undefined;
|
||||
for (;;) {
|
||||
const response = await produce();
|
||||
|
||||
// Success or abort: never retry an aborted message; non-error returns as-is.
|
||||
if (response.stopReason !== "error") {
|
||||
if (lastRetry) callbacks?.onRetryEnd?.(true, lastRetry.attempt);
|
||||
return response;
|
||||
}
|
||||
|
||||
// Non-retryable, or budget exhausted: return the final error message.
|
||||
if (attempt >= maxAttempts || !isRetryableAssistantError(response)) {
|
||||
if (lastRetry) callbacks?.onRetryEnd?.(false, lastRetry.attempt, response.errorMessage);
|
||||
return response;
|
||||
}
|
||||
|
||||
attempt++;
|
||||
lastRetry = { attempt, errorMessage: response.errorMessage || "Unknown error" };
|
||||
const delayMs = policy!.baseDelayMs * 2 ** (attempt - 1);
|
||||
callbacks?.onRetry?.(attempt, maxAttempts, delayMs, lastRetry.errorMessage);
|
||||
|
||||
// Normalize aborts during retry backoff to the same AssistantMessage shape as
|
||||
// provider stream aborts, so callers do not need to care when cancellation happened.
|
||||
try {
|
||||
await sleep(delayMs, signal);
|
||||
} catch (error) {
|
||||
callbacks?.onRetryEnd?.(false, attempt, lastRetry.errorMessage);
|
||||
if (error instanceof RetrySleepAbortError) {
|
||||
return { ...response, stopReason: "aborted", errorMessage: undefined };
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
callbacks?.onRetryAttemptStart?.();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Classifies whether a failed assistant message looks like a transient provider
|
||||
* or transport error, so callers can decide if the last assistant turn should be
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { fauxAssistantMessage } from "../src/providers/faux.ts";
|
||||
import { isRetryableAssistantError } from "../src/utils/retry.ts";
|
||||
import { isRetryableAssistantError, type RetryPolicy, retryAssistantCall } from "../src/utils/retry.ts";
|
||||
|
||||
const openAIExplicitRetryMessage =
|
||||
"An error occurred while processing your request. You can retry your request, or contact us through our help center at help.openai.com if the error persists. Please include the request ID req_******** in your message.";
|
||||
@@ -66,3 +66,121 @@ describe("provider retry classification", () => {
|
||||
expect(isRetryableAssistantError(fauxAssistantMessage("not an error"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("retryAssistantCall", () => {
|
||||
const disabled: RetryPolicy = { enabled: false, maxRetries: 3, baseDelayMs: 0 };
|
||||
const enabled: RetryPolicy = { enabled: true, maxRetries: 3, baseDelayMs: 0 };
|
||||
|
||||
it("returns a successful response immediately without retrying", async () => {
|
||||
const produce = vi.fn(async () => fauxAssistantMessage("ok"));
|
||||
const res = await retryAssistantCall(produce, enabled, undefined);
|
||||
expect(res.content).toEqual([{ type: "text", text: "ok" }]);
|
||||
expect(produce).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not retry an aborted message", async () => {
|
||||
const produce = vi.fn(async () => fauxAssistantMessage("", { stopReason: "aborted" }));
|
||||
const onRetry = vi.fn();
|
||||
const res = await retryAssistantCall(produce, enabled, undefined, { onRetry });
|
||||
expect(res.stopReason).toBe("aborted");
|
||||
expect(produce).toHaveBeenCalledTimes(1);
|
||||
expect(onRetry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not retry a non-retryable error (quota/billing)", async () => {
|
||||
const produce = vi.fn(async () =>
|
||||
fauxAssistantMessage("", { stopReason: "error", errorMessage: "insufficient_quota" }),
|
||||
);
|
||||
const onRetry = vi.fn();
|
||||
const onRetryEnd = vi.fn();
|
||||
const res = await retryAssistantCall(produce, enabled, undefined, { onRetry, onRetryEnd });
|
||||
expect(res.stopReason).toBe("error");
|
||||
expect(produce).toHaveBeenCalledTimes(1);
|
||||
expect(onRetry).not.toHaveBeenCalled();
|
||||
expect(onRetryEnd).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("retries a transient error up to maxRetries then returns the final error", async () => {
|
||||
const produce = vi.fn(async () => fauxAssistantMessage("", { stopReason: "error", errorMessage: "terminated" }));
|
||||
const onRetry = vi.fn();
|
||||
const onRetryEnd = vi.fn();
|
||||
const res = await retryAssistantCall(produce, enabled, undefined, { onRetry, onRetryEnd });
|
||||
expect(res.stopReason).toBe("error");
|
||||
expect(produce).toHaveBeenCalledTimes(4); // 1 initial + 3 retries
|
||||
expect(onRetry).toHaveBeenCalledTimes(3);
|
||||
expect(onRetryEnd).toHaveBeenCalledWith(false, 3, "terminated");
|
||||
});
|
||||
|
||||
it("stops retrying once a call succeeds", async () => {
|
||||
let n = 0;
|
||||
const produce = vi.fn(async () => {
|
||||
n++;
|
||||
return n < 3
|
||||
? fauxAssistantMessage("", { stopReason: "error", errorMessage: "terminated" })
|
||||
: fauxAssistantMessage("recovered");
|
||||
});
|
||||
const onRetryEnd = vi.fn();
|
||||
const res = await retryAssistantCall(produce, enabled, undefined, { onRetryEnd });
|
||||
expect(res.content).toEqual([{ type: "text", text: "recovered" }]);
|
||||
expect(produce).toHaveBeenCalledTimes(3);
|
||||
expect(onRetryEnd).toHaveBeenCalledWith(true, 2);
|
||||
});
|
||||
|
||||
it("does not retry when policy is disabled", async () => {
|
||||
const produce = vi.fn(async () => fauxAssistantMessage("", { stopReason: "error", errorMessage: "terminated" }));
|
||||
const onRetry = vi.fn();
|
||||
const onRetryEnd = vi.fn();
|
||||
const res = await retryAssistantCall(produce, disabled, undefined, { onRetry, onRetryEnd });
|
||||
expect(res.stopReason).toBe("error");
|
||||
expect(produce).toHaveBeenCalledTimes(1);
|
||||
expect(onRetry).not.toHaveBeenCalled();
|
||||
expect(onRetryEnd).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("emits onRetryAttemptStart after backoff before each retried call", async () => {
|
||||
const events: string[] = [];
|
||||
let n = 0;
|
||||
const produce = vi.fn(async () => {
|
||||
events.push(`produce:${n}`);
|
||||
n++;
|
||||
return n < 3
|
||||
? fauxAssistantMessage("", { stopReason: "error", errorMessage: "terminated" })
|
||||
: fauxAssistantMessage("recovered");
|
||||
});
|
||||
const onRetry = vi.fn((attempt: number) => {
|
||||
events.push(`retry:${attempt}`);
|
||||
});
|
||||
const onRetryAttemptStart = vi.fn(() => {
|
||||
events.push("attempt-start");
|
||||
});
|
||||
const res = await retryAssistantCall(produce, enabled, undefined, { onRetry, onRetryAttemptStart });
|
||||
expect(res.content).toEqual([{ type: "text", text: "recovered" }]);
|
||||
expect(onRetry).toHaveBeenCalledTimes(2);
|
||||
expect(onRetryAttemptStart).toHaveBeenCalledTimes(2);
|
||||
expect(events).toEqual([
|
||||
"produce:0",
|
||||
"retry:1",
|
||||
"attempt-start",
|
||||
"produce:1",
|
||||
"retry:2",
|
||||
"attempt-start",
|
||||
"produce:2",
|
||||
]);
|
||||
});
|
||||
|
||||
it("aborts backoff sleep via signal, returns an aborted message, and emits onRetryEnd(false)", async () => {
|
||||
const controller = new AbortController();
|
||||
const produce = vi.fn(async () => fauxAssistantMessage("", { stopReason: "error", errorMessage: "terminated" }));
|
||||
const policy: RetryPolicy = { enabled: true, maxRetries: 5, baseDelayMs: 10_000 };
|
||||
const onRetryEnd = vi.fn();
|
||||
const p = retryAssistantCall(produce, policy, controller.signal, { onRetryEnd });
|
||||
// Let one error call resolve and the first backoff sleep start, then abort.
|
||||
await vi.waitFor(() => expect(produce).toHaveBeenCalled());
|
||||
controller.abort();
|
||||
const res = await p;
|
||||
expect(res.stopReason).toBe("aborted");
|
||||
expect(res.errorMessage).toBeUndefined();
|
||||
expect(produce).toHaveBeenCalledTimes(1);
|
||||
expect(onRetryEnd).toHaveBeenCalledWith(false, 1, "terminated");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user