fix: fetch cache key generation — handle all body types, include all headers minus blocklist

[threepointone]

Summary

Fixes two bugs in the fetch cache key generation and adds a third bug fix discovered during review.

Bug 1: Binary/FormData bodies ignored in cache key

buildFetchCacheKey only handled string bodies. Requests with Uint8Array, ReadableStream, FormData, Blob, or URLSearchParams bodies all produced the same cache key regardless of body content — causing incorrect cache hits.

Fix: serializeBody() now handles all BodyInit types. buildFetchCacheKey is now async to support stream/blob consumption.

Bug 2: Header inclusion strategy inverted vs Next.js

We used an allowlist of 3 auth headers (authorization, cookie, x-api-key). Next.js uses a blocklist (traceparent, tracestate) and includes everything else. This meant headers like Accept, Accept-Language, and custom headers were ignored — different requests could incorrectly share cache entries.

Fix: Switched to blocklist approach matching Next.js. All headers are now included in the cache key except W3C trace context headers.

Bug 3: Consumed stream body sent to network (found during review)

After serializeBody consumed a ReadableStream body for cache key generation, the spent stream was still passed to originalFetch(). The reconstructed body (_ogBody) was stashed but never restored.

Fix: stripNextFromInit now restores _ogBody as the body before passing to originalFetch.

Additional improvements

• Cache key includes mode, redirect, credentials, referrer, referrerPolicy, integrity, cache from RequestInit
• Key is SHA-256 hashed (matching Next.js) with CACHE_KEY_PREFIX = "v1" for future cache-busting
• URLSearchParams handled separately from FormData (different APIs)

Tests

19 new tests (52 total):

• 7 body type cache key tests (string, Uint8Array, Blob, FormData, ReadableStream)
• 6 header inclusion tests (Accept, custom headers, trace header exclusion, order independence)
• 4 body restoration tests (verify originalFetch gets usable body after key generation)
• 2 URLSearchParams tests

Not breaking

All changes are internal. Public API unchanged. Only user-visible effect: one-time cold cache after upgrade (new key format).

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/vinext@111

commit: 3d5c73d

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original

[threepointone]

/bigbonk review this PR

[ask-bonk]

Review Summary

Solid, well-tested bug fix PR. All three bugs are real and the fixes are correct. The approach of switching from an allowlist to a blocklist for header inclusion matches Next.js behavior and is the right call. 52 tests pass (19 new).

What's Good

• Comprehensive test coverage: 19 new tests covering all body types (string, Uint8Array, Blob, FormData, ReadableStream, URLSearchParams), header edge cases (Accept, custom headers, trace header exclusion, order independence), body restoration, and cache key determinism.
• SHA-256 hashing: Moving from plaintext keys (fetch:GET:url|body|auth:...) to SHA-256 hashed keys is good practice — prevents key injection and keeps key length bounded.
• CACHE_KEY_PREFIX = "v1": Smart approach for future cache-busting on key format changes.
• Bug 3 fix is subtle and important: The _ogBody stashing in serializeBody + restoration in stripNextFromInit is a clean solution for the consumed-stream problem. Without this, every cached ReadableStream request would send a spent stream to the network.
• collectHeaders normalizes through the Headers API (line 57-59), which sorts by header name per spec. This makes the cache key deterministic regardless of property insertion order in plain objects.

Issues

Stream error handling leaves request in broken state (non-blocking)

serializeBody at fetch-cache.ts:104-132: if readableBody.pipeTo() fails, the catch block logs but doesn't set _ogBody. This means:

1. bodyChunks may contain partial data → cache key generated from incomplete body
2. _ogBody is never set → stripNextFromInit won't find it → the original (now consumed) stream is passed to originalFetch → network request fails with a "body already consumed" error

The catch block should either re-throw (so the fetch falls through to the original unpatched path) or set _ogBody to the partial data so the request at least has something to send. As-is, a stream read failure during cache key generation silently breaks the actual fetch call.

    } catch (err) {
      console.error("[vinext] Problem reading body for cache key", err);
      // Still reconstruct what we have so originalFetch doesn't get a spent stream
      if (chunks.length > 0) {
        const length = chunks.reduce((total, arr) => total + arr.length, 0);
        const partial = new Uint8Array(length);
        let offset = 0;
        for (const chunk of chunks) {
          partial.set(chunk, offset);
          offset += chunk.length;
        }
        (init as any)._ogBody = partial;
      }
    }

FormData File serialization doesn't include metadata (informational)

fetch-cache.ts:146-149: File objects in FormData are serialized via val.text(), which reads content but ignores name, lastModified, and type. Two Files with identical content but different names produce the same cache key. This is an edge case (caching FormData with File uploads is unusual) but worth a code comment for future reference.

Fixture rebuild (entry.js)

The tests/fixtures/pages-basic/dist/server/entry.js change (+111/-29) is a rebuilt production bundle reflecting the fetch-cache changes. This is expected since this module is bundled into it. Just confirming this file is intentionally tracked in the repo.

Verdict

Approving — the fixes are correct, well-tested, and the approach matches Next.js semantics. The stream error handling suggestion is non-blocking and can be addressed in a follow-up.

[ask-bonk]

Review posted. Here's a summary of my findings:

Approved PR #111 with the following observations:

All 52 tests pass (19 new), covering body type handling, header inclusion/exclusion, body restoration, and URLSearchParams.

The three bug fixes are correct:

1. serializeBody() now handles all BodyInit types (Uint8Array, ReadableStream, FormData, Blob, URLSearchParams, string)
2. Header strategy switched from allowlist (3 auth headers) to blocklist (only traceparent/tracestate excluded), matching Next.js
3. stripNextFromInit now restores _ogBody so consumed streams aren't sent to the network

One non-blocking suggestion: If ReadableStream.pipeTo() fails in serializeBody, the catch block logs but doesn't set _ogBody, which means originalFetch would receive a spent stream and fail silently. Suggested reconstructing partial data in the error path so the fetch doesn't break entirely.

github run

[threepointone]

/bigbonk revew this pr again

[ask-bonk]

Review Summary

All three bug fixes are correct and well-implemented. 52/52 tests pass. The previous review's non-blocking suggestion (stream error handling) has already been addressed in the current code. Approving.

Analysis

Bug 1: Binary/FormData bodies — Correct

serializeBody() at fetch-cache.ts:89-178 correctly handles all BodyInit types with appropriate serialization for each. The type detection ordering matters and is correct:

1. Uint8Array (direct decode)
2. ReadableStream (duck-typed via getReader)
3. URLSearchParams (checked before FormData since both have .keys(), but USP has .toString())
4. FormData (duck-typed via .keys())
5. Blob (duck-typed via .arrayBuffer())
6. string (fallback)

Bug 2: Header blocklist — Correct

collectHeaders() at fetch-cache.ts:47-69 merges Request + init headers with correct precedence (init wins), normalizes through the Headers API which sorts by name per spec, then strips only traceparent/tracestate. This matches Next.js behavior.

Bug 3: Body restoration — Correct

stripNextFromInit() at fetch-cache.ts:481-491 destructures _ogBody from init and restores it as body before passing to originalFetch. The stream error catch block at lines 130-142 now also sets _ogBody with partial data, addressing the previous review's suggestion.

Cache key hashing — Correct

buildFetchCacheKey() at fetch-cache.ts:188-228 includes CACHE_KEY_PREFIX, URL, method, all headers, relevant RequestInit properties (mode, redirect, credentials, referrer, referrerPolicy, integrity, cache), and serialized body chunks in a JSON array, then SHA-256 hashes it. The prefix enables future cache-busting on format changes.

Fixture rebuild

The entry.js change is a production bundle reflecting the source changes — expected and correct.

One observation (informational, not blocking)

The collectHeaders approach of building a plain object merged and then JSON-serializing it in buildFetchCacheKey relies on the fact that Headers.forEach iterates in sorted order per spec. If headers were added via the plain object path (init.headers is a Record<string, string>), they go through new Headers() which normalizes and sorts them. This is correct but worth noting: the determinism guarantee comes from the Headers constructor, not from JSON.stringify property ordering.

[ask-bonk]

PR #111 reviewed and approved.

All 52 tests pass (19 new). The three bug fixes are correct:

1. Binary/FormData bodies — serializeBody() handles all BodyInit types with correct type detection ordering (Uint8Array > ReadableStream > URLSearchParams > FormData > Blob > string)
2. Header blocklist — Switched from 3-header allowlist to blocklist of only traceparent/tracestate, matching Next.js behavior. Header merging respects fetch spec precedence (init > Request).
3. Body restoration — stripNextFromInit now restores _ogBody so consumed streams aren't sent to the network. The stream error path also reconstructs partial data, addressing the previous review's suggestion.

The SHA-256 hashing with CACHE_KEY_PREFIX = "v1" is a clean approach for bounded key length and future cache-busting. The fixture entry.js change is just the rebuilt production bundle.

github run