Skip to content

Commit 3e07400

Browse files
moeghassiCopilot
andcommitted
module: test package config error propagation
Cover pending exceptions in package JSON traversal and scope lookups, including failed-read recovery, and update the loader error expectation. Apply the C++ formatting requested by CI. Signed-off-by: Moe Ghasemi <moeghasemi@microsoft.com> Assisted-by: GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39bc512f-a84d-4c8b-a2f8-a2f25e9ee33f
1 parent 9ded778 commit 3e07400

3 files changed

Lines changed: 81 additions & 5 deletions

File tree

‎src/node_modules.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,9 +264,9 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
264264
// these failures, since read access may be restored later.
265265
if (read_error != UV_ENOENT && read_error != UV_ENOTDIR) {
266266
THROW_ERR_ACCESS_DENIED(realm->isolate(),
267-
"Cannot read package config %s: %s",
268-
path.data(),
269-
uv_strerror(read_error));
267+
"Cannot read package config %s: %s",
268+
path.data(),
269+
uv_strerror(read_error));
270270
return nullptr;
271271
}
272272

‎test/parallel/test-module-loading-error.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ assert.throws(
9090
assert.throws(
9191
() => { require('../fixtures/packages/is-dir'); },
9292
common.isAIX ? { code: 'ERR_INVALID_PACKAGE_CONFIG' } : {
93-
code: 'MODULE_NOT_FOUND',
94-
message: /Cannot find module '\.\.\/fixtures\/packages\/is-dir'/
93+
code: 'ERR_ACCESS_DENIED',
94+
message: /Cannot read package config .*package\.json: /
9595
}
9696
);
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
'use strict';
2+
// Flags: --expose-internals
3+
4+
require('../common');
5+
const assert = require('node:assert');
6+
const fs = require('node:fs');
7+
const path = require('node:path');
8+
const { pathToFileURL } = require('node:url');
9+
const tmpdir = require('../common/tmpdir');
10+
const {
11+
getNearestParentPackageJSON,
12+
getRootPackageJSON,
13+
getPackageScopeConfig,
14+
getPackageType,
15+
} = require('internal/modules/package_json_reader');
16+
17+
tmpdir.refresh();
18+
19+
const lookups = {
20+
nearestParent: (file) => getNearestParentPackageJSON(file).data.type,
21+
root: (file) => getRootPackageJSON(file).data.type,
22+
scope: (file) => getPackageScopeConfig(pathToFileURL(file)).type,
23+
type: (file) => getPackageType(pathToFileURL(file)),
24+
};
25+
26+
for (const [name, lookup] of Object.entries(lookups)) {
27+
for (const failure of ['invalid', 'directory']) {
28+
const outer = tmpdir.resolve(`${name}-${failure}`);
29+
const parent = name === 'root' ?
30+
path.join(outer, 'node_modules', 'outer') : outer;
31+
const inner = name === 'root' ?
32+
path.join(parent, 'node_modules', 'inner') : path.join(parent, 'inner');
33+
const manifest = path.join(inner, 'package.json');
34+
const parentManifest = path.join(parent, 'package.json');
35+
const file = path.join(inner, 'lib', 'index.js');
36+
fs.mkdirSync(path.dirname(file), { recursive: true });
37+
38+
// Continuing past the first error would replace it with this parent's error.
39+
fs.writeFileSync(parentManifest, '{');
40+
if (failure === 'invalid') {
41+
fs.writeFileSync(manifest, '{');
42+
} else {
43+
fs.mkdirSync(manifest);
44+
45+
// Some platforms allow reading a directory.
46+
let directoryReadError;
47+
try {
48+
fs.readFileSync(manifest);
49+
} catch (err) {
50+
directoryReadError = err;
51+
}
52+
if (!directoryReadError) {
53+
continue;
54+
}
55+
assert.strictEqual(directoryReadError.code, 'EISDIR');
56+
}
57+
58+
const expectedError = {
59+
code: failure === 'invalid' ?
60+
'ERR_INVALID_PACKAGE_CONFIG' : 'ERR_ACCESS_DENIED',
61+
message: new RegExp(RegExp.escape(path.toNamespacedPath(manifest))),
62+
};
63+
assert.throws(() => lookup(file), expectedError);
64+
65+
// A failed lookup must not cache the manifest as missing.
66+
assert.throws(() => lookup(file), expectedError);
67+
68+
if (failure === 'directory') {
69+
fs.rmdirSync(manifest);
70+
}
71+
const config = JSON.stringify({ type: 'module', private: true });
72+
fs.writeFileSync(manifest, config);
73+
fs.writeFileSync(parentManifest, config);
74+
assert.strictEqual(lookup(file), 'module');
75+
}
76+
}

0 commit comments

Comments
 (0)