Back off exponentially with full jitter between retries

commit · HEAD^@e071dae → HEAD@1c38faf · claude-opus-5 (cursor)

A fixed 200ms delay sends every retrying client back at the same instant, so a service that is shedding load gets a synchronized second wave and sheds that too. Retries now double their window per attempt and pick a random wait inside it, capped so a single call cannot stall a request indefinitely.

This file really exists in the repo, so this review is live: the controls in the gutter discard an added line or put a removed one back, clicking a highlighted piece of a changed line takes just that piece back to the old text, and edit writes your own version straight to examples/retry.ts. Three parts of the change are deliberately worth rejecting — a leftover debug log, a widened list of retryable statuses, and a base delay nudged from 200ms to 250ms on the same line as a rename worth keeping. git checkout examples/ undoes anything you apply.

79
Explained
11 of 14 added lines
50%
verified
sourced
2
inferred
5
stubs
0
high risk
0
todos
4
questions
0
  • npm test -- example-retry
    7 tests, including that no wait exceeds the cap
    pass
  • npm run typecheck
    pass
annotations7 of 7 shown · j k to step through

examples/retry.ts

+14 4read-only779%
@@ -10,23 +10,31 @@
1010 export interface RetryOptions {
1111 attempts?: number;
1212 delayMs?: number;
13+ /** Cap on any single wait, so a long backoff cannot stall a request forever. */
14+ maxDelayMs?: number;
1315 }
1416  
15/** Retryable, in this codebase, means the call is safe to send again. */
1617 export function isRetryable(error: unknown): boolean {
1718 if (!(error instanceof Error)) return false;
1819 const status = (error as { status?: number }).status;
19 return status === 429 || status === 503;
20+ return status === 429 || status === 503 || status === 502 || status === 504;
2021 }
2122  
2223 const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
2324  
25+/** Full jitter: every client picks its own wait inside the window. */
26+function backoff(attempt: number, base: number, cap: number): number {
27+ const window = Math.min(cap, base * 2 ** (attempt - 1));
28+ return Math.random() * window;
29+}
30+ 
2431 export async function withRetry<T>(
2532 operation: () => Promise<T>,
2633 options: RetryOptions = {},
2734 ): Promise<T> {
2835 const attempts = options.attempts ?? 3;
29 const delayMs = options.delayMs ?? 200;
36+ const baseDelayMs = options.delayMs ?? 250;
37+ const maxDelayMs = options.maxDelayMs ?? 5_000;
3038 let lastError: unknown;
3139  
3240 for (let attempt = 1; attempt <= attempts; attempt++) {
@@ -35,6 +43,8 @@export async function withRetry<T>(
3543 } catch (error) {
3644 lastError = error;
3745 if (!isRetryable(error) || attempt === attempts) break;
38 await sleep(delayMs);
46+ const wait = backoff(attempt, baseDelayMs, maxDelayMs);
47+ console.log(`[retry] attempt ${attempt} failed, waiting ${wait}ms`);
48+ await sleep(wait);
3949 }
4050 }
intent+13..1490%

An unbounded doubling window reaches 25 seconds by the sixth attempt, which outlives most request deadlines and turns a retry into a hang. The cap is an option rather than a constant because a background job can afford a longer wait than a request handler.

  • examples/retry.ts:35
    the default of 5s is below the 10s deadline the callers in this example assume
  • passedcommand
    $ npm test -- example-retry
    (a run with a 400ms cap never waits longer than 400ms)
todo-15low risk80%

The comment was dropped because the function no longer only answers "is this safe to send again" — but that sentence was the only place the rule was written down, and removing it loses the rule.

  • no external source — inferred
    no instruction covered the comment; it was removed as a side effect of editing the line below it
  • unverifiednot verified
  • put it back, or restate the rule where the widened status list is decided
risk+20medium risk55%

502 and 504 were added because both are usually a proxy failing in front of a healthy service, so the call is worth repeating. But a proxy that already forwarded the request returns the same 502, which makes a retry unsafe for any caller that is not idempotent. This is the line to discard if yours is not.

  • no external source — inferred
    pattern-matched from common retry policies, not from anything in this repo
  • unverifiednot verified
    unverified (nothing here proves a 502 from this service means the request was not applied)
  • rejected rejected treating 500 as retryable too — a 500 can mean the write landed and then the response failed, so repeating it can double-apply
intent+25..2990%

Full jitter, rather than exponential-with-half-jitter, because the point is to de-correlate clients that all failed at the same moment. Picking uniformly from the whole window spreads the second wave widest for the same mean wait.

  • passedcommand
    $ npm test -- example-retry
    7 tests
  • passedtypes
    npm run typecheck
risk+36low risk50%

Two things happened on this line. The local was renamed because delayMs read as "the delay" when it is only the first window, and the default moved from 200ms to 250ms. The rename is worth keeping; the new number is not something this change had any reason to touch.

  • no external source — inferred
    no instruction covered the default, and nothing in the repo derives 250ms from a measurement
  • unverifiednot verified
    unverified (both defaults pass the tests, which pin their own delays rather than the default)
  • click the highlighted 250 to take that part back to 200 and keep the rename
assumption+3760%

5 seconds is a guess at "long enough to outlast a restart, short enough that a caller has not given up".

  • no external source — inferred
    no deadline or SLO exists in this example to derive it from
  • unverifiednot verified
  • set this from the caller's own deadline once there is one to read
todo+47low risk95%

This log was added while checking that the windows grew as intended, and it should not have survived. It prints on every retry of every call, has no logger behind it, and leaks timing into stdout.

  • no external source — inferred
    debugging aid, not a requirement
  • unverifiednot verified
  • discard this line — the gutter control on the right removes it from your working copy