Skip to content

stream: proxy first own method in Readable.wrap()#64048

Open
watilde wants to merge 1 commit into
nodejs:mainfrom
watilde:stream-fix-wrap-proxy
Open

stream: proxy first own method in Readable.wrap()#64048
watilde wants to merge 1 commit into
nodejs:mainfrom
watilde:stream-fix-wrap-proxy

Conversation

@watilde

@watilde watilde commented Jun 21, 2026

Copy link
Copy Markdown
Member

The method-proxying loop in Readable.prototype.wrap() started at index 1, silently skipping streamKeys[0] and leaving a method at the first own-enumerable key unproxied.

This was introduced in ee9e2a2 when ArrayPrototypeForEach( ObjectKeys(stream), ...) β€” which iterates from index 0 β€” was rewritten as a manual for loop for performance. The faithful translation starts at 0; a sibling loop converted in the same commit correctly does so. The bug stayed hidden because keys[0] is usually _events (a non-function), which the loop's typeof guard skips anyway.

The method-proxying loop in Readable.prototype.wrap() started at
index 1, silently skipping streamKeys[0] and leaving a method at the
first own-enumerable key unproxied.

This was introduced in ee9e2a2 when ArrayPrototypeForEach(
ObjectKeys(stream), ...) β€” which iterates from index 0 β€” was
rewritten as a manual for loop for performance. The faithful
translation starts at 0; a sibling loop converted in the same commit
correctly does so. The bug stayed hidden because keys[0] is usually
_events (a non-function), which the loop's typeof guard skips anyway.

Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
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. labels Jun 21, 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. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants