New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
streams: add null check in Readable.from #32873
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test needed
ping @addaleax @himself65 |
@rexagod This LGTM but it looks like this needs to be rebased against |
lib/internal/streams/from.js
Outdated
} else { | ||
reading = false; | ||
const res = await value; | ||
if (res == null) throw new ERR_STREAM_NULL_VALUES(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree with this throwing. There is no way to catch this? I think it should do readable.destroy(new ERR_STREAM_NULL_VALUES())
. @lpinca Thoughts?
@nodejs/streams |
I would like @lpinca's feedback here on whether we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@rexagod Looks like a merge commit somehow got in here, can you rebase this against |
Throws `ERR_STREAM_NULL_VALUES` error if a null value is passed to `Readable.from`. Also added docs for the same. Fixes: nodejs#32845 resolve recieved value and add test Update test/parallel/test-stream-readable-next-no-null.js Co-Authored-By: 扩散性百万甜面包 <himself65@outlook.com> rebase fix fixup fixup: destroy -> throw
Throws `ERR_STREAM_NULL_VALUES` error if a null value is passed to `Readable.from`. Also added docs for the same. Co-Authored-By: 扩散性百万甜面包 <himself65@outlook.com> Fixes: #32845 PR-URL: #32873 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Landed in 2cd7970 |
Throws `ERR_STREAM_NULL_VALUES` error if a null value is passed to `Readable.from`. Also added docs for the same. Co-Authored-By: 扩散性百万甜面包 <himself65@outlook.com> Fixes: #32845 PR-URL: #32873 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Throws `ERR_STREAM_NULL_VALUES` error if a null value is passed to `Readable.from`. Also added docs for the same. Co-Authored-By: 扩散性百万甜面包 <himself65@outlook.com> Fixes: #32845 PR-URL: #32873 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Throws `ERR_STREAM_NULL_VALUES` error if a null value is passed to `Readable.from`. Also added docs for the same. Co-Authored-By: 扩散性百万甜面包 <himself65@outlook.com> Fixes: #32845 PR-URL: #32873 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Throws
ERR_STREAM_NULL_VALUES
error if a null value is passed toReadable.from
. Also added docs for the same.Fixes: #32845
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes