feat!: add support for manual retries from hooks - #618
Open
cernymatej wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughRetry handling now supports typed retry intents and history, manual retries from hooks, automatic retry classification, per-attempt timeouts, abort-aware delays, and retry history exposure on errors. A comprehensive test suite covers retry lifecycle behavior and ChangesRetry lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant doFetch
participant RetryHooks
participant Fetch
participant ErrorHandler
Caller->>doFetch: start request
doFetch->>Fetch: execute attempt with timeout signal
Fetch-->>doFetch: response or request error
doFetch->>RetryHooks: run retry-capable hooks
RetryHooks-->>doFetch: retry intent or no intent
doFetch->>ErrorHandler: classify failure and inspect history
ErrorHandler-->>doFetch: perform retry or return error
doFetch->>Fetch: execute next attempt
doFetch-->>Caller: response or FetchError with retries
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
some use case examples: exponential backoff await $fetch("/flaky", {
retry: 5,
// or just `ctx.retries.count()` if no other retries are present
retryDelay: (ctx) => 100 * 2 ** ctx.retries.count('auto'),
});simple polling await $fetch(`/jobs/${id}`, {
onResponse(ctx) {
if (ctx.response._data?.pending && ctx.retries.count("poll") < 10) {
return ctx.retry({ cause: "poll", delay: 1000 });
}
},
});auth session refresh await $fetch('/protected', {
async onResponseError(ctx) {
if (ctx.response.status === 401 && ctx.retries.count("auth") === 0) {
await refreshSession()
return ctx.retry({
cause: "auth",
options: { headers: { /* ... */ } },
})
}
},
}) |
cernymatej
force-pushed
the
feat/retry-support
branch
from
July 29, 2026 20:10
ccafc8d to
95d8d58
Compare
cernymatej
marked this pull request as ready for review
July 29, 2026 20:10
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
test/retry.test.ts (3)
175-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the
console.errorspy unconditionally.If any assertion between lines 193–197 fails,
mockRestore()at line 198 never runs and the mockedconsole.errorleaks into the remaining tests in this file, hiding real errors. Prefer a globalafterEach(() => vi.restoreAllMocks())in this suite.♻️ Suggested suite-level cleanup
beforeEach(() => { authCalls = []; }); + + afterEach(() => { + vi.restoreAllMocks(); + });Then drop line 198.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/retry.test.ts` around lines 175 - 198, Ensure the console.error spy in the retry test suite is restored unconditionally by adding suite-level afterEach cleanup with vi.restoreAllMocks(). Remove the local consoleError.mockRestore() call and preserve the existing assertions.
422-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTight timing margin makes this test flake-prone.
Three sequential 60 ms server delays under a 150 ms per-attempt timeout leaves only ~90 ms of slack per attempt for request overhead; a loaded CI runner can push a single attempt past 150 ms. Widening both values preserves the intent (cumulative 600 ms > 500 ms timeout still proves per-attempt semantics) with far more headroom.
♻️ Suggested widening
const url = setSequence( "timeout-per-attempt", [500, 500, 200], - [60, 60, 60] + [200, 200, 200] ); const res = await $fetch<{ calls: number }>(url, { retry: 2, - timeout: 150, + timeout: 500, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/retry.test.ts` around lines 422 - 433, Widen the timing values in the “applies the timeout to each attempt, not to the whole retry chain” test to reduce CI flakiness while preserving the per-attempt semantics. Increase both the timeout and each setSequence delay so the cumulative delay remains greater than the timeout, and keep the expected three calls unchanged.
353-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the replay never fired.
Both tests verify the rejection and elapsed time, but not that the retry attempt was skipped. Adding a call-count check makes the abort-during-delay contract explicit.
💚 Suggested assertions
await expect(promise).rejects.toThrow(/abort/i); expect(Date.now() - start).toBeLessThan(1000); + expect(sequences.get("abort-auto-delay")!.calls).toBe(1);and likewise
sequences.get("abort-manual-delay")!.callsin the manual-delay test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/retry.test.ts` around lines 353 - 385, Add assertions to both abort-delay tests, using the sequence records from setSequence, to verify only the initial request occurred: check sequences.get("abort-auto-delay")!.calls and sequences.get("abort-manual-delay")!.calls after rejection. Preserve the existing abort and elapsed-time assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fetch.ts`:
- Around line 103-108: Update the retry delay calculation surrounding retryDelay
and sleep to implement the configured retryBackoff strategies: full jitter,
equal jitter, and decorrelated jitter, using the configured base and maximum
delays. Apply the documented precedence and coexistence rules between
retryBackoff and retryDelay, while preserving abort-signal handling and the
existing sleep flow.
In `@src/types.ts`:
- Around line 117-138: Mark the cause property readonly in both RetryEntry and
RetryIntent so recorded history and pending retry intents cannot be relabeled.
Leave the existing RetryHistory behavior and RetryCause type constraints
unchanged.
- Around line 94-104: Update the module augmentation example in the
documentation comment above the relevant types declaration so
FetchTypes.retryCause appears only once with the intended flexible
string-compatible type. Remove the conflicting duplicate property example while
preserving the explanation that strict and unrestricted string variants are
alternatives.
In `@src/utils.ts`:
- Around line 194-212: Centralize the stream-body replayability check in the
createRetryIntent flow, validating both context.options.body and any body
embedded in context.request while preserving the existing TypeError. In
src/utils.ts lines 194-212, expose or reuse a shared validation helper; in
src/fetch.ts lines 97-115, invoke it before recording or executing each
automatic retry so consumed Request streams are rejected consistently.
---
Nitpick comments:
In `@test/retry.test.ts`:
- Around line 175-198: Ensure the console.error spy in the retry test suite is
restored unconditionally by adding suite-level afterEach cleanup with
vi.restoreAllMocks(). Remove the local consoleError.mockRestore() call and
preserve the existing assertions.
- Around line 422-433: Widen the timing values in the “applies the timeout to
each attempt, not to the whole retry chain” test to reduce CI flakiness while
preserving the per-attempt semantics. Increase both the timeout and each
setSequence delay so the cumulative delay remains greater than the timeout, and
keep the expected three calls unchanged.
- Around line 353-385: Add assertions to both abort-delay tests, using the
sequence records from setSequence, to verify only the initial request occurred:
check sequences.get("abort-auto-delay")!.calls and
sequences.get("abort-manual-delay")!.calls after rejection. Preserve the
existing abort and elapsed-time assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e0f05072-7625-49a3-ad99-bea026bd9361
📒 Files selected for processing (5)
src/error.tssrc/fetch.tssrc/types.tssrc/utils.tstest/retry.test.ts
Comment on lines
+117
to
+138
| export interface RetryEntry { | ||
| cause: RetryCause; | ||
| trigger: RetryTrigger; | ||
| status?: number; | ||
| } | ||
|
|
||
| export interface RetryHistory { | ||
| readonly history: readonly RetryEntry[]; | ||
| readonly last: RetryEntry | undefined; | ||
| count(cause?: RetryCause): number; | ||
| } | ||
|
|
||
| export interface RetryIntent<T = any, R extends ResponseType = ResponseType> { | ||
| /** | ||
| * `'auto'` is reserved for automatic retries | ||
| * @default 'manual' | ||
| */ | ||
| cause?: Exclude<RetryCause, "auto">; | ||
| request?: FetchRequest; | ||
| options?: FetchOptions<R, T>; | ||
| delay?: number; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Mark retry causes readonly to preserve cause-specific history.
Both recorded entries and pending intents expose an assignable cause. Hooks can therefore relabel pending or historical retries, changing count(cause) retroactively despite the immutable-cause contract.
Proposed fix
export interface RetryEntry {
- cause: RetryCause;
+ readonly cause: RetryCause;
trigger: RetryTrigger;
status?: number;
}
export interface RetryIntent<T = any, R extends ResponseType = ResponseType> {
- cause?: Exclude<RetryCause, "auto">;
+ readonly cause?: Exclude<RetryCause, "auto">;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export interface RetryEntry { | |
| cause: RetryCause; | |
| trigger: RetryTrigger; | |
| status?: number; | |
| } | |
| export interface RetryHistory { | |
| readonly history: readonly RetryEntry[]; | |
| readonly last: RetryEntry | undefined; | |
| count(cause?: RetryCause): number; | |
| } | |
| export interface RetryIntent<T = any, R extends ResponseType = ResponseType> { | |
| /** | |
| * `'auto'` is reserved for automatic retries | |
| * @default 'manual' | |
| */ | |
| cause?: Exclude<RetryCause, "auto">; | |
| request?: FetchRequest; | |
| options?: FetchOptions<R, T>; | |
| delay?: number; | |
| } | |
| export interface RetryEntry { | |
| readonly cause: RetryCause; | |
| trigger: RetryTrigger; | |
| status?: number; | |
| } | |
| export interface RetryHistory { | |
| readonly history: readonly RetryEntry[]; | |
| readonly last: RetryEntry | undefined; | |
| count(cause?: RetryCause): number; | |
| } | |
| export interface RetryIntent<T = any, R extends ResponseType = ResponseType> { | |
| /** | |
| * `'auto'` is reserved for automatic retries | |
| * `@default` 'manual' | |
| */ | |
| readonly cause?: Exclude<RetryCause, "auto">; | |
| request?: FetchRequest; | |
| options?: FetchOptions<R, T>; | |
| delay?: number; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/types.ts` around lines 117 - 138, Mark the cause property readonly in
both RetryEntry and RetryIntent so recorded history and pending retry intents
cannot be relabeled. Leave the existing RetryHistory behavior and RetryCause
type constraints unchanged.
Comment on lines
+194
to
+212
| export function createRetryIntent< | ||
| T = any, | ||
| R extends ResponseType = ResponseType, | ||
| >( | ||
| context: FetchContext<T, R>, | ||
| input: RetryIntent<T, R> = {} | ||
| ): RetryIntent<T, R> { | ||
| const body = context.options.body; | ||
| if ( | ||
| body && | ||
| (typeof (body as ReadableStream).pipeTo === "function" || | ||
| typeof (body as any).pipe === "function") | ||
| ) { | ||
| throw new TypeError( | ||
| "[ofetch] Cannot retry: request body is a stream and cannot be replayed." | ||
| ); | ||
| } | ||
| return { ...input, [kRetryIntent]: true } as RetryIntent<T, R>; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Centralize replayability validation across every retry path.
The current guard only covers manual retries whose body is present in context.options, leaving Request bodies and automatic retries able to replay consumed streams.
src/utils.ts#L194-L212: extract a shared check covering bothcontext.options.bodyand bodies embedded incontext.request.src/fetch.ts#L97-L115: invoke that check before recording or executing an automatic retry.
📍 Affects 2 files
src/utils.ts#L194-L212(this comment)src/fetch.ts#L97-L115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils.ts` around lines 194 - 212, Centralize the stream-body
replayability check in the createRetryIntent flow, validating both
context.options.body and any body embedded in context.request while preserving
the existing TypeError. In src/utils.ts lines 194-212, expose or reuse a shared
validation helper; in src/fetch.ts lines 97-115, invoke it before recording or
executing each automatic retry so consumed Request streams are rejected
consistently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves #581
resolves #503 (PR #513)
resolves #358 (PR #359)
resolves #355
resolves #536 (PR #537)
Important
This PR is meant for the v2
This PR aims to resolve the above-mentioned issues by introducing a way to retry the current request based on custom logic in the interceptors. The PRs that are open are IMO not flexible enough to solve all the issues.
Interceptors can now start a retry by returning
ctx.retry():The replayed request goes through the whole pipeline again, so all interceptors re-run.
Every retry is recorded in
ctx.retries:Merging / Cancelling
causeis immutable once setdelaycoerces to the longestrequestthan an alreadyctx.pendingRetryis ignored with an error logctx.retry()throws for stream bodiesAutomatic retries
The declarative options (
retry,retryDelay,retryStatusCodes) are unchanged, but the "budget" (retry,retryDelay) is reset after every manual retry. (500 -> auto retry -> 401 -> refresh -> 500 -> auto retry -> 200works withretry: 1)options.retryis no longer decremented between attemptsonResponsemay now return aRetryIntent(return type widened fromvoid)Summary by CodeRabbit
New Features
Bug Fixes