From 9083af5614b55c5d001c2a641bd5467a2e0df51a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 16:31:32 +0000 Subject: [PATCH 1/3] fix(net): make fetch behave like native fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @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 --- packages/net/README.md | 21 +++ packages/net/src/fetch.ts | 267 +++++++++++++++++++++----------- packages/net/src/index.ts | 91 ++++++++--- packages/net/test/fetch.test.ts | 144 +++++++++++++++++ packages/net/test/index.test.ts | 63 +++++++- 5 files changed, 473 insertions(+), 113 deletions(-) diff --git a/packages/net/README.md b/packages/net/README.md index 2ef91d00..6d6d785e 100644 --- a/packages/net/README.md +++ b/packages/net/README.md @@ -119,6 +119,27 @@ const result2 = await net.post('https://api.example.com/data', { value: 1 }, { }); ``` +## Error Handling + +`@cacheable/net` follows native `fetch` semantics. It **resolves** with a `Response` for +every completed HTTP exchange — including `4xx` and `5xx` — and only rejects when the request +itself fails (DNS failure, connection refused, abort, etc.). Use `response.ok` (or +`response.status`) to detect HTTP errors instead of a `try/catch`: + +```javascript +const net = new CacheableNet(); + +const { response, data } = await net.get('https://api.example.com/thing'); +if (!response.ok) { + // 404, 500, etc. — `data` holds any error body the server returned + throw new Error(`Request failed with status ${response.status}`); +} +``` + +Only storable responses are cached. In simple caching mode (`httpCachePolicy: false`) that +means successful (2xx) responses only; under the default HTTP cache semantics it follows +RFC 7234. Error responses are always returned to the caller, never silently cached. + ## API Reference ### CacheableNet Class diff --git a/packages/net/src/fetch.ts b/packages/net/src/fetch.ts index b33959a6..44251b98 100644 --- a/packages/net/src/fetch.ts +++ b/packages/net/src/fetch.ts @@ -12,6 +12,52 @@ const runtimeFetch = globalThis.fetch.bind(globalThis) as unknown as ( init?: RequestInit, ) => Promise; +/** + * Reconstruct a `Response` while preserving the properties the WHATWG + * `Response` constructor does not let you set. Native `fetch` exposes the final + * URL via `response.url`; rebuilding with `new Response()` resets it to "". + * We reattach `url` (and `redirected`/`type` when rebuilding from a live + * response) so callers see the same shape native `fetch` returns. + * + * @param body The response body. + * @param init Standard response init (status, statusText, headers). + * @param source Optional source values to reattach (url, redirected, type). + * @returns A Response with native-`fetch` properties preserved. + */ +export function makeResponse( + body: BodyInit | null | undefined, + init: { status?: number; statusText?: string; headers?: HeadersInit }, + source: { url: string; redirected?: boolean; type?: string }, +): UndiciResponse { + const response = new Response(body, init); + + // `url` is always known (the request URL or the live response's final URL). + Object.defineProperty(response, "url", { + value: source.url, + configurable: true, + enumerable: true, + }); + + // `redirected`/`type` are only known when rebuilding from a live response. + if (source.redirected !== undefined) { + Object.defineProperty(response, "redirected", { + value: source.redirected, + configurable: true, + enumerable: true, + }); + } + + if (source.type !== undefined) { + Object.defineProperty(response, "type", { + value: source.type, + configurable: true, + enumerable: true, + }); + } + + return response as unknown as UndiciResponse; +} + export type FetchOptions = Omit & { cache?: Cacheable; /** @@ -54,21 +100,17 @@ export type FetchOptions = Omit & { */ export async function fetch( url: string, - options: FetchOptions, + options?: FetchOptions, ): Promise { const fetchOptions: RequestInit = { ...options, cache: "no-cache", }; - // If no cache provided, skip all caching logic - if (!options.cache) { - const response = await runtimeFetch(url, fetchOptions); - /* c8 ignore next 3 */ - if (!response.ok) { - throw new Error(`Fetch failed with status ${response.status}`); - } - return response; + // If no cache provided, skip all caching logic. Like native fetch, the + // response is returned regardless of status (no throw on non-2xx). + if (!options?.cache) { + return runtimeFetch(url, fetchOptions); } // Skip caching for POST, PATCH, DELETE, and HEAD requests @@ -78,12 +120,7 @@ export async function fetch( options.method === "DELETE" || options.method === "HEAD" ) { - const response = await runtimeFetch(url, fetchOptions); - /* c8 ignore next 3 */ - if (!response.ok) { - throw new Error(`Fetch failed with status ${response.status}`); - } - return response; + return runtimeFetch(url, fetchOptions); } const httpCachePolicy = options.httpCachePolicy !== false; // Default to true @@ -92,49 +129,61 @@ export async function fetch( // Create a cache key that includes the method const cacheKey = `${method}:${url}`; + type CachedResponse = { + body: string; + status: number; + statusText: string; + headers: Record; + }; + if (!httpCachePolicy) { - // Simple caching without HTTP cache semantics - const cachedData = await options.cache.getOrSet(cacheKey, async () => { - // Perform the fetch operation - const response = await runtimeFetch(url, fetchOptions); - /* v8 ignore next -- @preserve */ - if (!response.ok) { - throw new Error(`Fetch failed with status ${response.status}`); - } + // Simple caching without HTTP cache semantics. Return a cached response + // when present; otherwise fetch and only store successful responses. + // Non-2xx responses are returned (like native fetch) but never cached. + const cachedData = await options.cache.get(cacheKey); + if (cachedData) { + return makeResponse( + cachedData.body, + { + status: cachedData.status, + statusText: cachedData.statusText, + headers: cachedData.headers, + }, + { url }, + ); + } - // Convert response to cacheable format - const body = await response.text(); - return { + const response = await runtimeFetch(url, fetchOptions); + const body = await response.text(); + const headers = Object.fromEntries(response.headers.entries()); + + if (response.ok) { + await options.cache.set(cacheKey, { body, status: response.status, statusText: response.statusText, - headers: Object.fromEntries(response.headers.entries()), - }; - }); - - // Reconstruct Response object from cached data - /* v8 ignore next -- @preserve */ - if (!cachedData) { - throw new Error("Failed to get or set cache data"); + headers, + }); } - return new Response(cachedData.body, { - status: cachedData.status, - statusText: cachedData.statusText, - headers: cachedData.headers, - }) as UndiciResponse; + return makeResponse( + body, + { + status: response.status, + statusText: response.statusText, + headers, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); } // HTTP cache semantics enabled const policyKey = `${cacheKey}:policy`; - type CachedResponse = { - body: string; - status: number; - statusText: string; - headers: Record; - }; - // Try to get cached response and policy const [cachedResponse, cachedPolicyData] = await Promise.all([ options.cache.get(cacheKey), @@ -170,11 +219,15 @@ export async function fetch( if (policy?.satisfiesWithoutRevalidation(request)) { // Return cached response with updated headers const headers = policy.responseHeaders(); - return new Response(cachedBody, { - status: cachedStatus, - statusText: cachedStatusText, - headers: headers as HeadersInit, - }) as UndiciResponse; + return makeResponse( + cachedBody, + { + status: cachedStatus, + statusText: cachedStatusText, + headers: headers as HeadersInit, + }, + { url }, + ); } // Check if we need revalidation @@ -225,20 +278,20 @@ export async function fetch( // Return cached response with updated headers const headers = updatedPolicy.responseHeaders(); - return new Response(cachedBody, { - status: cachedStatus, - statusText: cachedStatusText, - headers: headers as HeadersInit, - }) as UndiciResponse; + return makeResponse( + cachedBody, + { + status: cachedStatus, + statusText: cachedStatusText, + headers: headers as HeadersInit, + }, + { url }, + ); } } - /* v8 ignore next -- @preserve */ - if (!response.ok && response.status !== 304) { - throw new Error(`Fetch failed with status ${response.status}`); - } - - // Read the response body + // Read the response body. Like native fetch, non-2xx responses are returned; + // http-cache-semantics decides below whether they are storable. const body = await response.text(); // Create response object for http-cache-semantics @@ -273,11 +326,19 @@ export async function fetch( } // Return the response - return new Response(body, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as UndiciResponse; + return makeResponse( + body, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); } export type DataResponse = { @@ -310,11 +371,19 @@ export async function get( } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as UndiciResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data, @@ -373,11 +442,19 @@ export async function post( } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as UndiciResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, @@ -437,11 +514,19 @@ export async function patch( } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as UndiciResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, @@ -525,11 +610,19 @@ export async function del( } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as UndiciResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, diff --git a/packages/net/src/index.ts b/packages/net/src/index.ts index e95aa423..8007cd24 100644 --- a/packages/net/src/index.ts +++ b/packages/net/src/index.ts @@ -5,6 +5,7 @@ import { type FetchOptions, type Response as FetchResponse, fetch, + makeResponse, } from "./fetch.js"; export type NetFetchOptions = { @@ -259,11 +260,19 @@ export class CacheableNet extends Hookified { } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as FetchResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data, @@ -333,11 +342,19 @@ export class CacheableNet extends Hookified { } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as FetchResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, @@ -434,11 +451,19 @@ export class CacheableNet extends Hookified { } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as FetchResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, @@ -508,11 +533,19 @@ export class CacheableNet extends Hookified { } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as FetchResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, @@ -584,11 +617,19 @@ export class CacheableNet extends Hookified { } // Create a new response with the text already consumed - const newResponse = new Response(text, { - status: response.status, - statusText: response.statusText, - headers: response.headers as HeadersInit, - }) as FetchResponse; + const newResponse = makeResponse( + text, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); return { data: responseData, diff --git a/packages/net/test/fetch.test.ts b/packages/net/test/fetch.test.ts index b16bd16b..67d3f399 100644 --- a/packages/net/test/fetch.test.ts +++ b/packages/net/test/fetch.test.ts @@ -1150,4 +1150,148 @@ describe("Fetch", () => { expect(body.toString("utf8")).toContain('name="id"'); }); }); + + // Native fetch parity: @cacheable/net previously threw on any non-2xx + // response, making `response.ok` checks unreachable dead code. Native fetch + // resolves with the Response regardless of status and only rejects on + // network errors. These tests use a local server to assert that behavior + // deterministically across every code path. + describe("Native fetch parity (local server)", () => { + let baseUrl = ""; + let server: http.Server; + + beforeAll(async () => { + server = http.createServer((req, res) => { + const path = req.url ?? "/"; + if (path.startsWith("/status/")) { + const code = Number.parseInt(path.slice("/status/".length), 10); + res.writeHead(code, { "content-type": "application/json" }); + res.end(JSON.stringify({ status: code })); + return; + } + if (path === "/redirect") { + res.writeHead(302, { location: "/ok" }); + res.end(); + return; + } + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ ok: true })); + }); + await new Promise((resolve) => server.listen(0, resolve)); + const { port } = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${port}`; + }); + + afterAll(async () => { + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + }); + + test("fetch resolves (does not throw) on 404 with a cache", async () => { + const response = await fetch(`${baseUrl}/status/404`, { + cache: new Cacheable(), + }); + expect(response.status).toBe(404); + expect(response.ok).toBe(false); + }); + + test("fetch resolves on 500 in simple-cache mode", async () => { + const response = await fetch(`${baseUrl}/status/500`, { + cache: new Cacheable(), + httpCachePolicy: false, + }); + expect(response.status).toBe(500); + expect(response.ok).toBe(false); + }); + + test("fetch resolves on non-2xx without a cache", async () => { + const response = await fetch(`${baseUrl}/status/404`, { + cache: undefined, + }); + expect(response.status).toBe(404); + expect(response.ok).toBe(false); + }); + + test("fetch works with no options at all (like native fetch)", async () => { + const response = await fetch(`${baseUrl}/status/500`); + expect(response.status).toBe(500); + expect(response.ok).toBe(false); + }); + + test("POST resolves on non-2xx (not cached)", async () => { + const response = await fetch(`${baseUrl}/status/404`, { + method: "POST", + cache: new Cacheable(), + body: JSON.stringify({ a: 1 }), + headers: { "Content-Type": "application/json" }, + }); + expect(response.status).toBe(404); + expect(response.ok).toBe(false); + }); + + test("HEAD resolves on non-2xx (not cached)", async () => { + const response = await fetch(`${baseUrl}/status/500`, { + method: "HEAD", + cache: new Cacheable(), + }); + expect(response.status).toBe(500); + expect(response.ok).toBe(false); + }); + + test("error responses are not cached in simple-cache mode", async () => { + const cache = new Cacheable({ stats: true }); + const options: FetchOptions = { cache, httpCachePolicy: false }; + const first = await fetch(`${baseUrl}/status/500`, options); + const second = await fetch(`${baseUrl}/status/500`, options); + expect(first.status).toBe(500); + expect(second.status).toBe(500); + // Both were live fetches; nothing was served from cache. + expect(cache.stats.hits).toBe(0); + }); + + test("successful responses are still cached in simple-cache mode", async () => { + const cache = new Cacheable({ stats: true }); + const options: FetchOptions = { cache, httpCachePolicy: false }; + await fetch(`${baseUrl}/ok`, options); + await fetch(`${baseUrl}/ok`, options); + expect(cache.stats.hits).toBe(1); + }); + + test("get helper returns non-2xx without throwing", async () => { + const result = await get<{ status: number }>(`${baseUrl}/status/404`, { + cache: new Cacheable(), + }); + expect(result.response.status).toBe(404); + expect(result.response.ok).toBe(false); + expect(result.data).toEqual({ status: 404 }); + }); + + test("post helper returns non-2xx without throwing", async () => { + const result = await post( + `${baseUrl}/status/500`, + { a: 1 }, + { cache: new Cacheable() }, + ); + expect(result.response.status).toBe(500); + expect(result.response.ok).toBe(false); + }); + + test("response.url is preserved on a reconstructed cached response", async () => { + const cache = new Cacheable(); + const options: FetchOptions = { cache, httpCachePolicy: false }; + await fetch(`${baseUrl}/ok`, options); + const cached = await fetch(`${baseUrl}/ok`, options); + expect(cached.url).toBe(`${baseUrl}/ok`); + }); + + test("response.url and redirected survive the get helper", async () => { + const result = await get(`${baseUrl}/redirect`, { + cache: new Cacheable(), + }); + expect(result.response.status).toBe(200); + expect(result.response.url).toBe(`${baseUrl}/ok`); + expect(result.response.redirected).toBe(true); + }); + }); }); diff --git a/packages/net/test/index.test.ts b/packages/net/test/index.test.ts index b3a5d1d1..4db2c2cb 100644 --- a/packages/net/test/index.test.ts +++ b/packages/net/test/index.test.ts @@ -1,7 +1,9 @@ +import http from "node:http"; +import type { AddressInfo } from "node:net"; import process from "node:process"; import { faker } from "@faker-js/faker"; import { Cacheable } from "cacheable"; -import { describe, expect, test } from "vitest"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { CacheableNet, type CacheableNetOptions, @@ -1376,4 +1378,63 @@ describe("Cacheable Net", () => { }, testTimeout, ); + + // Native fetch parity: CacheableNet helpers must resolve with the Response + // on non-2xx (like native fetch) instead of throwing, so callers can inspect + // response.ok/status. A local server makes this deterministic. + describe("Native fetch parity (local server)", () => { + let baseUrl = ""; + let server: http.Server; + + beforeAll(async () => { + server = http.createServer((req, res) => { + const path = req.url ?? "/"; + if (path.startsWith("/status/")) { + const code = Number.parseInt(path.slice("/status/".length), 10); + res.writeHead(code, { "content-type": "application/json" }); + res.end(JSON.stringify({ status: code })); + return; + } + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ ok: true })); + }); + await new Promise((resolve) => server.listen(0, resolve)); + const { port } = server.address() as AddressInfo; + baseUrl = `http://127.0.0.1:${port}`; + }); + + afterAll(async () => { + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); + }); + + test("get resolves with the Response on 404 (no throw)", async () => { + const net = new CacheableNet(); + const result = await net.get<{ status: number }>(`${baseUrl}/status/404`); + expect(result.response.status).toBe(404); + expect(result.response.ok).toBe(false); + expect(result.data).toEqual({ status: 404 }); + }); + + test("post resolves with the Response on 500 (no throw)", async () => { + const net = new CacheableNet(); + const result = await net.post(`${baseUrl}/status/500`, { a: 1 }); + expect(result.response.status).toBe(500); + expect(result.response.ok).toBe(false); + }); + + test("fetch resolves with the Response on non-2xx", async () => { + const net = new CacheableNet(); + const response = await net.fetch(`${baseUrl}/status/404`); + expect(response.status).toBe(404); + expect(response.ok).toBe(false); + }); + + test("response.url is preserved through the get helper", async () => { + const net = new CacheableNet(); + const result = await net.get(`${baseUrl}/ok`); + expect(result.response.url).toBe(`${baseUrl}/ok`); + }); + }); }); From 3e950abdf8b5e026c0133270a5e5746018c5f7b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 16:53:56 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(net):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?no=20error=20caching,=20coalescing,=20304=20+=20cached-redirect?= =?UTF-8?q?=20metadata?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/net/README.md | 8 +- packages/net/package.json | 1 + packages/net/src/fetch.ts | 134 +++++++++++++++++++++++--------- packages/net/test/fetch.test.ts | 99 +++++++++++++++++++++++ pnpm-lock.yaml | 3 + 5 files changed, 204 insertions(+), 41 deletions(-) diff --git a/packages/net/README.md b/packages/net/README.md index 6d6d785e..ee3ab1e5 100644 --- a/packages/net/README.md +++ b/packages/net/README.md @@ -136,9 +136,11 @@ if (!response.ok) { } ``` -Only storable responses are cached. In simple caching mode (`httpCachePolicy: false`) that -means successful (2xx) responses only; under the default HTTP cache semantics it follows -RFC 7234. Error responses are always returned to the caller, never silently cached. +Only successful responses are cached. Under the default HTTP cache mode, `2xx` responses are +cached per RFC 7234 (honoring `Cache-Control`, `ETag`, `Expires`, etc.); in simple mode +(`httpCachePolicy: false`) every `2xx` response is cached. Error responses (`4xx`/`5xx`) are +always returned to the caller but **never** cached, so a transient failure is never replayed +from a cache hit. ## API Reference diff --git a/packages/net/package.json b/packages/net/package.json index c4cf14bb..f58ff766 100644 --- a/packages/net/package.json +++ b/packages/net/package.json @@ -40,6 +40,7 @@ "typescript": "^5.9.3" }, "dependencies": { + "@cacheable/utils": "workspace:^", "cacheable": "workspace:^", "hookified": "^1.15.1", "http-cache-semantics": "^4.2.0", diff --git a/packages/net/src/fetch.ts b/packages/net/src/fetch.ts index 44251b98..aebba4fe 100644 --- a/packages/net/src/fetch.ts +++ b/packages/net/src/fetch.ts @@ -1,3 +1,4 @@ +import { coalesceAsync } from "@cacheable/utils"; import type { Cacheable } from "cacheable"; import CachePolicy from "http-cache-semantics"; import type { RequestInit, Response as UndiciResponse } from "undici"; @@ -12,6 +13,11 @@ const runtimeFetch = globalThis.fetch.bind(globalThis) as unknown as ( init?: RequestInit, ) => Promise; +// Statuses that must not carry a response body. Reconstructing one of these +// with a non-null body (e.g. the "" returned by response.text() on a 304/204) +// throws in the Response constructor, so callers coerce the body to null. +const NULL_BODY_STATUSES = new Set([101, 103, 204, 205, 304]); + /** * Reconstruct a `Response` while preserving the properties the WHATWG * `Response` constructor does not let you set. Native `fetch` exposes the final @@ -29,7 +35,13 @@ export function makeResponse( init: { status?: number; statusText?: string; headers?: HeadersInit }, source: { url: string; redirected?: boolean; type?: string }, ): UndiciResponse { - const response = new Response(body, init); + // Null-body statuses (204/304/…) cannot carry a body; coerce to null so a + // reconstructed conditional/empty response is returned instead of throwing. + const safeBody = + init.status !== undefined && NULL_BODY_STATUSES.has(init.status) + ? null + : body; + const response = new Response(safeBody, init); // `url` is always known (the request URL or the live response's final URL). Object.defineProperty(response, "url", { @@ -134,49 +146,61 @@ export async function fetch( status: number; statusText: string; headers: Record; + // Native-fetch metadata persisted so cache hits report the same final URL + // (after redirects) and redirected/type as the original cache miss. + url?: string; + redirected?: boolean; + type?: string; }; if (!httpCachePolicy) { - // Simple caching without HTTP cache semantics. Return a cached response - // when present; otherwise fetch and only store successful responses. - // Non-2xx responses are returned (like native fetch) but never cached. - const cachedData = await options.cache.get(cacheKey); - if (cachedData) { - return makeResponse( - cachedData.body, - { - status: cachedData.status, - statusText: cachedData.statusText, - headers: cachedData.headers, - }, - { url }, - ); - } + // Simple caching without HTTP cache semantics. Coalesce concurrent misses + // for the same key so we don't stampede the origin (like Cacheable.getOrSet + // did before), return cached data when present, and only store successful + // responses. Non-2xx responses are returned (like native fetch) but never + // cached. Each caller rebuilds its own Response from the shared data so the + // body can be consumed independently. + // Capture the (now-narrowed) cache so it stays non-undefined inside the + // coalesce closure, where TypeScript would otherwise widen it again. + const cache = options.cache; + const cachedData = await coalesceAsync( + `net:simple:${cacheKey}`, + async () => { + const existing = await cache.get(cacheKey); + if (existing) { + return existing; + } + + const response = await runtimeFetch(url, fetchOptions); + const result: CachedResponse = { + body: await response.text(), + status: response.status, + statusText: response.statusText, + headers: Object.fromEntries(response.headers.entries()), + url: response.url, + redirected: response.redirected, + type: response.type, + }; - const response = await runtimeFetch(url, fetchOptions); - const body = await response.text(); - const headers = Object.fromEntries(response.headers.entries()); + if (response.ok) { + await cache.set(cacheKey, result); + } - if (response.ok) { - await options.cache.set(cacheKey, { - body, - status: response.status, - statusText: response.statusText, - headers, - }); - } + return result; + }, + ); return makeResponse( - body, + cachedData.body, { - status: response.status, - statusText: response.statusText, - headers, + status: cachedData.status, + statusText: cachedData.statusText, + headers: cachedData.headers, }, { - url: response.url, - redirected: response.redirected, - type: response.type, + url: cachedData.url ?? url, + redirected: cachedData.redirected, + type: cachedData.type, }, ); } @@ -195,6 +219,9 @@ export async function fetch( let cachedStatus: number | undefined; let cachedStatusText: string | undefined; let cachedHeaders: Record | undefined; + let cachedUrl: string | undefined; + let cachedRedirected: boolean | undefined; + let cachedType: string | undefined; if (cachedPolicyData && cachedResponse) { // Deserialize the policy @@ -205,6 +232,9 @@ export async function fetch( cachedStatus = cachedResponse.status; cachedStatusText = cachedResponse.statusText; cachedHeaders = cachedResponse.headers; + cachedUrl = cachedResponse.url; + cachedRedirected = cachedResponse.redirected; + cachedType = cachedResponse.type; } // Prepare the request for http-cache-semantics @@ -226,7 +256,7 @@ export async function fetch( statusText: cachedStatusText, headers: headers as HeadersInit, }, - { url }, + { url: cachedUrl ?? url, redirected: cachedRedirected, type: cachedType }, ); } @@ -272,6 +302,9 @@ export async function fetch( status: cachedStatus, statusText: cachedStatusText, headers: cachedHeaders, + url: cachedUrl, + redirected: cachedRedirected, + type: cachedType, }, ttl, ); @@ -285,15 +318,37 @@ export async function fetch( statusText: cachedStatusText, headers: headers as HeadersInit, }, - { url }, + { + url: cachedUrl ?? url, + redirected: cachedRedirected, + type: cachedType, + }, ); } } - // Read the response body. Like native fetch, non-2xx responses are returned; - // http-cache-semantics decides below whether they are storable. + // Read the response body. Like native fetch, non-2xx responses are returned. const body = await response.text(); + // Per the documented contract, error responses are returned to the caller but + // never cached — even though RFC 7234 would consider some (404/410/501/…) + // storable. Return early so no policy is created or stored for them. + if (!response.ok) { + return makeResponse( + body, + { + status: response.status, + statusText: response.statusText, + headers: response.headers as HeadersInit, + }, + { + url: response.url, + redirected: response.redirected, + type: response.type, + }, + ); + } + // Create response object for http-cache-semantics const responseForPolicy = { status: response.status, @@ -318,6 +373,9 @@ export async function fetch( status: response.status, statusText: response.statusText, headers: responseForPolicy.headers, + url: response.url, + redirected: response.redirected, + type: response.type, }, ttl, ), diff --git a/packages/net/test/fetch.test.ts b/packages/net/test/fetch.test.ts index 67d3f399..13895338 100644 --- a/packages/net/test/fetch.test.ts +++ b/packages/net/test/fetch.test.ts @@ -1159,6 +1159,7 @@ describe("Fetch", () => { describe("Native fetch parity (local server)", () => { let baseUrl = ""; let server: http.Server; + let coalesceHits = 0; beforeAll(async () => { server = http.createServer((req, res) => { @@ -1174,6 +1175,23 @@ describe("Fetch", () => { res.end(); return; } + if (path === "/coalesce") { + // Count origin hits and delay so concurrent requests overlap. + coalesceHits += 1; + setTimeout(() => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ ok: true })); + }, 50); + return; + } + if (path === "/cacheable") { + res.writeHead(200, { + "content-type": "application/json", + "cache-control": "max-age=3600", + }); + res.end(JSON.stringify({ ok: true })); + return; + } res.writeHead(200, { "content-type": "application/json" }); res.end(JSON.stringify({ ok: true })); }); @@ -1293,5 +1311,86 @@ describe("Fetch", () => { expect(result.response.url).toBe(`${baseUrl}/ok`); expect(result.response.redirected).toBe(true); }); + + test("get helper returns a 304 without throwing (null-body status)", async () => { + // A conditional GET with no cache hits the no-cache path; the helper + // must rebuild a 304 (a null-body status) without throwing. + const result = await get(`${baseUrl}/status/304`, { cache: undefined }); + expect(result.response.status).toBe(304); + }); + + test("concurrent simple-cache misses coalesce into one origin request", async () => { + const cache = new Cacheable(); + const options: FetchOptions = { cache, httpCachePolicy: false }; + coalesceHits = 0; + const responses = await Promise.all([ + fetch(`${baseUrl}/coalesce`, options), + fetch(`${baseUrl}/coalesce`, options), + fetch(`${baseUrl}/coalesce`, options), + ]); + // Every caller succeeds and can read its own independent body. + for (const response of responses) { + expect(response.status).toBe(200); + expect(await response.text()).toBe('{"ok":true}'); + } + // The origin was hit only once despite three concurrent misses. + expect(coalesceHits).toBe(1); + }); + + test("cached redirect reports the final url + redirected on cache hits", async () => { + const cache = new Cacheable(); + const options: FetchOptions = { cache, httpCachePolicy: false }; + const miss = await fetch(`${baseUrl}/redirect`, options); + const hit = await fetch(`${baseUrl}/redirect`, options); + expect(miss.url).toBe(`${baseUrl}/ok`); + // The cache hit reports the same final URL and redirected flag as the miss. + expect(hit.url).toBe(`${baseUrl}/ok`); + expect(hit.redirected).toBe(true); + }); + + test("simple-cache hit falls back to the request url for legacy entries", async () => { + // Entries written by older versions have no url/redirected/type metadata; + // the request URL is used as a fallback so they still resolve correctly. + const cache = new Cacheable(); + await cache.set(`GET:${baseUrl}/ok`, { + body: '{"legacy":true}', + status: 200, + statusText: "OK", + headers: { "content-type": "application/json" }, + }); + const response = await fetch(`${baseUrl}/ok`, { + cache, + httpCachePolicy: false, + }); + expect(response.status).toBe(200); + expect(response.url).toBe(`${baseUrl}/ok`); + expect(await response.text()).toBe('{"legacy":true}'); + }); + + test("http-cache hit falls back to the request url for legacy entries", async () => { + const cache = new Cacheable(); + const key = `GET:${baseUrl}/cacheable`; + // Populate a real, fresh cache policy from a cacheable response. + await fetch(`${baseUrl}/cacheable`, { cache, httpCachePolicy: true }); + // Simulate a legacy entry (no url/redirected/type) while keeping the + // freshly-stored policy, so the next request satisfies without + // revalidation and exercises the request-url fallback. + await cache.set(key, { + body: '{"cached":true}', + status: 200, + statusText: "OK", + headers: { + "content-type": "application/json", + "cache-control": "max-age=3600", + }, + }); + const response = await fetch(`${baseUrl}/cacheable`, { + cache, + httpCachePolicy: true, + }); + expect(response.status).toBe(200); + expect(response.url).toBe(`${baseUrl}/cacheable`); + expect(await response.text()).toBe('{"cached":true}'); + }); }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9a47d5df..a1869e58 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -255,6 +255,9 @@ importers: packages/net: dependencies: + '@cacheable/utils': + specifier: workspace:^ + version: link:../utils cacheable: specifier: workspace:^ version: link:../cacheable From 961093dcdb3be7ff491b8dc9fc19d13c251d5a70 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 17:05:45 +0000 Subject: [PATCH 3/3] docs(net): describe native fetch semantics in README features 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 --- packages/net/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/net/README.md b/packages/net/README.md index ee3ab1e5..f1a9f31f 100644 --- a/packages/net/README.md +++ b/packages/net/README.md @@ -10,7 +10,8 @@ Features: -* `fetch` from [undici](https://github.com/nodejs/undici) with caching enabled via `cacheable` +* Drop-in `fetch` with native semantics (built on the runtime's global `fetch`) — resolves with a `Response` on any status (check `response.ok`, no throwing on `4xx`/`5xx`) and preserves `response.url`, `redirected`, and `type` +* Optional response caching via `cacheable` — pass a cache instance (or use `CacheableNet`) to enable it * HTTP method helpers: `get`, `post`, `put`, `patch`, `delete`, and `head` for easier development * [RFC 7234](http://httpwg.org/specs/rfc7234.html) compliant HTTP caching with `http-cache-semantics` * Smart caching with automatic cache key generation