fix(node/http): resolve stream.finished() hanging on destroyed server requests #30580
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
![gitstart-app[bot]](https://avatars.githubusercontent.com/in/46283?s=60&v=4){kind=small}
… requests IncomingMessageForServer was missing proper aborted state management, causing stream.finished() to hang when req.destroy() was called. - Add #aborted field and update aborted getter - Implement proper destroy() method with correct event emission - Add test to node compatibility suite - Handle edge cases and race conditions Fixes parallel/test-stream-finished.js hanging at line 579.
gitstart-app
bot
changed the title
node:stream tracking issue - parallel/test-stream-finished.js… requests IncomingMessageForServer was missing proper aborted state management, causing stream.finished() to hang when req.destroy() was called. - Add #aborted field and update aborted getter - Implement proper destroy() method with correct event emission - Add test to node compatibility suite - Handle edge cases and race conditions Fixes parallel/test-stream-finished.js hanging at line 579.
bartlomieju
requested changes
Sep 1, 2025
added 4 commits
September 1, 2025 11:56
…s hanging requests into proper timeout errors, matching Node.js behavior.
added 2 commits
September 3, 2025 06:23
…shed() compatibility - Add connection termination error detection in Rust HTTP ops layer - Implement client-server request registry for error propagation - Fix FakeSocket destroy behavior with proper ECONNRESET emission - Enhance IncomingMessageForServer with proper destroy() method - Add stream state management for finished() compatibility - Enable parallel/test-stream-finished.js in Node.js compatibility suite Fixes failing Node.js compatibility test by implementing proper request destruction patterns that emit ECONNRESET errors when server calls req.destroy(), matching Node.js behavior.
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.
This PR was created by GitStart to address the requirements from this ticket: DENB-29802.
Summary
Fixes
parallel/test-stream-finished.jsin the Node.js compatibility suite by adopting Node.js-compatible patterns for HTTP request destruction and error propagation, restoring correctstream.finished()behavior.Problem
req.destroy(), client requests did not receive the expectedECONNRESET.stream.finished()misbehaved due to improper stream state management.Solution
ext/node/ops/http.rs— detect termination conditions and map Hyper errors to Node.js-compatibleECONNRESET.ext/node/polyfills/http.ts— port-based registry to match client requests with server connections and signal them on destruction.FakeSocketdestroy semanticsProper socket destruction with error propagation to the corresponding client requests.
IncomingMessageForServerImplement
destroy()with aborted-state tracking and correct stream state handling.ClientRequestcleanupRobust error propagation and registry cleanup on destruction.
finished()ext/node/polyfills/_http_outgoing.ts— add explicit error/finished flags forfinished()compatibility.Key Features
ECONNRESETpropagation when the server destroys requests.finished().Files Changed
ext/node/ops/http.rs— termination detection andECONNRESETmappingext/node/polyfills/http.ts— request registry,FakeSocketdestroy behavior, server/client integrationext/node/polyfills/_http_outgoing.ts— state flags forfinished()compatibilityTesting
parallel/test-stream-finished.jsECONNRESET) and cleanup