Skip to content

module: report unreadable package.json - #65223

Open
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:module/unreadable-package-json
Open

module: report unreadable package.json#65223
bitpshr wants to merge 1 commit into
nodejs:mainfrom
bitpshr:module/unreadable-package-json

Conversation

@bitpshr

@bitpshr bitpshr commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #65220. A package.json that exists but cannot be read was treated as absent, so exports and type silently disappeared and a specifier could resolve to a different file than the package declares. An unparsable package.json already throws ERR_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.json in an ancestor directory. Happy to change course on either.

Fixes: #65220

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 11, 2026
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>
@guybedford

Copy link
Copy Markdown
Contributor

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

codecov Bot commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (4a5eb1c) to head (e3bd440).
⚠️ Report is 268 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65223      +/-   ##
==========================================
+ Coverage   90.15%   90.32%   +0.16%     
==========================================
  Files         744      760      +16     
  Lines      242517   248608    +6091     
  Branches    45688    46934    +1246     
==========================================
+ Hits       218642   224553    +5911     
- Misses      15358    15471     +113     
- Partials     8517     8584      +67     
Files with missing lines Coverage Δ
src/node_modules.cc 80.55% <100.00%> (+0.32%) ⬆️

... and 204 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bitpshr

bitpshr commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Checked the commit before the resolver moved to C++ (f13dbfd, parent 4ec085b). The JS read() there treated an undefined return from internalModuleReadJSON() as "does not exist" with no throw, and only a parse failure threw ERR_INVALID_PACKAGE_CONFIG. InternalModuleReadJSON() in node_file.cc bailed with a bare return for any uv_fs_open() failure, so EACCES came back as undefined just like ENOENT.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESM resolver treats an unreadable package.json as absent: exports silently vanish and the wrong file can load

3 participants