Skip to content

stream: use validateBuffer for BYOB reader view - #65046

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
greenheadHQ:readablestream-validate-buffer
Aug 13, 2026
Merged

stream: use validateBuffer for BYOB reader view#65046
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
greenheadHQ:readablestream-validate-buffer

Conversation

@greenheadHQ

Copy link
Copy Markdown
Contributor

ReadableStreamBYOBReader.prototype.read() open-codes the check that validateBuffer performs, while ReadableStreamBYOBRequest.prototype.respondWithNewView() a few hundred lines above already calls validateBuffer(view, 'view') for the same argument. Both build ERR_INVALID_ARG_TYPE from the same name and type list, so the rejection is unchanged.

No new test: readable-byte-streams/general.any.js already covers read() with a non-view argument.

The same check is already spelled validateBuffer(view, 'view') in
ReadableStreamBYOBRequest.respondWithNewView(), and the thrown error is
unchanged.

Signed-off-by: greenhead <shren0812@gmail.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.29%. Comparing base (3e6cca0) to head (7002d14).
⚠️ Report is 130 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65046      +/-   ##
==========================================
- Coverage   90.30%   90.29%   -0.01%     
==========================================
  Files         759      759              
  Lines      247612   247602      -10     
  Branches    46686    46681       -5     
==========================================
- Hits       223604   223579      -25     
- Misses      15460    15462       +2     
- Partials     8548     8561      +13     
Files with missing lines Coverage Δ
lib/internal/webstreams/readablestream.js 98.16% <100.00%> (-0.01%) ⬇️

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

@greenheadHQ

Copy link
Copy Markdown
Contributor Author

The aarch64-darwin: with shared libraries failure is unrelated to this change. It is parallel/test-stream-pipeline-http2, which is listed as flaky in nodejs/reliability#1622. The same commit passes on every other platform, including x86_64-darwin, and the test does not touch web streams.

@daeyeon daeyeon added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 9, 2026
@daeyeon daeyeon added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 12, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 12, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/65046
✔  Done loading data for nodejs/node/pull/65046
----------------------------------- PR info ------------------------------------
Title      stream: use validateBuffer for BYOB reader view (#65046)
Author     greenhead <shren0812@gmail.com> (@greenheadHQ)
Branch     greenheadHQ:readablestream-validate-buffer -> nodejs:main
Labels     author ready, needs-ci, commit-queue, web streams
Commits    1
 - stream: use validateBuffer for BYOB reader view
Committers 1
 - greenhead <shren0812@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65046
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65046
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 05 Aug 2026 12:51:50 GMT
   ✔  Approvals: 2
   ✔  - Daeyeon Jeong (@daeyeon): https://github.com/nodejs/node/pull/65046#pullrequestreview-4891279202
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/65046#pullrequestreview-4892848785
   ✔  Last GitHub CI successful
   ✘  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/31602140325

@greenheadHQ

greenheadHQ commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@daeyeon It looks like the queue stopped because Jenkins CI hadn’t run yet. When you have a chance, could you help trigger the full CI? Thanks for looking into this! 🫡

@daeyeon daeyeon added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 12, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@daeyeon daeyeon added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 13, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 1d9d49c into nodejs:main Aug 13, 2026
101 of 102 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 1d9d49c

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 13, 2026
aduh95 pushed a commit that referenced this pull request Aug 13, 2026
The same check is already spelled validateBuffer(view, 'view') in
ReadableStreamBYOBRequest.respondWithNewView(), and the thrown error is
unchanged.

Signed-off-by: greenhead <shren0812@gmail.com>
PR-URL: #65046
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. web streams

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants