Skip to content

Pin error() during a pending close in the readable stream suites - #7335

Open
jasnell wants to merge 1 commit into
mainfrom
jasnell/streams-pin-error-pending-close
Open

Pin error() during a pending close in the readable stream suites#7335
jasnell wants to merge 1 commit into
mainfrom
jasnell/streams-pin-error-pending-close

Conversation

@jasnell

@jasnell jasnell commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

After close() with a chunk still queued the stream has only requested its close and, per spec, is still readable, so a later error() errors it and discards the queue; the TypeScript implementation does that, while the C++ implementation treats the requested close as final, ignores the late error and drains the chunk to a clean close (and, for byte streams, already reports desiredSize as 0 rather than the high-water mark minus the queued bytes). Both suites pinned error() after a completed close only; pin the pending-close ordering on each side and record it in the ledgers.

After close() with a chunk still queued the stream has only requested
its close and, per spec, is still readable, so a later error() errors it
and discards the queue; the TypeScript implementation does that, while
the C++ implementation treats the requested close as final, ignores the
late error and drains the chunk to a clean close (and, for byte streams,
already reports desiredSize as 0 rather than the high-water mark minus
the queued bytes). Both suites pinned error() after a completed close
only; pin the pending-close ordering on each side and record it in the
ledgers.
@jasnell
jasnell requested review from guybedford and npaun September 12, 2026 23:49
@jasnell
jasnell requested review from a team as code owners September 12, 2026 23:49
@jasnell
jasnell added this pull request to stack #7337 September 12, 2026 23:51
@ask-bonk

ask-bonk Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.25%. Comparing base (925464b) to head (e56ff79).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7335      +/-   ##
==========================================
- Coverage   37.35%   37.25%   -0.11%     
==========================================
  Files         806      810       +4     
  Lines      253047   253975     +928     
  Branches    20087    20103      +16     
==========================================
+ Hits        94526    94608      +82     
- Misses     147114   147952     +838     
- Partials    11407    11415       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants