Skip to content

feat: automatic retry with backoff#108

Merged
lorenzopantano merged 7 commits intomainfrom
feat/auto-retry
Apr 15, 2026
Merged

feat: automatic retry with backoff#108
lorenzopantano merged 7 commits intomainfrom
feat/auto-retry

Conversation

@lorenzopantano
Copy link
Copy Markdown
Contributor

Summary

Complements the existing rate limiter with automatic retry logic for transient failures. When enabled, the client retries failed requests up to a configurable number of times using full-jitter exponential back-off before propagating the error.

Opt-in — disabled by default, no behaviour change for existing consumers.

What's new

retry option on TMDB / ApiClient

// Enable with defaults (3 retries, 500 ms base, 30 s cap)
const tmdb = new TMDB(token, { retry: true });

// Custom settings
const tmdb = new TMDB(token, {
  retry: {
    max_retries: 5,
    base_delay_ms: 200,
    max_delay_ms: 10_000,
    shouldRetry: (error, attempt) => {
      if (error instanceof TMDBError) return error.http_status_code >= 500;
      return attempt <= 2;
    },
  },
});

Default retry behaviour

Condition Retried?
HTTP 5xx (TMDBError with http_status_code >= 500)
Network / DNS errors (non-TMDBError)
HTTP 4xx client errors

Back-off algorithm

Full-jitter exponential: delay = random(0, min(base * 2^(attempt-1), max_delay)). Full jitter distributes retry load evenly and avoids the thundering-herd problem when multiple clients retry after the same outage.

Implementation details

  • RetryManager (utils/retry.ts) — self-contained class that owns execute<T>(fn, sleep?), delayFor(attempt), and shouldRetry. The injectable sleep argument makes it trivially testable without real timers.
  • ApiClient.execute() — the existing per-attempt closure already holds the rate-limiter acquire() call, so each retry attempt correctly acquires its own rate-limit slot with no extra logic.
  • RetryOptions is exported from the package public surface (utils/index.ts).

Files changed

File Change
retry.ts New — RetryManager + RetryOptions
options.ts retry field added to TMDBOptions
client.ts Wire RetryManager into execute()
index.ts Re-export RetryOptions
client.retry.test.ts New — 36 tests
retry.mdx New docs page
index.mdx retry option entry + section

Testing

36 new tests across two suites:

  • RetryManager unit tests — success path, 5xx retry, network error retry, 4xx no-retry, exhaustion, shouldRetry predicate (sync + async), jitter bounds, max_delay_ms clamping, constructor validation
  • ApiClient integration testsretry: true shorthand, 5xx recovery, network error recovery, 4xx no-retry, exhaustion, custom predicate, mutate() coverage, rate-limit slot per attempt

@lorenzopantano lorenzopantano self-assigned this Apr 13, 2026
@lorenzopantano lorenzopantano added the enhancement New feature or request label Apr 13, 2026
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lorenzopant-tmdb-docs Ready Ready Preview, Comment Apr 15, 2026 7:45pm

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add automatic retry with exponential back-off for transient errors

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Adds automatic retry with exponential back-off for transient failures
• Retries HTTP 5xx errors and network failures by default, skips 4xx errors
• Uses full-jitter algorithm to prevent thundering-herd problem
• Opt-in feature with sensible defaults (3 retries, 500ms base, 30s cap)
• Includes comprehensive test coverage and documentation
Diagram
flowchart LR
  A["Request fails"] --> B["RetryManager evaluates error"]
  B --> C{"shouldRetry predicate?"}
  C -->|5xx or network error| D["Compute exponential delay"]
  C -->|4xx client error| E["Throw immediately"]
  D --> F["Wait with jitter"]
  F --> G["Retry attempt"]
  G --> H{"Success?"}
  H -->|Yes| I["Return result"]
  H -->|No| B
  B -->|Max retries exhausted| E
Loading

Grey Divider

File Changes

1. packages/tmdb/src/utils/retry.ts ✨ Enhancement +148/-0

New RetryManager with exponential back-off logic

• New RetryManager class implementing retry logic with full-jitter exponential back-off
• Exports RetryOptions type for configuration (max_retries, base_delay_ms, max_delay_ms,
 shouldRetry)
• Default behavior retries HTTP 5xx and network errors, skips 4xx errors
• execute() method wraps async functions with retry loop and configurable sleep
• delayFor() computes random delay within exponential bounds clamped to max_delay_ms

packages/tmdb/src/utils/retry.ts


2. packages/tmdb/src/client.ts ✨ Enhancement +55/-40

Integrate RetryManager into ApiClient request execution

• Adds retryManager private field to ApiClient class
• Wires retry option into constructor, instantiating RetryManager when enabled
• Refactors execute() method to wrap fetch logic in attemptFetch closure
• Each retry attempt acquires its own rate-limit slot before fetch
• Delegates to RetryManager.execute() when retry is enabled, otherwise calls fetch directly

packages/tmdb/src/client.ts


3. packages/tmdb/src/types/config/options.ts ✨ Enhancement +35/-0

Add retry option to TMDBOptions configuration type

• Adds retry field to TMDBOptions type accepting boolean | RetryOptions
• Includes comprehensive JSDoc with examples and default behavior documentation
• Documents retry conditions (5xx, network errors) and non-retry conditions (4xx)
• Shows usage examples for both shorthand (true) and custom configuration

packages/tmdb/src/types/config/options.ts


View more (4)
4. packages/tmdb/src/utils/index.ts ✨ Enhancement +1/-0

Export RetryOptions from utils public surface

• Re-exports RetryOptions type from ./retry module
• Makes retry configuration type available to public API consumers

packages/tmdb/src/utils/index.ts


5. packages/tmdb/src/tests/client/client.retry.test.ts 🧪 Tests +336/-0

Comprehensive test suite for retry functionality

• New test file with 36 comprehensive tests across two suites
• RetryManager unit tests cover success path, 5xx/network error retry, 4xx no-retry, exhaustion,
 shouldRetry predicate (sync/async), jitter bounds, max_delay clamping, constructor validation
• ApiClient integration tests verify retry: true shorthand, 5xx recovery, network error recovery,
 4xx no-retry, exhaustion, custom predicates, mutate() coverage, rate-limit slot per attempt
• Uses mocked fetch and fake timers to test without real network/delays

packages/tmdb/src/tests/client/client.retry.test.ts


6. apps/docs/content/docs/getting-started/options/index.mdx 📝 Documentation +28/-0

Add retry option to options overview table

• Adds retry row to options table with type and description
• Links to dedicated retry guide documentation

apps/docs/content/docs/getting-started/options/index.mdx


7. apps/docs/content/docs/getting-started/options/retry.mdx 📝 Documentation +146/-0

New retry feature documentation and guide

• New comprehensive guide documenting retry feature
• Covers enabling retry with defaults and custom options
• Explains which errors are retried (5xx, network) vs not retried (4xx)
• Documents custom shouldRetry predicate with examples
• Explains full-jitter exponential back-off algorithm with delay table
• Shows integration with rate limiting and mutation support

apps/docs/content/docs/getting-started/options/retry.mdx


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 13, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. onError runs on success🐞
Description
When retry is enabled, transient 5xx responses call handleError(), which invokes the onError
response interceptor and throws a TMDBError that RetryManager may later recover from. This makes
onErrorInterceptor run even when the overall ApiClient.request()/mutate() call eventually succeeds,
contradicting the interceptor contract/tests and causing unintended side effects (e.g.
Sentry/toasts) on recovered attempts.
Code

packages/tmdb/src/client.ts[R328-384]

+		// Each attempt acquires its own rate-limit slot and makes a fresh fetch.
+		// This closure is invoked once per attempt by the optional RetryManager.
+		const attemptFetch = async (): Promise<T> => {
+			// Acquire a rate-limit slot immediately before the fetch so interceptor
+			// errors or serialisation failures never consume budget unnecessarily.
+			if (this.rateLimiter) await this.rateLimiter.acquire();
+
+			let res: Response;
+			try {
+				res = await fetch(url.toString(), {
+					method,
+					headers: jwt
+						? {
+								Authorization: `Bearer ${this.accessToken}`,
+								"Content-Type": "application/json;charset=utf-8",
+							}
+						: {
+								"Content-Type": "application/json;charset=utf-8",
+							},
+					...(bodyJson !== undefined ? { body: bodyJson } : {}),
+				});
+			} catch (error) {
+				this.logger?.log({
+					type: "error",
+					method,
+					endpoint: effectiveEndpoint,
+					errorMessage: error instanceof Error ? error.message : String(error),
+					durationMs: Date.now() - startedAt,
+				});
+				throw error;
+			}
+
+			if (!res.ok) await this.handleError(res, effectiveEndpoint, method);
-		let res: Response;
-		try {
-			res = await fetch(url.toString(), {
-				method,
-				headers: jwt
-					? {
-							Authorization: `Bearer ${this.accessToken}`,
-							"Content-Type": "application/json;charset=utf-8",
-						}
-					: {
-							"Content-Type": "application/json;charset=utf-8",
-						},
-				...(bodyJson !== undefined ? { body: bodyJson } : {}),
-			});
-		} catch (error) {
this.logger?.log({
-				type: "error",
+				type: "response",
  method,
  endpoint: effectiveEndpoint,
-				errorMessage: error instanceof Error ? error.message : String(error),
+				status: res.status,
+				statusText: res.statusText,
  durationMs: Date.now() - startedAt,
});
-			throw error;
-		}
-		if (!res.ok) await this.handleError(res, effectiveEndpoint, method);
+			const data = await res.json();
+			const sanitized = this.sanitizeNulls<T>(data);
+			const transformed = this.imageApi ? this.imageApi.autocompleteImagePaths(sanitized) : sanitized;
-		this.logger?.log({
-			type: "response",
-			method,
-			endpoint: effectiveEndpoint,
-			status: res.status,
-			statusText: res.statusText,
-			durationMs: Date.now() - startedAt,
-		});
-
-		const data = await res.json();
-		const sanitized = this.sanitizeNulls<T>(data);
-		const transformed = this.imageApi ? this.imageApi.autocompleteImagePaths(sanitized) : sanitized;
+			if (this.onSuccessInterceptor) {
+				const result = await this.onSuccessInterceptor(transformed);
+				return result !== undefined ? (result as T) : transformed;
+			}
-		if (this.onSuccessInterceptor) {
-			const result = await this.onSuccessInterceptor(transformed);
-			return result !== undefined ? (result as T) : transformed;
-		}
+			return transformed;
+		};
-		return transformed;
+		if (this.retryManager) return this.retryManager.execute(attemptFetch);
+		return attemptFetch();
Evidence
execute() wraps attemptFetch with RetryManager.execute(), but attemptFetch calls handleError() on
non-OK responses before the retry decision; handleError() always runs onErrorInterceptor and throws.
The interceptor contract and existing tests indicate onError is for errors that are re-thrown to the
caller, and that it must not run on successful responses—retries break that behavior by triggering
onError on intermediate failures that are later recovered.

packages/tmdb/src/client.ts[328-385]
packages/tmdb/src/client.ts[220-251]
packages/tmdb/src/types/config/options.ts[70-90]
packages/tmdb/src/tests/client/client.response-interceptors.test.ts[211-219]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
With `retry` enabled, `ApiClient.execute()` retries after `handleError()` has already called the `onError` interceptor. This causes `onError` to fire on transient errors that are later recovered, even though the overall request resolves successfully.
### Issue Context
`ResponseErrorInterceptor` is documented as a side-effect hook for errors that are re-thrown, and the test suite asserts it is not called on successful responses. Retrying after `handleError()` violates this contract.
### Fix Focus Areas
- packages/tmdb/src/client.ts[220-251]
- packages/tmdb/src/client.ts[328-385]
### Suggested approach
- Refactor error handling so retries do **not** invoke `onErrorInterceptor` on intermediate failures.
- Option A: Split `handleError()` into (1) a pure error-normalization function (parse response -> `TMDBError`) and (2) a notifier that invokes `onErrorInterceptor`.
- Wrap the retry execution with a `try/catch` and only call `onErrorInterceptor` once, right before rethrowing the final `TMDBError` to the caller.
- Ensure the existing `client.response-interceptors` expectations still hold when retry is enabled (add/adjust a test: 5xx then success ⇒ `onError` not called).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Retries non-network exceptions🐞
Description
RetryManager’s default predicate retries every non-TMDBError exception. Because ApiClient wraps
response parsing and the onSuccess interceptor inside the retried closure, JSON parse errors or
onSuccess interceptor exceptions will trigger retries and can re-send already-successful requests
(including mutations).
Code

