Skip to content

quic: fix h3 session close on early stream destroy - #67446

Open
Ethan-Arrowood wants to merge 1 commit into
nodejs:mainfrom
Ethan-Arrowood:quic-h3-reset-after-headers
Open

Ethan-Arrowood wants to merge 1 commit into
nodejs:mainfrom
Ethan-Arrowood:quic-h3-reset-after-headers

Conversation

@Ethan-Arrowood

Copy link
Copy Markdown
Contributor

If the stream is destroyed right after sendHeaders(), nghttp3 still has the response headers queued. On the next write it asks for the removed stream's body, the read callback fails, and the whole session closes without notifying the peer.

In an HTTP/3 session, before clearing nghttp3's pointer to a stream when the stream is destroyed, shut down nghttp3's writes for that stream so that other requests can continue to complete.

This PR adds regression tests to cover the issue.

Discovered this while reviewing #66007, which adds stream.cancel(). That PR's cancel-after-headers case depends on this fix. Calling destroy() without a code still sends no reset, which is out of scope here (continue discussion in #66007).

AI-assisted (Claude Opus 5.5, my setup). The words are my own, per the Human Voice policy.

If the stream is destroyed right after sendHeaders(), nghttp3 still has
the response headers queued. On the next write it asks for the removed
stream's body, the read callback fails, and the whole session closes
without notifying the peer.

In an HTTP/3 session, before clearing nghttp3's pointer to a stream when
the stream is destroyed, shut down nghttp3's writes for that stream so
that other requests can continue to complete.

Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Ethan Arrowood <ethan@arrowood.dev>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC transport implementation. labels Nov 12, 2026
@Ethan-Arrowood Ethan-Arrowood moved this to Ready for Review in QUIC Team Nov 12, 2026
@codecov

codecov Bot commented Nov 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.68%. Comparing base (6728a1c) to head (d51bf3e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67446      +/-   ##
==========================================
- Coverage   90.68%   90.68%   -0.01%     
==========================================
  Files         801      801              
  Lines      281889   281889              
  Branches    54318    54319       +1     
==========================================
- Hits       255645   255641       -4     
- Misses      16358    16360       +2     
- Partials     9886     9888       +2     

see 20 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++. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC transport implementation.

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

2 participants