fix: only original value destroy if the new value is not a stream#1914
fix: only original value destroy if the new value is not a stream#1914
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1914 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 9 9
Lines 2081 2084 +3
=======================================
+ Hits 2079 2082 +3
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where setting res.body to a new stream would incorrectly destroy the original stream, breaking middleware patterns that wrap streams (like compression middleware). The fix prevents destroying the original stream when the new value is also a stream, while still cleaning up streams when replaced by non-stream values.
Key Changes:
- Modified stream cleanup logic to only destroy the original stream when replaced by a non-stream value
- Updated test expectations to reflect that streams should not be destroyed when replaced by other streams
- Added a new test case demonstrating the wrapping stream middleware pattern
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/response.js | Added conditional check to prevent destroying original stream when new value is also a stream |
| tests/response/body.test.js | Updated test expectations and added new test case for wrapping stream middleware pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This PR adds a minimal fix + tests for #1913.
Thanks @nuintun for their observation and solutions.
Checklist