fs: make ReadStream throw TypeError on NaN #19775
Conversation
Make ReadStream (and thus createReadStream) throw a TypeError signalling towards an invalid argument type when either options.start or options.end (or obviously, both) are set to NaN. Also add regression tests for the same. Fixes: nodejs#19715
nodejs-github-bot
added
the
fs
addaleax
added
lts-watch-v6.x
author ready
|
The failure seems unrelated. |
|
@addaleax is this a flaky test? I think it seems completely unrelated. |
|
@ryzokuken yes it is unrelated. |
|
@lpinca if that's the case, please land this whenever you deem fit. Thanks. |
|
LGTM. Thank you for being patient. Good bug fix. |
|
shows that build was successful. Why does GitHub still show it as "pending" though? |
|
Can we land this now? |
|
@ryzokuken We have a policy that we typically wait 48 hours for PRs to land, and 72 hours over weekends, so that everybody who wants to has a chance to look at it. |
|
Oh, sorry. I already know about the policy. I thought that because this is just a continuation of #19732, it didn't need to be up for 48 hours by itself. Cool, let's wait for a day or two then. |
|
@ryzokuken I don’t think we need to hurry on this Also, this is adding an error throw with the intention of landing it on LTS releases, which is something that people might feel uncomfortable with (I don’t in this case) |
|
@addaleax I understand. Thanks for being patient with me 😅 |
|
Re-running sole CI failed task node-test-commit-plinux: https://ci.nodejs.org/job/node-test-commit-plinux/16650/ |
|
Landed in 38a6929. |
Make ReadStream (and thus createReadStream) throw a TypeError signalling towards an invalid argument type when either options.start or options.end (or obviously, both) are set to NaN. Also add regression tests for the same. PR-URL: #19775 Fixes: #19715 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
targos
removed
the
author ready
Make ReadStream (and thus createReadStream) throw a TypeError signalling towards an invalid argument type when either options.start or options.end (or obviously, both) are set to NaN. Also add regression tests for the same. PR-URL: nodejs#19775 Fixes: nodejs#19715 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Make ReadStream (and thus createReadStream) throw a TypeError signalling
towards an invalid argument type when either options.start or
options.end (or obviously, both) are set to NaN.
Also add regression tests for the same.
Fixes: #19715
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes/cc @lpinca @BridgeAR @addaleax @anliting