fix(fetch): retry network errors regardless of retryStatusCodes

[amitmishra11]

Bug

Closes #495.

When a request fails at the network level (no HTTP response is produced at all — e.g. a connection refused, DNS failure, or fetch abort via timeout), onError fell back to status code 500 as a sentinel:

// src/fetch.ts (before)
const responseCode = (context.response && context.response.status) || 500;

Because the default retryStatusCodes set includes 500, this worked fine out of the box. But callers who provided a custom retryStatusCodes array that deliberately excluded 500 (a reasonable choice: re-sending a POST on a 500 can cause duplicate side-effects on the server) would unknowingly also stop retrying pure connection errors that never returned any response at all. The two failure modes are conflated by the || 500 fallback.

Fix

When context.response is absent, skip the status-code gate entirely and allow the retry based solely on the remaining retry count. The retryStatusCodes option is only meaningful when the server actually returned an HTTP response.

// src/fetch.ts (after)
const hasResponse = !!(context.response && context.response.status);
const responseCode = hasResponse ? context.response!.status : undefined;
if (
  retries > 0 &&
  (!hasResponse ||
    (Array.isArray(context.options.retryStatusCodes)
      ? context.options.retryStatusCodes.includes(responseCode!)
      : retryStatusCodes.has(responseCode!)))
) {

Reproduction

// Before fix: only 1 call (no retries) because 500 is absent from the list
// After fix:  3 calls (initial + 2 retries) as expected
await $fetch("https://unreachable-host.example", {
  retry: 2,
  retryStatusCodes: [502, 503, 504], // 500 intentionally excluded
});

Test

A new test in test/index.test.ts stubs globalThis.fetch to throw a TypeError (simulating a network error), then asserts that ofetch still performs the expected number of retry attempts when retryStatusCodes excludes 500.

Summary by CodeRabbit

• Bug Fixes
  • Improved retry behavior for failed requests when no response is returned, so network or connection errors now continue retrying as expected.
  • Retry settings based on HTTP status codes now apply only when a status code is actually available, preventing unexpected stops on connection failures.
• Tests
  • Added coverage for retries on network-level failures to verify the new behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified the retry condition in createFetch's onError handler so that requests without an HTTP response (e.g., network errors) are retried whenever retries remain, without matching against retryStatusCodes. Added a test verifying retries occur on network failures even when retryStatusCodes excludes 500.

Changes

Network Error Retry Fix

Layer / File(s)	Summary
Retry condition and validation src/fetch.ts, test/index.test.ts	The onError retry check now treats missing responses as retryable without a synthetic 500 fallback, applying retryStatusCodes only when a response status exists; a new test mocks a throwing fetch to confirm retries still occur when retryStatusCodes excludes 500.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant ofetch as "$fetch"
  participant onError
  participant fetchImpl as "globalThis.fetch"

  Caller->>ofetch: call with retry: 2, retryStatusCodes: [502,503,504]
  ofetch->>fetchImpl: attempt request
  fetchImpl-->>ofetch: throws error (no response)
  ofetch->>onError: handle error
  onError->>onError: retries > 0 and no context.response -> retry
  onError->>fetchImpl: retry attempt
  fetchImpl-->>ofetch: throws error (no response)
  onError->>fetchImpl: retry attempt
  fetchImpl-->>ofetch: throws error (no response)
  ofetch-->>Caller: fails after 3 total attempts

Loading

Related Issues: #495

Suggested labels: bug, retry

Suggested reviewers: pi0

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: retrying network errors regardless of retryStatusCodes.
Linked Issues check	✅ Passed	The code and test directly address issue #495 by retrying network/client failures even when 500 is excluded.
Out of Scope Changes check	✅ Passed	The changes are narrowly scoped to fetch retry behavior and its test, with no unrelated edits visible.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

test/index.test.ts (1)

312-338: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test correctly validates the fix.

Covers the exact scenario from the PR: network error with retryStatusCodes excluding 500 still retries, and cleanup is handled via finally.

Optionally, vi.spyOn(globalThis, "fetch") with mockImplementation would be more idiomatic for vitest and integrates with vi.restoreAllMocks(), but the current manual save/restore pattern is functionally safe.

🤖 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/index.test.ts` around lines 312 - 338, The test in the $fetch retry
coverage works, but it manually replaces globalThis.fetch instead of using
Vitest’s spying utilities. Update the network error test to use
vi.spyOn(globalThis, "fetch") with a mock implementation, and rely on
vi.restoreAllMocks() for cleanup so it matches the rest of the suite while
preserving the same retry 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.

Nitpick comments:
In `@test/index.test.ts`:
- Around line 312-338: The test in the $fetch retry coverage works, but it
manually replaces globalThis.fetch instead of using Vitest’s spying utilities.
Update the network error test to use vi.spyOn(globalThis, "fetch") with a mock
implementation, and rely on vi.restoreAllMocks() for cleanup so it matches the
rest of the suite while preserving the same retry assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 19e0a1d9-5144-4f44-9aa3-a2bb246afd29

📥 Commits

Reviewing files that changed from the base of the PR and between 9102908 and 6a1a5d0.

📒 Files selected for processing (2)

• src/fetch.ts
• test/index.test.ts