Repository navigation
fix(listener): abort the request signal when an HTTP/2 client resets the stream - #404
Open
affanmomin wants to merge 1 commit into
Open
affanmomin wants to merge 1 commit into
affanmomin wants to merge 1 commit into
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
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 #398
When an HTTP/2 client cancels a request (for example Ctrl+C on
curl --http2-prior-knowledge), the client resets the stream with RST_STREAM.c.req.raw.signalnever fires on that reset.The close handler in
makeCloseHandleraborts the request whenoutgoing.writableFinishedis false. On HTTP/2 that check doesn't work: once the client resets the stream,Http2ServerResponsereportswritableFinished: trueeven though the response was never sent. Logged inside thecloselistener after a clientRST_STREAM(CANCEL):The handler therefore treated the reset as a normal finish and skipped the abort.
The fix also aborts when the request is an
Http2ServerRequestwithabortedset. Node only setsabortedwhen the stream is cut off before the response's writable side has ended, so a normally completed HTTP/2 response is unaffected. HTTP/1 goes through the same check as before.Tests
Abort requestblock intest/listener.test.ts. Each starts an HTTP/2 server throughgetRequestListener, sends a request to a handler that never resolves, then either resets the stream (NGHTTP2_CANCEL) or destroys the session. Both assert that the request signal aborts.main("abort signal did not fire") and passes with this change. The session case passes on both, and stays in as a regression guard.listener.test.tsalso passes on Node 20.lint,format,typecheckandbuildare clean.server.test.ts > should serve on ipv4failed. That test uses the fixed port 3000. It passes on its own, and the next two full runs passed, so the failure was a local port collision.🤖 Generated with Claude Code