stream: fix async iteration of undefined chunks - #65969
Open
everett1992 wants to merge 1 commit into
Open
Conversation
Collaborator
|
Review requested:
|
everett1992
marked this pull request as draft
September 10, 2026 22:48
everett1992
force-pushed
the
stream-undefined-chunk-async-iterator
branch
3 times, most recently
from
September 10, 2026 23:06
ccff441 to
4ce986f
Compare
The hand-rolled async iterator introduced in nodejs#64447 unwraps thenable chunks before delivering them, but guards the `then` access with `chunk !== null` while the dereference itself requires `chunk != null`. An object mode stream carrying an `undefined` chunk therefore threw TypeError: Cannot read properties of undefined (reading 'then') Because the throw happens in a microtask rather than rejecting the iterator's promise, it surfaces as an uncaught exception that terminates the process instead of a catchable stream error. `undefined` is a legal chunk value: doc/api/stream.md documents object mode chunks as "any JavaScript value other than `null`" and reserves `null` as the end-of-stream sentinel. `readable.push(undefined)` returns true in object mode, so the stream accepts the value and then crashes on the way out. Read `then` with optional chaining at both sites so that `undefined` is delivered as a value while `null` keeps signalling end-of-stream. `then` is still read at most once, so a getter cannot observe a second access. Refs: nodejs#64447 Assisted-by: a closed-source coding agent Signed-off-by: Caleb ツ Everett <calebev@amazon.com>
everett1992
force-pushed
the
stream-undefined-chunk-async-iterator
branch
from
September 10, 2026 23:14
4ce986f to
b970c5d
Compare
everett1992
marked this pull request as ready for review
September 11, 2026 00:05
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65969 +/- ##
==========================================
- Coverage 90.18% 90.18% -0.01%
==========================================
Files 771 771
Lines 265444 265472 +28
Branches 50445 50466 +21
==========================================
+ Hits 239401 239421 +20
- Misses 16983 17002 +19
+ Partials 9060 9049 -11
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Readable.prototype[Symbol.asyncIterator]throws an uncaughtTypeErrorwhen an object mode stream carries anundefinedchunk. This is a regression introduced by #64447, which replaced the async generator backing the iterator with a hand-rolled one.Minimal repro
This is not specific to
.map()/.toArray()— anyfor awaitover a stream containing anundefinedchunk crashes..map(() => undefined)is just an easy way to produce one.Cause
The new iterator unwraps thenable chunks before delivery, but the null-check guarding the
thenaccess is too narrow. There are two affected sites, both inlib/internal/streams/readable.js:pump()(line 1526 onmain)next()(line 1599 onmain)Both read:
The guard is
chunk !== null, but the dereference requireschunk != null. Whenstream.read()returnsundefinedthe guard passes andchunk.thenthrows.Because the throw happens in a microtask rather than rejecting the iterator's promise, it surfaces as an uncaught exception that terminates the process instead of a catchable stream error.
Why
undefinedis legaldoc/api/stream.mdstates in three places that an object mode chunk "can be any JavaScript value other thannull", and thatreadable.push()accepts "any JavaScript value".nullis reserved as the end-of-stream sentinel;undefinedis a documented valid value.readable.push(undefined)returnstruein object mode, so the stream accepts the value and then crashes on the way out.Readable.prototype.read()signals "no data" with exactlynull, neverundefined(if (n > 0) ret = fromList(n, state); else ret = null;), soundefinedcoming out ofread()is unambiguously a real chunk rather than an "empty" signal.Fix
Read
thenwith optional chaining at both sites, soundefinedis delivered as a value whilenullkeeps signalling end-of-stream:thenis still read at most once, so a getter cannot observe (or throw on) a second access — the property #64447 deliberately introduced is preserved.Tests
Adds three cases to
test/parallel/test-stream-readable-async-iterators.js:undefinedchunk already buffered before iteration, exercising the synchronous fast path innext()undefinedchunk pushed afternext(), exercising thepump()pathReadable.from([undefined])The first two cover the two distinct code sites, confirmed by stack traces on v26.8.2 (
Object.nextatreadable:1580andpumpatreadable:1507). All three fail before this change and pass after it.Refs: #64447