Skip to content

Commit

Permalink
errors: don't throw TypeError on missing export
Browse files Browse the repository at this point in the history
Logic in module_job.js assumes detailed stack trace from node_errors.cc
which is not populated when --enable-source-maps is set.

Fixes #38790

PR-URL: #39017
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
bcoe authored and sbaayel committed Dec 27, 2023
1 parent 0d86855 commit ea73af5
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 1 deletion.
10 changes: 9 additions & 1 deletion lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const {
const { ModuleWrap } = internalBinding('module_wrap');

const { decorateErrorStack } = require('internal/util');
const {
getSourceMapsEnabled,
} = require('internal/source_map/source_map_cache');
const assert = require('internal/assert');
const resolvedPromise = PromiseResolve();

Expand Down Expand Up @@ -122,7 +125,12 @@ class ModuleJob {
}
} catch (e) {
decorateErrorStack(e);
if (StringPrototypeIncludes(e.message,
// TODO(@bcoe): Add source map support to exception that occurs as result
// of missing named export. This is currently not possible because
// stack trace originates in module_job, not the file itself. A hidden
// symbol with filename could be set in node_errors.cc to facilitate this.
if (!getSourceMapsEnabled() &&
StringPrototypeIncludes(e.message,
' does not provide an export named')) {
const splitStack = StringPrototypeSplit(e.stack, '\n');
const parentFileUrl = StringPrototypeReplace(
Expand Down
Empty file.
1 change: 1 addition & 0 deletions test/fixtures/source-map/esm-export-missing-module.mjs.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/fixtures/source-map/esm-export-missing.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { Something } from './esm-export-missing-module.mjs';
//# sourceMappingURL=esm-export-missing.mjs.map
1 change: 1 addition & 0 deletions test/fixtures/source-map/esm-export-missing.mjs.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions test/fixtures/source-map/esm-export-missing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

import { Something } from './exm-export-missing-module.mjs';
console.info(Something);
19 changes: 19 additions & 0 deletions test/parallel/test-source-map-enable.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,25 @@ function nextdir() {
assert.ok(sourceMap);
}

// Does not throw TypeError when exception occurs as result of missing named
// export.
{
const coverageDirectory = nextdir();
const output = spawnSync(process.execPath, [
'--enable-source-maps',
require.resolve('../fixtures/source-map/esm-export-missing.mjs'),
], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
const sourceMap = getSourceMapFromCache(
'esm-export-missing.mjs',
coverageDirectory
);
// Module loader error displayed.
assert.match(output.stderr.toString(),
/does not provide an export named 'Something'/);
// Source map should have been serialized.
assert.ok(sourceMap);
}

function getSourceMapFromCache(fixtureFile, coverageDirectory) {
const jsonFiles = fs.readdirSync(coverageDirectory);
for (const jsonFile of jsonFiles) {
Expand Down

0 comments on commit ea73af5

Please sign in to comment.