Uh oh!
There was an error while loading. Please reload this page.
stream: skip pipeline callback on sync throw - #65128
Conversation
nodejs-github-bot
commented
Aug 8, 2026
Review requested:
|
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
369726d to
358beaaComparelazerg
commented
Aug 8, 2026
The x86_64-darwin shared-libraries job failed on a test-tick-processor-arguments timeout. That test is unrelated to this change, which only touches stream code. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #65128 +/- ##
==========================================
- Coverage 90.31% 90.30% -0.01%
==========================================
Files 759 759 Lines 248290 248297 +7 Branches 46859 46854 -5 ==========================================
- Hits 224241 224230 -11 - Misses 15472 15516 +44 + Partials 8577 8551 -26
🚀 New features to boost your workflow:
|
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
lazerg
commented
Aug 22, 2026
Thanks for the careful trace and repro, this is a real gap and I checked it myself against unpatched main (v26.7.0, That said, I don't think this PR should close that gap. This exact approach (destroy already-wired stages in the catch block, the same shape #65165 used) was tried before on #65064. Its first version did that, CI failed at line 890 on that assertion, @ronag flagged the ownership problem in review, and the PR got narrowed to disposing only the On the #65165 overlap: @pacocartones already reconciled it on 2026-08-12. The split is: this PR (#65128) fixes #65127, the double-report bug, with the |
When
pipelineImpl()throws while wiring the streams together, the stages it already wired stay live. As they close they callfinish(),finishCountdrains to zero andfinishImpl()runs withfinalset, so the callback is scheduled witherrorstill undefined. The caller ends up with the same failure reported twice, once as the exception and once as a successful completion.The callback is now only scheduled if the wiring loop actually finished. An error that arrives on an already-wired stream after the throw is no longer forwarded to the callback either, which is intentional and matches what already happens when the throw comes before anything was wired. The streams themselves are still left untouched, since ownership is not taken until
pipeline()succeeds.Fixes: #65127