Skip to content

stream: align Readable.toWeb termination with eos#62394

Open
ikeyan wants to merge 12 commits intonodejs:mainfrom
ikeyan:fix-stream-readable-to-web
Open

stream: align Readable.toWeb termination with eos#62394
ikeyan wants to merge 12 commits intonodejs:mainfrom
ikeyan:fix-stream-readable-to-web

Conversation

@ikeyan
Copy link
Contributor

@ikeyan ikeyan commented Mar 22, 2026

This aligns Readable.toWeb(stream) termination handling with
eos(stream, { writable: false }).

Changes:

  • add regression tests for half-open duplex termination
  • add regression coverage for adapter creation before and after
    end(), destroy(), and destroy(error)
  • preserve terminated Readable adapter state, including closed/error
    state and byte stream type, regardless of when the adapter is created
  • let Readable.toWeb() reuse eos() immediate completion handling
    for already-terminated streams
  • add regression coverage for the synchronous internal EOS callback
    path, including listener cleanup and BYOB termination

Tests:

  • make lint
  • ./configure && make -j4 test

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. web streams labels Mar 22, 2026
@codecov
Copy link

codecov bot commented Mar 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.69%. Comparing base (8199f9c) to head (4922bc4).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62394      +/-   ##
==========================================
- Coverage   89.69%   89.69%   -0.01%     
==========================================
  Files         676      676              
  Lines      206693   206767      +74     
  Branches    39577    39588      +11     
==========================================
+ Hits       185402   185457      +55     
- Misses      13435    13442       +7     
- Partials     7856     7868      +12     
Files with missing lines Coverage Δ
lib/internal/streams/end-of-stream.js 98.52% <100.00%> (+1.10%) ⬆️
lib/internal/webstreams/adapters.js 86.43% <100.00%> (-0.02%) ⬇️

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

@MattiasBuelens MattiasBuelens added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 23, 2026
@ikeyan ikeyan force-pushed the fix-stream-readable-to-web branch from b9b8a96 to a0fd4b7 Compare March 24, 2026 02:11
@ikeyan
Copy link
Contributor Author

ikeyan commented Mar 24, 2026

Nothing to do now. Ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. stream Issues and PRs related to the stream subsystem. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants