Skip to content

quic: add stream.cancel() sending H3_REQUEST_CANCELLED - #66007

Open
trivenay wants to merge 1 commit into
nodejs:mainfrom
trivenay:quic-stream-cancel
Open

quic: add stream.cancel() sending H3_REQUEST_CANCELLED#66007
trivenay wants to merge 1 commit into
nodejs:mainfrom
trivenay:quic-stream-cancel

Conversation

@trivenay

Copy link
Copy Markdown
Contributor

Follow-up to #65442. I opened #65509 to finish RFC 9114 compliance for the stream closure error codes, and this implements the cancellation half.

There is currently no way to signal that a request or response was deliberately abandoned: resetStream()/stopSending() default to the no-error code and destroy(error) sends the internal error code. Per RFC 9114 section 4.1.1, a cancelled request should be aborted with H3_REQUEST_CANCELLED.

This adds stream.cancel([reason]), which terminates both directions of the stream using the cancellation code defined by the negotiated application protocol -- H3_REQUEST_CANCELLED (0x10c) for HTTP/3, the no-error code for other applications. The code value is plumbed from the application layer through the session state, same pattern as the request-rejected code in #65442. The existing reset primitives are unchanged.

The main alternative considered in #65509 was exposing the cancellation code as a named constant and letting callers pass it to the existing resetStream()/stopSending(). That keeps the API surface smaller, but cancelling is a single intent that requires aborting both halves of the stream -- a RESET_STREAM and a STOP_SENDING, each carrying the right code for the negotiated protocol. Making every caller do that two-call dance and pick the code is exactly the part that seems worth encapsulating, the same way #65442 picks the rejected code internally. I'm using this PR as a vehicle for that discussion -- happy to change the approach if a different shape is preferred.

The other item from #65509, H3_REQUEST_INCOMPLETE, is still pending: nghttp3 defines the constant but exposes no hook for detecting an incomplete message, so emitting it would need message-completeness tracking on our side. That's considerably more invasive, and honestly I'm not yet convinced the value justifies the complexity -- left out of this PR.

Refs: #65509

When an endpoint deliberately abandons a request or response, RFC 9114
section 4.1.1 says the stream should be aborted with the error code
H3_REQUEST_CANCELLED. There is currently no way to express this: the
reset primitives default to the no-error code and destroy(error) uses
the internal error code.

Add stream.cancel([reason]), which abruptly terminates both directions
of the stream using the cancellation code defined by the negotiated
application protocol (H3_REQUEST_CANCELLED for HTTP/3; the no-error
code for other applications). The code value is plumbed from the
application layer through the session state, following the same
pattern as the request-rejected code.

Refs: nodejs#65509
Signed-off-by: Naman Trivedi <trivenay@amazon.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 13, 2026
@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.99%. Comparing base (666edfd) to head (5a420b1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66007   +/-   ##
=======================================
  Coverage   89.98%   89.99%           
=======================================
  Files         785      785           
  Lines      269282   269308   +26     
  Branches    51296    51292    -4     
=======================================
+ Hits       242323   242357   +34     
+ Misses      17464    17449   -15     
- Partials     9495     9502    +7     
Files with missing lines Coverage Δ
lib/internal/quic/quic.js 100.00% <100.00%> (ø)
lib/internal/quic/state.js 100.00% <100.00%> (ø)

... and 34 files with indirect coverage changes

🚀 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

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants