Skip to content

Conversation

@mcollina
Copy link
Member

Summary

  • Fixes cache.match() throwing "Body has already been consumed" after garbage collection runs between calls
  • When a cached response's stream becomes unusable (cancelled by FinalizationRegistry), recreates the stream from the stored body source

Root Cause

The bug occurs due to the interaction between the FinalizationRegistry (used to track Response objects) and the cloneBody() function which mutates the original body's stream in-place:

  1. cache.match() calls fromInnerResponse() which creates a temporary Response and registers the cached stream
  2. Then response.clone() is called, which tees the stream and mutates the cached stream to be one of the tee outputs
  3. clone() re-registers this mutated stream with the temporary Response
  4. When the temporary Response is GC'd, the FinalizationRegistry callback cancels the stream
  5. Since the cancelled stream IS the cached stream (mutated in place), subsequent cache.match() calls fail

Fix

The fix checks if the cached stream is unusable but body.source is available (which cache.put() always stores after reading the body bytes), and recreates the stream from source before creating the Response object.

Fixes: #4710

Test plan

  • Added test that reproduces the issue with GC pressure
  • Test passes with the fix
  • Full cache test suite passes
  • Linter passes

🤖 Generated with Claude Code

…after GC

The cached response's stream could be cancelled by the FinalizationRegistry
when a temporary Response object wrapping the inner response was garbage
collected. This caused cache.match() to throw "Body has already been consumed"
after garbage collection ran between calls.

The fix checks if the cached stream is unusable (locked or disturbed) but
the body source is available (which cache.put() always stores), and if so,
recreates the stream from the source before creating the Response object.

Fixes: #4710

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.15%. Comparing base (ec4a84e) to head (fcf306b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4713      +/-   ##
==========================================
+ Coverage   92.85%   93.15%   +0.29%     
==========================================
  Files         109      109              
  Lines       33809    33837      +28     
==========================================
+ Hits        31395    31522     +127     
+ Misses       2414     2315      -99     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

match throws an error after external resource allocation occurs

3 participants