set cap for max time to wait between retries (#4135)

Co-authored-by: GitHub Action <[email protected]>

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)

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)

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<void> {
     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
-
-    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) return base
-
-    const seconds = Number.parseFloat(retryAfter)
-    if (!Number.isNaN(seconds)) {
-      const normalized = normalizeDelay({ base, candidate: seconds * 1000 })
-      if (normalized != null) return normalized
-      return base
-    }
-
-    const dateMs = Date.parse(retryAfter) - Date.now()
-    const normalized = normalizeDelay({ base, candidate: dateMs })
-    if (normalized != null) return normalized
-
-    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 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)
+          }
+        }
+      }
+
+      return RETRY_INITIAL_DELAY * Math.pow(RETRY_BACKOFF_FACTOR, attempt - 1)
+    })
+
+    // dont retry if wait is too far from now
+    if (delay > RETRY_MAX_DELAY) return undefined
+
+    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)
   }
 }

packages/opencode/test/session/retry.test.ts

@@ -13,8 +13,8 @@ function apiError(headers?: Record<string, string>): 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()
+  })
 })