Skip to content

stream: align Readable.toWeb termination with eos#62394

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

stream: align Readable.toWeb termination with eos#62394
ikeyan wants to merge 8 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

❌ Patch coverage is 99.53052% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (8199f9c) to head (cbe1987).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/end-of-stream.js 99.31% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62394      +/-   ##
==========================================
- Coverage   89.69%   89.64%   -0.06%     
==========================================
  Files         676      674       -2     
  Lines      206693   205684    -1009     
  Branches    39577    39443     -134     
==========================================
- Hits       185402   184384    -1018     
- Misses      13435    13535     +100     
+ Partials     7856     7765      -91     
Files with missing lines Coverage Δ
lib/internal/webstreams/adapters.js 86.43% <100.00%> (-0.02%) ⬇️
lib/internal/streams/end-of-stream.js 98.28% <99.31%> (+0.85%) ⬆️

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