fix(net): make fetch behave like native fetch#1652
Conversation
@cacheable/net previously threw `Error("Fetch failed with status …")` on any
non-2xx response, which diverged from native `fetch` semantics. Callers written
for `fetch` check `response.ok`, but that became unreachable dead code because the
request already rejected, producing cryptic errors on non-2xx (e.g. routing a
subscription POST through the client).
Align with native `fetch`:
- Resolve with the Response for any completed exchange (4xx/5xx included); only
reject on real network errors. Removes all four throw sites.
- Only cache storable responses: simple mode caches 2xx only; HTTP-cache mode
continues to defer to RFC 7234 `storable()`. Errors are returned, never cached.
- Make `options` optional so `fetch(url)` works, matching `fetch(input, init?)`
and the documented signature.
- Preserve `response.url` (plus `redirected`/`type` from live responses) on
reconstructed/cached responses via a shared `makeResponse` helper, so the final
URL survives caching and the get/post/etc. helpers.
Add deterministic local-server tests covering non-2xx across every path (no
cache, simple cache, HTTP cache, POST/HEAD, helpers, CacheableNet), error
responses not being cached, optionless `fetch(url)`, and url/redirected
preservation. Document the native-fetch error semantics in the README.
https://claude.ai/code/session_01QeG7AkCcuwmpw9JJSX26zi
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aligns @cacheable/net with native fetch semantics by resolving completed HTTP exchanges (including 4xx and 5xx statuses) instead of throwing errors. It introduces a makeResponse helper to reconstruct responses while preserving native properties like url, redirected, and type. Additionally, the documentation has been updated to reflect these error handling changes, and comprehensive tests have been added to verify native fetch parity. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1652 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 26 26
Lines 2705 2725 +20
Branches 598 607 +9
=========================================
+ Hits 2705 2725 +20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9083af5614
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-redirect metadata
Resolves four issues raised in review of the native-fetch alignment:
- Don't cache HTTP errors under default cache semantics. Non-2xx responses now
return early before policy creation, so statuses RFC 7234 deems storable
(404/410/501/…) are no longer served from cache — matching the documented
"errors are never cached" contract.
- Fix a 304/null-body throw. Reconstructing `new Response("", { status: 304 })`
threw, so conditional GETs (and 204s) still rejected. makeResponse now coerces
the body to null for null-body statuses (101/103/204/205/304).
- Restore stampede protection in simple-cache mode. The get+fetch+set rewrite
dropped getOrSet's coalescing; concurrent misses now share a single origin
request via coalesceAsync, while each caller rebuilds its own Response (so the
body stays independently readable) and errors remain uncached.
- Persist final-URL metadata for cached responses. CachedResponse now stores
url/redirected/type so cache hits report the same final URL (after redirects)
as the original miss, instead of falling back to the request URL.
Adds local-server tests for each (304 helper, concurrent-miss coalescing,
cached-redirect metadata, and request-url fallback for legacy entries) and
updates the README error-handling note. Adds @cacheable/utils dependency for
coalesceAsync.
https://claude.ai/code/session_01QeG7AkCcuwmpw9JJSX26zi
Update the lead feature bullet, which still described fetch as coming from undici with caching always on. Reflect the current behavior: a drop-in fetch built on the runtime's global fetch that resolves on any status (check response.ok), preserves response.url/redirected/type, with caching opt-in. https://claude.ai/code/session_01QeG7AkCcuwmpw9JJSX26zi
2 participants
Problem
@cacheable/net'sfetchdiverged from nativefetchin ways that produced "a ton of bugs" for consumers. The biggest one: it threwError("Fetch failed with status …")on any non-2xx response.Native
fetchnever rejects on HTTP status — it resolves with aResponsewhose.okisfalsefor 4xx/5xx and only rejects on genuine network errors. Code written for native-fetch semantics checksresponse.ok, but with@cacheable/netthat check became unreachable dead code because the call already threw, surfacing as cryptic errors (e.g. routing a non-cacheable subscriptionPOSTthrough the client and getting an opaque throw instead of a4xxResponse).All four throw sites were wrapped in
/* c8 ignore *///* v8 ignore */and had zero tests — the throwing behavior was never validated.Changes
Aligned
@cacheable/netwith nativefetch(packages/net/src/fetch.ts,index.ts):Responseregardless of status; reject only on network errorsstorable(). Errors are returned, never cachedfetch(url, options)requiredoptions→fetch(url)crashedoptionsoptional, matchingfetch(input, init?)and the documented signature.url(→""),.redirected,.typemakeResponsehelper that reattachesurl(andredirected/typefrom live responses), so the final URL survives caching and theget/post/… helpersTests
Added deterministic, self-contained local-server tests (no external dependency) in both
fetch.test.tsandindex.test.tscovering:POST/HEAD, theget/posthelpers, andCacheableNetmethods.fetch(url)works with no options at all.response.url(andredirected) are preserved through reconstruction/caching and the helpers.All new tests pass;
pnpm build,tsc --noEmit, and strict Biome lint are green. The pre-existing mock-backed suite (Dockerjaredwray/mockhttpon:3737) is unchanged and runs in CI; thedel()-without-options validation throw is intentionally retained.Behavior change note
Removing the throw is a deliberate behavior change: callers that wrapped
fetchintry/catchto catch HTTP errors will now receive a resolvedResponse. They should switch to checkingresponse.ok— the README's new "Error Handling" section documents this.https://claude.ai/code/session_01QeG7AkCcuwmpw9JJSX26zi
Generated by Claude Code