Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: only try to enrich CJS syntax errors #35691

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
const {
Boolean,
JSONParse,
ObjectGetPrototypeOf,
ObjectPrototypeHasOwnProperty,
ObjectKeys,
PromisePrototypeCatch,
Expand All @@ -15,6 +16,7 @@ const {
StringPrototypeReplace,
StringPrototypeSplit,
StringPrototypeStartsWith,
SyntaxErrorPrototype,
} = primordials;

let _TYPES = null;
Expand Down Expand Up @@ -147,6 +149,9 @@ translators.set('module', async function moduleStrategy(url) {
});

function enrichCJSError(err) {
if (err == null || ObjectGetPrototypeOf(err) !== SyntaxErrorPrototype) {
return;
}
const stack = StringPrototypeSplit(err.stack, '\n');
/*
* The regular expression below targets the most common import statement
Expand Down
56 changes: 56 additions & 0 deletions test/es-module/test-esm-cjs-load-error-note.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ const Import2 = fixtures.path('/es-modules/es-note-promiserej-import-2.cjs');
const Import3 = fixtures.path('/es-modules/es-note-unexpected-import-3.cjs');
const Import4 = fixtures.path('/es-modules/es-note-unexpected-import-4.cjs');
const Import5 = fixtures.path('/es-modules/es-note-unexpected-import-5.cjs');
const Error1 = fixtures.path('/es-modules/es-note-error-1.mjs');
const Error2 = fixtures.path('/es-modules/es-note-error-2.mjs');
const Error3 = fixtures.path('/es-modules/es-note-error-3.mjs');
const Error4 = fixtures.path('/es-modules/es-note-error-4.mjs');

// Expect note to be included in the error output
const expectedNote = 'To load an ES module, ' +
Expand Down Expand Up @@ -105,3 +109,55 @@ pImport5.on('close', mustCall((code) => {
assert.ok(!pImport5Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pImport5Stderr}`);
}));

const pError1 = spawn(process.execPath, [Error1]);
let pError1Stderr = '';
pError1.stderr.setEncoding('utf8');
pError1.stderr.on('data', (data) => {
pError1Stderr += data;
});
pError1.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pError1Stderr.includes('Error: some error'));
assert.ok(!pError1Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pError1Stderr}`);
}));

const pError2 = spawn(process.execPath, [Error2]);
let pError2Stderr = '';
pError2.stderr.setEncoding('utf8');
pError2.stderr.on('data', (data) => {
pError2Stderr += data;
});
pError2.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pError2Stderr.includes('string'));
assert.ok(!pError2Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pError2Stderr}`);
}));

const pError3 = spawn(process.execPath, [Error3]);
let pError3Stderr = '';
pError3.stderr.setEncoding('utf8');
pError3.stderr.on('data', (data) => {
pError3Stderr += data;
});
pError3.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pError3Stderr.includes('null'));
assert.ok(!pError3Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pError3Stderr}`);
}));

const pError4 = spawn(process.execPath, [Error4]);
let pError4Stderr = '';
pError4.stderr.setEncoding('utf8');
pError4.stderr.on('data', (data) => {
pError4Stderr += data;
});
pError4.on('close', mustCall((code) => {
assert.strictEqual(code, expectedCode);
assert.ok(pError4Stderr.includes('undefined'));
assert.ok(!pError4Stderr.includes(expectedNote),
`${expectedNote} must not be included in ${pError4Stderr}`);
}));
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-1.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error('some error');
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-1.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './es-note-error-1.cjs';
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-2.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw 'string';
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-2.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './es-note-error-2.cjs';
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-3.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw null;
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-3.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './es-note-error-3.cjs';
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-4.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw undefined;
1 change: 1 addition & 0 deletions test/fixtures/es-modules/es-note-error-4.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './es-note-error-4.cjs';