FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

quic: fixup quic stream variable chunk len by jasnell · Pull Request #63230 · nodejs/node · GitHub

/ node Public

quic: fixup quic stream variable chunk len - #63230

Closed
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/quic-issue-63216
ConversationCommits (1)ChecksFiles changed
Closed

quic: fixup quic stream variable chunk len#63230
jasnell wants to merge 1 commit into
nodejs:mainfrom
jasnell:jasnell/quic-issue-63216

Conversation

jasnell commented May 10, 2026

Copy link
Copy Markdown
Member

Fixes: #63216

jasnell added the quic Issues and PRs related to the QUIC implementation / HTTP/3. label May 10, 2026

nodejs-github-bot commented May 10, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

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. labels May 10, 2026
jasnell mentioned this pull request May 10, 2026
Closed
jasnell changed the title quic: fiup quic stream variable chunk len quic: fixup quic stream variable chunk len May 10, 2026
jasnell force-pushed the jasnell/quic-issue-63216 branch from a560a10 to 4ade24f Compare May 10, 2026 20:38

This comment was marked as outdated.

Sign in to view

codecov Bot commented May 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.03%. Comparing base (facd71e) to head (eafeb01).
⚠️ Report is 39 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #63230      +/-   ##
==========================================
- Coverage   90.04%   90.03%   -0.01%     
==========================================
  Files         713      714       +1     
  Lines      224950   225243     +293     
  Branches    42530    42572      +42     
==========================================
+ Hits       202548   202802     +254     
- Misses      14188    14221      +33     
- Partials     8214     8220       +6
Files with missing lines Coverage Δ
lib/internal/quic/quic.js 100.00% <100.00%> (ø)

... and 44 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.

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view
jasnell requested review from Qard and pimterry May 11, 2026 02:26
pimterry reviewed May 12, 2026
View reviewed changes
Comment thread lib/internal/quic/quic.js Outdated
Comment thread src/quic/streams.cc Outdated
quic: fixup quic stream variable chunk len
eafeb01 
Signed-off-by: James M Snell <jasnell@gmail.com>
jasnell force-pushed the jasnell/quic-issue-63216 branch from 4ade24f to eafeb01 Compare May 13, 2026 03:15
jasnell requested a review from pimterry May 13, 2026 03:22

This comment was marked as outdated.

Sign in to view

nodejs-github-bot commented May 14, 2026 •
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/73430/ 💛

martenrichter mentioned this pull request May 14, 2026
Open
jasnell requested a review from mcollina May 14, 2026 13:38
jasnell added the experimental Issues and PRs related to experimental features. label May 14, 2026
pimterry approved these changes May 15, 2026
View reviewed changes

pimterry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This approach looks much better, very nice 👍

I take it this means we're going towards option 1 from my previous backpressure comment? I.e. stream/iter writables are required to always accept one pending write, even in strict mode, and to deal with any internal byte-based limits & backpressure via internal buffering only. Doesn't need to happen immediately, but once we're settled there it'd be good to write that into the stream/iter writer docs explicitly.

mcollina approved these changes May 15, 2026
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

jasnell commented May 15, 2026

Copy link
Copy Markdown
Member Author

@pimterry ... I think the backpressure strategy for both stream/iter and this as an application of that api still needs a thorough review, which I plan to get to soon.

jasnell added a commit that referenced this pull request May 15, 2026
quic: fixup quic stream variable chunk len
e9c49ea 
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #63230
Fixes: #63216
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

jasnell commented May 15, 2026

Copy link
Copy Markdown
Member Author

Landed in e9c49ea

jasnell closed this May 15, 2026
aduh95 pushed a commit that referenced this pull request May 19, 2026
quic: fixup quic stream variable chunk len
87fe57a 
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #63230
Fixes: #63216
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
aduh95 pushed a commit that referenced this pull request May 19, 2026
quic: fixup quic stream variable chunk len
526313b 
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #63230
Fixes: #63216
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
github-actions Bot mentioned this pull request May 19, 2026
Merged
martenrichter mentioned this pull request Jun 23, 2026
Open
aduh95 mentioned this pull request Jul 22, 2026
Closed
aduh95 pushed a commit to aduh95/node that referenced this pull request Jul 30, 2026
quic: fixup quic stream variable chunk len
bbda997 
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#63230
Fixes: nodejs#63216
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
aduh95 pushed a commit to aduh95/node that referenced this pull request Aug 3, 2026
quic: fixup quic stream variable chunk len
aa5dc94 
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#63230
Fixes: nodejs#63216
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
aduh95 pushed a commit that referenced this pull request Aug 6, 2026
quic: fixup quic stream variable chunk len
575dc7d 
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: #63230
Backport-PR-URL: #64675
Fixes: #63216
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
github-actions Bot mentioned this pull request Aug 21, 2026
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
pimterry pimterry approved these changes
Qard Awaiting requested review from Qard

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. experimental Issues and PRs related to experimental features. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

quic: stream stalls with chunks of varying sizes

4 participants

Back | FazBrowse Home | New Git URL