From dc855af18eb2b0b764d6a7ae37f0fdf11a07dc36 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 12 Jun 2021 10:17:31 -0700 Subject: [PATCH] errors: don't throw TypeError on missing export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/39017 Reviewed-By: Antoine du Hamel Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- lib/internal/modules/esm/module_job.js | 10 +++++++++- .../source-map/esm-export-missing-module.mjs | 0 .../esm-export-missing-module.mjs.map | 1 + .../source-map/esm-export-missing.mjs | 2 ++ .../source-map/esm-export-missing.mjs.map | 1 + .../fixtures/source-map/esm-export-missing.ts | 3 +++ test/parallel/test-source-map-enable.js | 19 +++++++++++++++++++ 7 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/source-map/esm-export-missing-module.mjs create mode 100644 test/fixtures/source-map/esm-export-missing-module.mjs.map create mode 100644 test/fixtures/source-map/esm-export-missing.mjs create mode 100644 test/fixtures/source-map/esm-export-missing.mjs.map create mode 100644 test/fixtures/source-map/esm-export-missing.ts diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 0e1a7eb7b6a779..15bdbf3bfc6a66 100644 --- 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 = splitStack[0]; diff --git a/test/fixtures/source-map/esm-export-missing-module.mjs b/test/fixtures/source-map/esm-export-missing-module.mjs new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/fixtures/source-map/esm-export-missing-module.mjs.map b/test/fixtures/source-map/esm-export-missing-module.mjs.map new file mode 100644 index 00000000000000..17417c928dc04d --- /dev/null +++ b/test/fixtures/source-map/esm-export-missing-module.mjs.map @@ -0,0 +1 @@ +{"version":3,"file":"esm-export-missing-module.esm","sourceRoot":"","sources":["./exm-export-missing-module.ts"],"names":[],"mappings":"AAEA,MAAM,UAAU,SAAS;AAEzB,CAAC"} \ No newline at end of file diff --git a/test/fixtures/source-map/esm-export-missing.mjs b/test/fixtures/source-map/esm-export-missing.mjs new file mode 100644 index 00000000000000..4bda755a86b389 --- /dev/null +++ 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 new file mode 100644 index 00000000000000..2d1d482dc97083 --- /dev/null +++ b/test/fixtures/source-map/esm-export-missing.mjs.map @@ -0,0 +1 @@ +{"version":3,"file":"esm-export-missing.ts","sourceRoot":"","sources":["./esm-export-missing.ts"],"names":[],"mappings":"AACA,OAAO,EAAE,SAAS,EAAE,MAAM,gBAAgB,CAAC;AAC3C,OAAO,CAAC,IAAI,CAAC,SAAS,CAAC,CAAC"} \ No newline at end of file diff --git a/test/fixtures/source-map/esm-export-missing.ts b/test/fixtures/source-map/esm-export-missing.ts new file mode 100644 index 00000000000000..14797c69feaebc --- /dev/null +++ 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 index 92dddc9dff64ca..5dc5b88f31344c 100644 --- 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) {