packages/tmdb/src/utils/retry.ts[R65-71]

+function defaultShouldRetry(error: unknown): boolean {
+	if (error instanceof TMDBError) {
+		return error.http_status_code >= 500;
+	}
+	// Network / DNS / connection reset errors — always retry
+	return true;
+}
Evidence
defaultShouldRetry() returns true for any error that is not a TMDBError. In ApiClient.execute(), the
retried closure includes res.json() and the onSuccessInterceptor; any thrown error from those
steps is not a TMDBError, so it is treated as retriable by default, causing additional fetch
attempts (and potentially duplicate POST/PUT/DELETE side effects).

packages/tmdb/src/utils/retry.ts[65-71]
packages/tmdb/src/client.ts[328-381]
packages/tmdb/src/client.ts[383-385]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The default retry predicate retries *all* non-`TMDBError` exceptions. Since `ApiClient` retries a closure that includes JSON parsing and `onSuccessInterceptor` execution, non-network failures (e.g., `SyntaxError` from `res.json()` or a bug in `onSuccess`) will be retried, which can resend successful requests and duplicate mutations.
### Issue Context
PR docs describe default retries as “network/DNS errors” for non-`TMDBError`, but the implementation equates “non-`TMDBError`” with “network,” which is not true given the current retry boundary.
### Fix Focus Areas
- packages/tmdb/src/utils/retry.ts[65-71]
- packages/tmdb/src/client.ts[328-381]
- packages/tmdb/src/client.ts[383-385]
### Suggested approach
Choose one (or both):
1) **Tighten `defaultShouldRetry`** to only retry known network-level failures (e.g., `TypeError` from `fetch` in many runtimes) and/or explicitly exclude common non-network exceptions (e.g., `SyntaxError`).
2) **Reduce the retry boundary** in `ApiClient.execute()` so the retried function only covers `fetch` + HTTP-status evaluation, and do JSON parsing + `onSuccessInterceptor` *after* a successful attempt is returned (so parse/interceptor errors don’t trigger a re-fetch).
Add a regression test: successful HTTP 2xx followed by `onSuccess` throwing should not trigger a second fetch attempt.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Retry JSDoc incorrect🐞
Description
RetryOptions JSDoc describes a clamp + fixed-range jitter calculation, but RetryManager.delayFor()
implements full-jitter exponential backoff. This mismatch can mislead users relying on TypeScript
IntelliSense for timing behavior.
Code

packages/tmdb/src/utils/retry.ts[R13-18]

+	 * The delay for attempt `n` (1-indexed) is:
+	 * ```
+	 * clamp(base_delay_ms * 2^(n-1), 0, max_delay_ms) + jitter
+	 * ```
+	 * where `jitter` is a uniform random value in `[0, base_delay_ms)`.
+	 *
Evidence
The JSDoc says clamp(...) + jitter with jitter in [0, base_delay_ms), but the implementation
samples uniformly from [0, exponentialCap) where `exponentialCap = min(base * 2^(attempt-1),
max)`, which is full-jitter behavior.

packages/tmdb/src/utils/retry.ts[13-18]
packages/tmdb/src/utils/retry.ts[109-112]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `RetryOptions` JSDoc describes a different delay algorithm than what `RetryManager.delayFor()` actually implements.
### Issue Context
The runtime behavior is correct for full-jitter exponential backoff; only the IntelliSense/JSDoc text is misleading.
### Fix Focus Areas
- packages/tmdb/src/utils/retry.ts[13-18]
- packages/tmdb/src/utils/retry.ts[100-112]
### Suggested approach
Update the JSDoc to match the implemented formula, e.g.:
`delay = random(0, min(base_delay_ms * 2^(attempt-1), max_delay_ms))`
(and remove the `+ jitter` / `[0, base_delay_ms)` wording).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread packages/tmdb/src/client.ts Outdated
Comment thread packages/tmdb/src/utils/retry.ts
@lorenzopantano lorenzopantano merged commit 4546960 into main Apr 15, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant