From f7cc46cd9f07138a3c7aa516ca1829caa5cdbb70 Mon Sep 17 00:00:00 2001 From: Aiden Cline <63023139+rekram1-node@users.noreply.github.com> Date: Sun, 9 Nov 2025 09:46:58 -0800 Subject: [PATCH] set cap for max time to wait between retries (#4135) Co-authored-by: GitHub Action --- packages/opencode/src/session/compaction.ts | 11 +- packages/opencode/src/session/prompt.ts | 11 +- packages/opencode/src/session/retry.ts | 78 +++++++---- packages/opencode/test/session/retry.test.ts | 139 ++++++++++++++++++- 4 files changed, 201 insertions(+), 38 deletions(-) diff --git a/packages/opencode/src/session/compaction.ts b/packages/opencode/src/session/compaction.ts index 504f7a4b..ff988845 100644 --- a/packages/opencode/src/session/compaction.ts +++ b/packages/opencode/src/session/compaction.ts @@ -267,15 +267,24 @@ export namespace SessionCompaction { max: maxRetries, }) if (result.shouldRetry) { + const start = Date.now() for (let retry = 1; retry < maxRetries; retry++) { const lastRetryPart = result.parts.findLast((p): p is MessageV2.RetryPart => p.type === "retry") if (lastRetryPart) { - const delayMs = SessionRetry.getRetryDelayInMs(lastRetryPart.error, retry) + const delayMs = SessionRetry.getBoundedDelay({ + error: lastRetryPart.error, + attempt: retry, + startTime: start, + }) + if (!delayMs) { + break + } log.info("retrying with backoff", { attempt: retry, delayMs, + elapsed: Date.now() - start, }) const stop = await SessionRetry.sleep(delayMs, signal) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index b746fdec..28afe61c 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -355,15 +355,24 @@ export namespace SessionPrompt { max: maxRetries, }) if (result.shouldRetry) { + const start = Date.now() for (let retry = 1; retry < maxRetries; retry++) { const lastRetryPart = result.parts.findLast((p): p is MessageV2.RetryPart => p.type === "retry") if (lastRetryPart) { - const delayMs = SessionRetry.getRetryDelayInMs(lastRetryPart.error, retry) + const delayMs = SessionRetry.getBoundedDelay({ + error: lastRetryPart.error, + attempt: retry, + startTime: start, + }) + if (!delayMs) { + break + } log.info("retrying with backoff", { attempt: retry, delayMs, + elapsed: Date.now() - start, }) const stop = await SessionRetry.sleep(delayMs, abort.signal) diff --git a/packages/opencode/src/session/retry.ts b/packages/opencode/src/session/retry.ts index 30f2035b..8bad7afa 100644 --- a/packages/opencode/src/session/retry.ts +++ b/packages/opencode/src/session/retry.ts @@ -1,8 +1,11 @@ +import { iife } from "@/util/iife" import { MessageV2 } from "./message-v2" export namespace SessionRetry { export const RETRY_INITIAL_DELAY = 2000 export const RETRY_BACKOFF_FACTOR = 2 + export const RETRY_MAX_DELAY = 600_000 // 10 minutes + export const RETRY_HEADER_BUFFER = 1000 // add 1s buffer to server-provided delays export async function sleep(ms: number, signal: AbortSignal): Promise { return new Promise((resolve, reject) => { @@ -18,40 +21,57 @@ export namespace SessionRetry { }) } - export function getRetryDelayInMs(error: MessageV2.APIError, attempt: number): number { - const base = RETRY_INITIAL_DELAY * Math.pow(RETRY_BACKOFF_FACTOR, attempt - 1) - const headers = error.data.responseHeaders - if (!headers) return base + export function getRetryDelayInMs(error: MessageV2.APIError, attempt: number) { + const delay = iife(() => { + const headers = error.data.responseHeaders + if (headers) { + const retryAfterMs = headers["retry-after-ms"] + if (retryAfterMs) { + const parsedMs = Number.parseFloat(retryAfterMs) + if (!Number.isNaN(parsedMs)) { + return parsedMs + } + } - const retryAfterMs = headers["retry-after-ms"] - if (retryAfterMs) { - const parsed = Number.parseFloat(retryAfterMs) - const normalized = normalizeDelay({ base, candidate: parsed }) - if (normalized != null) return normalized - } + const retryAfter = headers["retry-after"] + if (retryAfter) { + const parsedSeconds = Number.parseFloat(retryAfter) + if (!Number.isNaN(parsedSeconds)) { + // convert seconds to milliseconds + return Math.ceil(parsedSeconds * 1000) + } + // Try parsing as HTTP date format + const parsed = Date.parse(retryAfter) - Date.now() + if (!Number.isNaN(parsed) && parsed > 0) { + return Math.ceil(parsed) + } + } + } - const retryAfter = headers["retry-after"] - if (!retryAfter) return base + return RETRY_INITIAL_DELAY * Math.pow(RETRY_BACKOFF_FACTOR, attempt - 1) + }) - const seconds = Number.parseFloat(retryAfter) - if (!Number.isNaN(seconds)) { - const normalized = normalizeDelay({ base, candidate: seconds * 1000 }) - if (normalized != null) return normalized - return base - } + // dont retry if wait is too far from now + if (delay > RETRY_MAX_DELAY) return undefined - const dateMs = Date.parse(retryAfter) - Date.now() - const normalized = normalizeDelay({ base, candidate: dateMs }) - if (normalized != null) return normalized - - return base + return delay } - function normalizeDelay(input: { base: number; candidate: number }): number | undefined { - if (Number.isNaN(input.candidate)) return undefined - if (input.candidate < 0) return undefined - if (input.candidate < 60_000) return input.candidate - if (input.candidate < input.base) return input.candidate - return undefined + export function getBoundedDelay(input: { + error: MessageV2.APIError + attempt: number + startTime: number + maxDuration?: number + }) { + const elapsed = Date.now() - input.startTime + const maxDuration = input.maxDuration ?? RETRY_MAX_DELAY + const remaining = maxDuration - elapsed + + if (remaining <= 0) return undefined + + const delay = getRetryDelayInMs(input.error, input.attempt) + if (!delay) return undefined + + return Math.min(delay, remaining) } } diff --git a/packages/opencode/test/session/retry.test.ts b/packages/opencode/test/session/retry.test.ts index edce412c..27148e2a 100644 --- a/packages/opencode/test/session/retry.test.ts +++ b/packages/opencode/test/session/retry.test.ts @@ -13,8 +13,8 @@ function apiError(headers?: Record): MessageV2.APIError { describe("session.retry.getRetryDelayInMs", () => { test("doubles delay on each attempt when headers missing", () => { const error = apiError() - const delays = Array.from({ length: 7 }, (_, index) => SessionRetry.getRetryDelayInMs(error, index + 1)) - expect(delays).toStrictEqual([2000, 4000, 8000, 16000, 32000, 64000, 128000]) + const delays = Array.from({ length: 10 }, (_, index) => SessionRetry.getRetryDelayInMs(error, index + 1)) + expect(delays).toStrictEqual([2000, 4000, 8000, 16000, 32000, 64000, 128000, 256000, 512000, undefined]) }) test("prefers retry-after-ms when shorter than exponential", () => { @@ -27,11 +27,6 @@ describe("session.retry.getRetryDelayInMs", () => { expect(SessionRetry.getRetryDelayInMs(error, 3)).toBe(30000) }) - test("falls back to exponential when server delay is long", () => { - const error = apiError({ "retry-after": "120" }) - expect(SessionRetry.getRetryDelayInMs(error, 2)).toBe(4000) - }) - test("accepts http-date retry-after values", () => { const date = new Date(Date.now() + 20000).toUTCString() const error = apiError({ "retry-after": date }) @@ -44,4 +39,134 @@ describe("session.retry.getRetryDelayInMs", () => { const error = apiError({ "retry-after": "not-a-number" }) expect(SessionRetry.getRetryDelayInMs(error, 1)).toBe(2000) }) + + test("ignores malformed date retry hints", () => { + const error = apiError({ "retry-after": "Invalid Date String" }) + expect(SessionRetry.getRetryDelayInMs(error, 1)).toBe(2000) + }) + + test("ignores past date retry hints", () => { + const pastDate = new Date(Date.now() - 5000).toUTCString() + const error = apiError({ "retry-after": pastDate }) + expect(SessionRetry.getRetryDelayInMs(error, 1)).toBe(2000) + }) + + test("returns undefined when delay exceeds 10 minutes", () => { + const error = apiError() + expect(SessionRetry.getRetryDelayInMs(error, 10)).toBeUndefined() + }) + + test("returns undefined when retry-after exceeds 10 minutes", () => { + const error = apiError({ "retry-after": "50" }) + expect(SessionRetry.getRetryDelayInMs(error, 1)).toBe(50000) + + const longError = apiError({ "retry-after-ms": "700000" }) + expect(SessionRetry.getRetryDelayInMs(longError, 1)).toBeUndefined() + }) +}) + +describe("session.retry.getBoundedDelay", () => { + test("returns full delay when under time budget", () => { + const error = apiError() + const startTime = Date.now() + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + }) + expect(delay).toBe(2000) + }) + + test("returns remaining time when delay exceeds budget", () => { + const error = apiError() + const startTime = Date.now() - 598_000 // 598 seconds elapsed, 2 seconds remaining + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + }) + expect(delay).toBeGreaterThanOrEqual(1900) + expect(delay).toBeLessThanOrEqual(2100) + }) + + test("returns undefined when time budget exhausted", () => { + const error = apiError() + const startTime = Date.now() - 600_000 // exactly 10 minutes elapsed + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + }) + expect(delay).toBeUndefined() + }) + + test("returns undefined when time budget exceeded", () => { + const error = apiError() + const startTime = Date.now() - 700_000 // 11+ minutes elapsed + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + }) + expect(delay).toBeUndefined() + }) + + test("respects custom maxDuration", () => { + const error = apiError() + const startTime = Date.now() - 58_000 // 58 seconds elapsed + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + maxDuration: 60_000, // 1 minute max + }) + expect(delay).toBeGreaterThanOrEqual(1900) + expect(delay).toBeLessThanOrEqual(2100) + }) + + test("caps exponential backoff to remaining time", () => { + const error = apiError() + const startTime = Date.now() - 595_000 // 595 seconds elapsed, 5 seconds remaining + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 5, // would normally be 32 seconds + startTime, + }) + expect(delay).toBeGreaterThanOrEqual(4900) + expect(delay).toBeLessThanOrEqual(5100) + }) + + test("respects server retry-after within budget", () => { + const error = apiError({ "retry-after": "30" }) + const startTime = Date.now() - 550_000 // 550 seconds elapsed, 50 seconds remaining + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + }) + expect(delay).toBe(30000) + }) + + test("caps server retry-after to remaining time", () => { + const error = apiError({ "retry-after": "30" }) + const startTime = Date.now() - 590_000 // 590 seconds elapsed, 10 seconds remaining + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 1, + startTime, + }) + expect(delay).toBeGreaterThanOrEqual(9900) + expect(delay).toBeLessThanOrEqual(10100) + }) + + test("returns undefined when getRetryDelayInMs returns undefined", () => { + const error = apiError() + const startTime = Date.now() + const delay = SessionRetry.getBoundedDelay({ + error, + attempt: 10, // exceeds RETRY_MAX_DELAY + startTime, + }) + expect(delay).toBeUndefined() + }) })