errors: don't throw TypeError on missing export

Logic in module_job.js assumes detailed stack trace from node_errors.cc which is not populated when --enable-source-maps is set. Fixes nodejs#38790 PR-URL: nodejs#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]>
cjihrig · Dec 27, 2023 · ea73af5 · ea73af5
1 parent 0d86855
commit ea73af5
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 1 deletion.
diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -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();
 
@@ -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(

diff --git a/test/fixtures/source-map/esm-export-missing-module.mjs b/test/fixtures/source-map/esm-export-missing-module.mjs
diff --git a/test/fixtures/source-map/esm-export-missing-module.mjs.map b/test/fixtures/source-map/esm-export-missing-module.mjs.map
diff --git a/test/fixtures/source-map/esm-export-missing.mjs b/test/fixtures/source-map/esm-export-missing.mjs
@@ -0,0 +1,2 @@
+import { Something } from './esm-export-missing-module.mjs';
+//# sourceMappingURL=esm-export-missing.mjs.map
diff --git a/test/fixtures/source-map/esm-export-missing.mjs.map b/test/fixtures/source-map/esm-export-missing.mjs.map
diff --git a/test/fixtures/source-map/esm-export-missing.ts b/test/fixtures/source-map/esm-export-missing.ts
@@ -0,0 +1,3 @@
+
+import { Something } from './exm-export-missing-module.mjs';
+console.info(Something);
diff --git a/test/parallel/test-source-map-enable.js b/test/parallel/test-source-map-enable.js
@@ -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) {