quic: apply multiple fixes to flow control signaling - #65309
Conversation
|
Review requested:
|
This comment was marked as outdated.
This comment was marked as outdated.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65309 +/- ##
==========================================
- Coverage 90.05% 90.05% -0.01%
==========================================
Files 751 751
Lines 254868 254906 +38
Branches 48107 48115 +8
==========================================
+ Hits 229531 229543 +12
- Misses 16511 16528 +17
- Partials 8826 8835 +9
🚀 New features to boost your workflow:
|
|
Great addition to find more stalls in the pipeline. |
|
@pimterry @Ethan-Arrowood @nodejs/quic ... really need to get this landed before we muck around with more of the quic impl |
242734c to
d889ee8
Compare
pimterry
left a comment
There was a problem hiding this comment.
Lots of small points where I think we could be stricter and cover related cases, but nothing major. Happy to merge as-is or extend for the various notes here, up to you.
Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode/Opus
d889ee8 to
3bdafb6
Compare
Ethan-Arrowood
left a comment
There was a problem hiding this comment.
All LGTM. Tests coverage is great. Had my agent help me review the rest of the implementation and it didn't find anything either. One minor nit that can honestly be ignored unless you're doing another commit anyways.
Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode/Opus PR-URL: #65309 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
|
Landed in 05a8e91 |
Fixing up the QUIC flow control signaling and closed stream handling, expand tests.
Update the flow control handling after the iter streams byte-budget based flow control updates.
@nodejs/quic @pimterry @martenrichter