Skip to content

Commit 1b4c9b8

Browse files
committed
fix(retry): preserve uncapped Retry-After comparison in tools/index.ts
parseRetryAfter() caps its return value at 30s by default. tools/index.ts compares the parsed Retry-After against a caller-configured maxDelayMs to decide whether to skip a retry entirely -- capping before that comparison silently defeats the skip check whenever maxDelayMs is configured above 30s, since a Retry-After between 30s and maxDelayMs would incorrectly look "within limits" and get retried instead of skipped (caught by Cursor Bugbot). Added an optional maxMs param (default unchanged) so tools/index.ts can request the raw, uncapped value for its own comparison while backoffWithJitter still clamps the actual sleep duration to maxDelayMs. Added a regression test covering maxDelayMs > 30s.
1 parent c440d1a commit 1b4c9b8

3 files changed

Lines changed: 39 additions & 8 deletions

File tree

apps/sim/tools/index.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,6 +1896,28 @@ describe('MCP Tool Execution', () => {
18961896
expect(result.success).toBe(false)
18971897
})
18981898

1899+
it('skips retry when Retry-After exceeds a maxDelayMs configured above the 30s default cap', async () => {
1900+
global.fetch = Object.assign(
1901+
vi
1902+
.fn()
1903+
.mockResolvedValueOnce(
1904+
makeJsonResponse(429, { error: 'rate limited' }, { 'retry-after': '50' })
1905+
)
1906+
.mockResolvedValueOnce(makeJsonResponse(200, { ok: true })),
1907+
{ preconnect: vi.fn() }
1908+
) as typeof fetch
1909+
1910+
const result = await executeTool('http_request', {
1911+
url: '/api/test',
1912+
method: 'GET',
1913+
retries: 3,
1914+
retryMaxDelayMs: 40000,
1915+
})
1916+
1917+
expect(global.fetch).toHaveBeenCalledTimes(1)
1918+
expect(result.success).toBe(false)
1919+
})
1920+
18991921
it('retries when Retry-After header is within maxDelayMs', async () => {
19001922
global.fetch = Object.assign(
19011923
vi

apps/sim/tools/index.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,7 +1495,10 @@ function shouldRetryWithoutReadingBody(
14951495
if (!retryConfig || isLastAttempt || !isRetryableFailure(null, status)) {
14961496
return false
14971497
}
1498-
return (parseRetryAfter(headers.get('retry-after')) ?? 0) <= retryConfig.maxDelayMs
1498+
return (
1499+
(parseRetryAfter(headers.get('retry-after'), Number.POSITIVE_INFINITY) ?? 0) <=
1500+
retryConfig.maxDelayMs
1501+
)
14991502
}
15001503

15011504
/**
@@ -1741,7 +1744,10 @@ async function executeToolRequest(
17411744
!response.ok &&
17421745
isRetryableFailure(null, response.status)
17431746
) {
1744-
const retryAfterMs = parseRetryAfter(response.headers.get('retry-after'))
1747+
const retryAfterMs = parseRetryAfter(
1748+
response.headers.get('retry-after'),
1749+
Number.POSITIVE_INFINITY
1750+
)
17451751
if (retryAfterMs !== null && retryAfterMs > retryConfig.maxDelayMs) {
17461752
logger.warn(
17471753
`[${requestId}] Retry-After (${retryAfterMs}ms) exceeds maxDelayMs (${retryConfig.maxDelayMs}ms), skipping retry`

packages/utils/src/retry.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,28 +32,31 @@ export function backoffWithJitter(
3232
return exponential * (0.8 + jitter * 0.4)
3333
}
3434

35-
/** Maximum `Retry-After` value honored: 30 s. Prevents a misconfigured upstream from stalling callers. */
35+
/** Default maximum `Retry-After` value honored: 30 s. Prevents a misconfigured upstream from stalling callers. */
3636
const RETRY_AFTER_MAX_MS = 30_000
3737

3838
/**
3939
* Parses an HTTP `Retry-After` header (either delta-seconds or an HTTP-date)
40-
* into a millisecond delay, capped at 30 s.
40+
* into a millisecond delay, capped at `maxMs` (default 30 s).
4141
* Returns `null` when the header is absent or unparseable so callers can fall
42-
* back to their own backoff.
42+
* back to their own backoff. Pass the caller's own `maxDelayMs` as `maxMs` when
43+
* that value needs to be compared against the parsed delay (e.g. to decide
44+
* whether to skip a retry) — otherwise the default cap silently truncates the
45+
* comparison.
4346
*/
44-
export function parseRetryAfter(header: string | null): number | null {
47+
export function parseRetryAfter(header: string | null, maxMs = RETRY_AFTER_MAX_MS): number | null {
4548
if (!header) return null
4649
const trimmed = header.trim()
4750
if (trimmed.length === 0) return null
4851
const seconds = Number(trimmed)
4952
if (Number.isFinite(seconds) && seconds >= 0) {
50-
return Math.min(Math.floor(seconds * 1000), RETRY_AFTER_MAX_MS)
53+
return Math.min(Math.floor(seconds * 1000), maxMs)
5154
}
5255
const dateMs = Date.parse(trimmed)
5356
if (!Number.isNaN(dateMs)) {
5457
const delta = dateMs - Date.now()
5558
if (delta <= 0) return 0
56-
return Math.min(delta, RETRY_AFTER_MAX_MS)
59+
return Math.min(delta, maxMs)
5760
}
5861
return null
5962
}

0 commit comments

Comments
 (0)