Skip to content

quic: extract transport logic from Application to Session#64127

Merged
nodejs-github-bot merged 5 commits into
nodejs:mainfrom
pimterry:extract-h3-refactor
Jul 8, 2026
Merged

quic: extract transport logic from Application to Session#64127
nodejs-github-bot merged 5 commits into
nodejs:mainfrom
pimterry:extract-h3-refactor

Conversation

@pimterry

Copy link
Copy Markdown
Member

Extracting out some standalone refactoring from #63995 for easier review while we keep debating the API. I think this is a helpful refactoring generally, but especially in a world where we're restructuring Application.

This shrinks the application layer by moving generic QUIC logic that will be used by all implementations into the Session itself, and makes the Session act as a clean interface over ngtcp2 - so that the Application exclusively uses Session methods for all connection manipulation, instead of managing ngtcp2 directly. This gives a cleaner split between the low-level QUIC behaviour and the per-application-protocol layer.

There's two small fixes notably included in the migration, contained within the moved code:

  • This fixes a SendPendingData bug where datagrams would stall and never send on an idle connection, in the nwrite == 0 branch.
  • We now check is_destroyed() after StreamCommit, since it calls JS callbacks which could destroy the session.

@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 related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 25, 2026
@pimterry pimterry force-pushed the extract-h3-refactor branch from 19197f6 to 5fdb7e8 Compare June 25, 2026 12:30
Comment thread src/quic/session.h Outdated
Comment thread src/quic/session.h Outdated
Comment thread src/quic/session.h Outdated
Comment thread src/quic/session.h Outdated
Comment thread src/quic/session.h Outdated
Comment thread src/quic/session.h Outdated
Comment thread src/quic/session.cc
pimterry added 4 commits July 7, 2026 17:49
One small fix notably included:

- Check is_destroyed() after StreamCommit, since it calls JS callbacks
  which could destroy the session.

Signed-off-by: Tim Perry <pimterry@gmail.com>
Fix preexisting bug in TryWritePendingDatagram
@pimterry pimterry force-pushed the extract-h3-refactor branch from 5fdb7e8 to 157417a Compare July 7, 2026 16:05
@pimterry

pimterry commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

@jasnell Most comments handled here, and rebased onto main which had a conflict from #64303..

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.25%. Comparing base (f91697a) to head (fa84d48).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64127      +/-   ##
==========================================
- Coverage   92.04%   90.25%   -1.80%     
==========================================
  Files         381      741     +360     
  Lines      169461   240976   +71515     
  Branches    25975    45393   +19418     
==========================================
+ Hits       155983   217488   +61505     
- Misses      13188    15069    +1881     
- Partials      290     8419    +8129     

see 487 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.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

nodejs-github-bot commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

@jasnell jasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2026
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 8, 2026
@nodejs-github-bot nodejs-github-bot merged commit 0792ee5 into nodejs:main Jul 8, 2026
78 of 79 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 0792ee5

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 related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants