os: add fallback for undefined CPUs - #25493
Conversation
Sorry, something went wrong.
|
@JungMinu sadly an error occured when I tried to trigger a build :( |
Sorry, something went wrong.
addaleax
left a comment
There was a problem hiding this comment.
I’m wondering if it might make sense to add a “real” fallback, i.e. a dummy result that lists 1 CPU or so?
Sorry, something went wrong.
devsnek
left a comment
There was a problem hiding this comment.
I'd rather this threw. It's not accurate to return no cpus or one fake cpu. If this function can't find cpus it shouldn't make something up.
Sorry, something went wrong.
Does this affect any supported platforms? Assuming this only affects unsupported platforms, I'd like to see us introduce the throw, but in a major release. (If it only affects unsupported platforms, I wouldn't consider it semver-major. But doing it in a major release anyway is the kind thing to do, so I'm 👍with that.) The behavior here will be difficult to test robustly unless it applies to a supported platform and configuration that we can test in CI. So removing the bits of code we can't test and that don't affect support platforms/configurations seems like the right thing to do. |
Sorry, something went wrong.
For an unsupported OS, a call to os.cpus() throws an error within os.cpus() itself where it tries to get the length of it. This fixes the issue by adding fallback for undefined CPUs. Fixes: #25483 PR-URL: #25493 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
For an unsupported OS, a call to os.cpus() throws an error within os.cpus() itself where it tries to get the length of it. This fixes the issue by adding fallback for undefined CPUs. Fixes: #25483 PR-URL: #25493 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
For an unsupported OS, a call to os.cpus() throws an error within os.cpus() itself where it tries to get the length of it. This fixes the issue by adding fallback for undefined CPUs. Fixes: nodejs#25483 PR-URL: nodejs#25493 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
For an unsupported OS, a call to os.cpus() throws an error within os.cpus() itself where it tries to get the length of it. This fixes the issue by adding fallback for undefined CPUs. Fixes: nodejs#25483 PR-URL: nodejs#25493 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Currently, for an unsupported OS, a call to os.cpus() throws an error within os.cpus() itself where it tries to get the length of undefined. This PR fixes the issue by adding fallback for undefined CPUs.
Fixes: #25483
Checklist