fix: prevent connection leak when redirect='error' #4629
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes issue where SSE and other long-lived connections could leak when using fetch with redirect: 'error' option. The spec optimization of reusing fetchParams when window is 'no-window' and redirect is 'error' was causing connection lifecycle issues. In Node.js, request.window is always set to 'no-window', so this optimization was being applied in all cases where redirect: 'error' was used. This caused problems with connection cleanup, particularly for EventSource and SSE use cases where the connection needs to be properly aborted. The fix disables this optimization and always clones the request and copies fetchParams to ensure proper connection lifecycle management. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Fixes: #4627 Signed-off-by: Matteo Collina <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4629 +/- ##
==========================================
- Coverage 92.94% 92.92% -0.02%
==========================================
Files 106 106
Lines 33092 33094 +2
==========================================
- Hits 30756 30754 -2
- Misses 2336 2340 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
KhafraDev
requested changes
Oct 18, 2025
Member
There was a problem hiding this comment.
I find it highly unlikely that this is an issue in undici. The eventsource package on npm creates its own signal and doesn't seem to work with a custom one (when providing your own fetch function), hence it's never aborted. I'd need a smaller repro to be certain. Also, if it is a spec issue, we should upstream a fix, or at the very least a note.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes issue where SSE and other long-lived connections could leak when using fetch with redirect: 'error' option. The spec optimization of reusing fetchParams when window is 'no-window' and redirect is 'error' was causing connection lifecycle issues.
In Node.js, request.window is always set to 'no-window', so this optimization was being applied in all cases where redirect: 'error' was used. This caused problems with connection cleanup, particularly for EventSource and SSE use cases where the connection needs to be properly aborted.
The fix disables this optimization and always clones the request and copies fetchParams to ensure proper connection lifecycle management.
🤖 Generated with Claude Code
Fixes: #4627