module: report unreadable package.json - #65223
Conversation
b4271b7 to
e3bd440
Compare
|
It could be worth checking what error used to be thrown here before the resolver was refactored into C++, as I'm pretty sure this did used to throw an error, it just clearly didn't have coverage. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65223 +/- ##
==========================================
+ Coverage 90.15% 90.32% +0.17%
==========================================
Files 744 751 +7
Lines 242517 249656 +7139
Branches 45688 47178 +1490
==========================================
+ Hits 218642 225505 +6863
- Misses 15358 15535 +177
- Partials 8517 8616 +99
🚀 New features to boost your workflow:
|
|
Checked the commit before the resolver moved to C++ (f13dbfd, parent 4ec085b). The JS So the silent fallback looks like it predates the C++ move rather than being a regression from it, which fits there never being coverage. Happy to be wrong if you remember an earlier version that threw, I only went back as far as the refactor. |
A package.json that exists but cannot be read was treated the same as one that is not there: the read failure returned no config and resolution continued as if the package had none. Fields such as "exports" and "type" silently disappear, so a specifier can resolve to a different file than the package declares, while an unparsable package.json already throws ERR_INVALID_PACKAGE_CONFIG. Keep treating ENOENT and ENOTDIR as "no package config here", and report any other read failure with the underlying error. Fixes: nodejs#65220 Signed-off-by: Paul Bouchon <mail@bitpshr.net>
e3bd440 to
3957a8d
Compare
|
Applied both, thanks. The status assertions come last now, and the final |
|
Landed in c4cfa63 |
A package.json that exists but cannot be read was treated the same as one that is not there: the read failure returned no config and resolution continued as if the package had none. Fields such as "exports" and "type" silently disappear, so a specifier can resolve to a different file than the package declares, while an unparsable package.json already throws ERR_INVALID_PACKAGE_CONFIG. Keep treating ENOENT and ENOTDIR as "no package config here", and report any other read failure with the underlying error. Fixes: #65220 Signed-off-by: Paul Bouchon <mail@bitpshr.net> PR-URL: #65223 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
A package.json that exists but cannot be read was treated the same as one that is not there: the read failure returned no config and resolution continued as if the package had none. Fields such as "exports" and "type" silently disappear, so a specifier can resolve to a different file than the package declares, while an unparsable package.json already throws ERR_INVALID_PACKAGE_CONFIG. Keep treating ENOENT and ENOTDIR as "no package config here", and report any other read failure with the underlying error. Fixes: #65220 Signed-off-by: Paul Bouchon <mail@bitpshr.net> PR-URL: #65223 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
A package.json that exists but cannot be read was treated the same as one that is not there: the read failure returned no config and resolution continued as if the package had none. Fields such as "exports" and "type" silently disappear, so a specifier can resolve to a different file than the package declares, while an unparsable package.json already throws ERR_INVALID_PACKAGE_CONFIG. Keep treating ENOENT and ENOTDIR as "no package config here", and report any other read failure with the underlying error. Fixes: #65220 Signed-off-by: Paul Bouchon <mail@bitpshr.net> PR-URL: #65223 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
A package.json that exists but cannot be read was treated the same as one that is not there: the read failure returned no config and resolution continued as if the package had none. Fields such as "exports" and "type" silently disappear, so a specifier can resolve to a different file than the package declares, while an unparsable package.json already throws ERR_INVALID_PACKAGE_CONFIG. Keep treating ENOENT and ENOTDIR as "no package config here", and report any other read failure with the underlying error. Fixes: #65220 Signed-off-by: Paul Bouchon <mail@bitpshr.net> PR-URL: #65223 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
A package.json that exists but cannot be read was treated the same as one that is not there: the read failure returned no config and resolution continued as if the package had none. Fields such as "exports" and "type" silently disappear, so a specifier can resolve to a different file than the package declares, while an unparsable package.json already throws ERR_INVALID_PACKAGE_CONFIG. Keep treating ENOENT and ENOTDIR as "no package config here", and report any other read failure with the underlying error. Fixes: #65220 Signed-off-by: Paul Bouchon <mail@bitpshr.net> PR-URL: #65223 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com>
Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com>
Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com>
Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove upstreamed patch * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
* chore: bump node in DEPS to v24.21.0 * fix(patch): module: report unreadable package.json Upstream now keeps the ReadFileSync error code and throws ERR_INVALID_PACKAGE_CONFIG for anything other than ENOENT/ENOTDIR/EISDIR. Route the ASAR override through the same read_error path and report a missing archive entry as UV_ENOENT so it is still negatively cached rather than surfaced as a read failure. Ref: nodejs/node#65223 Co-Authored-By: Claude <noreply@anthropic.com> * chore: remove upstreamed patch * chore: update patches (trivial only) * fix(patch): skip OpenSSL default-config test with BoringSSL Ref: nodejs/node#64949 Co-Authored-By: Claude <noreply@anthropic.com> * fix(patch): permission: enforce addon permission in GetLinkedBinding Upstream now throws ERR_ACCESS_DENIED from process._linkedBinding() when the permission model is enabled without --allow-addons. Electron's own bootstrap reaches all of its built-in modules through linked bindings, so `electron --permission` under ELECTRON_RUN_AS_NODE could no longer initialise Electron's Node.js integration. Add a patch exempting the electron_* bindings from the addon check; process.dlopen() and any other linked-binding name keep the upstream behaviour. Ref: nodejs/node#65432 Co-Authored-By: Claude <noreply@anthropic.com> * chore: update filenames.auto.gni --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Sam Attard <sattard@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
Fixes #65220. A
package.jsonthat exists but cannot be read was treated as absent, soexportsandtypesilently disappeared and a specifier could resolve to a different file than the package declares. An unparsablepackage.jsonalready throwsERR_INVALID_PACKAGE_CONFIG, so this makes the unreadable case consistent: ENOENT and ENOTDIR still mean "no package config here", anything else reports the underlying error.Two alternatives if you'd prefer them: warn and keep resolving, or restrict the new error to the direct package lookup and stay silent while walking up for the nearest parent config. That second one matters if anyone relies on resolution succeeding past an unreadable
package.jsonin an ancestor directory. Happy to change course on either.Fixes: #65220