fix(stream): wait for both finished sides - #9906
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughCallback-form ChangesDuplex stream completion
Estimated code review effort: 3 (Moderate) | ~15 minutes Merge Risk: ⚪ Minimal · up to Callback-form stream.finished() now completes after both sides of a duplex stream finish, preventing premature callbacks for unread PassThrough streams. The implementation and regression coverage align with the intended behavior, with no current merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…he new roots Two files went over 2000 lines: - regex.rs -> regex/compile_cache.rs takes the program-compilation and cache group (size limit, std/fancy builders, eviction, the checked compile-and-cache entry). `js_regexp_new_impl` was the obvious bigger extraction and is deliberately NOT the one taken: it carries the two #7341 raw-handle debt sites, and raw_handle_debt.py's --no-raise-vs arm refuses a ceiling on a file absent at the merge base, so moving it would have forced either surgery on the allocation path two perf PRs are tuning, or a with_const_ptr(|p| p) that games the ratchet without scoping anything. - regex/tests.rs -> regex/tests_part2.rs, split at a test boundary. All 70 #[test] items are accounted for across the two files, and part2 carries the same cfg(all(test, feature)) gate as its sibling. #9893's new roots are classified: REGEXP_PROTOTYPE_PTR_SLOT and REGEXP_PROTOTYPE_TEST_CLOSURE_SLOT are covered_elsewhere, naming scan_object_cache_roots_mut, which really does visit them (as an i64 slot and a nanbox word respectively) and is registered via reg_scanner!. The test index and the walk counter are not_a_gc_pointer. NEVER_MATCH moved with the compile-cache group, so its inventory entry is retargeted. #9906's new test thread_local is recorded cold — it is a test file, and the other 23 there are recorded the same way.
|
Landed on |
1 participant
Calling callback-form
stream.finished()on an unreadPassThroughfired as soon as the writable side emittedfinish. Node waits for the readable side too, so the minimal fixture exits without invoking the callback; Perry printederr: true.This gives the default callback form a completion listener that tracks the stream's actual directional terminal events. Readable-only and writable-only streams still complete on their one side, duplex streams wait for both
endandfinish, and existing error/close behavior remains intact. Option-specific paths such as{ readable: false }keep their existing listener.Refs #9202.
Validation:
finished_waits_for_both_passthrough_sidesstream/finished/*: 7/7 parity fixtures passed against Node 26.5.1stream/promises/finished-*: 12/12 parity fixtures passedperry-runtime: 3,243 passed, 4 ignored; doc tests 8 ignored./scripts/run_lint_gates.sh: all 64 gates passed, 2 CI-only skippedSummary by CodeRabbit
stream.finished()so it waits for both readable and writable sides of duplex streams to complete.PassThroughstreams now correctly wait for their readableendevent before invoking the completion callback.