Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit eddde6c

Browse filesBrowse files
bcoedanielleadams
authored andcommitted
errors: don't rekey on primitive type
If an error is thrown before a module is loaded, we attempt to cache source map against error object, rather than module object. We can't do this if the error is a primitive type Fixes #38945 PR-URL: #39025 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d2b972e commit eddde6c
Copy full SHA for eddde6c

File tree

Expand file treeCollapse file tree

5 files changed

+28
-24
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+28
-24
lines changed
Open diff view settings
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+1-15Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,7 @@ module.exports = {
7474

7575
const { NativeModule } = require('internal/bootstrap/loaders');
7676
const {
77-
getSourceMapsEnabled,
7877
maybeCacheSourceMap,
79-
rekeySourceMap
8078
} = require('internal/source_map/source_map_cache');
8179
const { pathToFileURL, fileURLToPath, isURLInstance } = require('internal/url');
8280
const { deprecate } = require('internal/util');
@@ -815,19 +813,7 @@ Module._load = function(request, parent, isMain) {
815813

816814
let threw = true;
817815
try {
818-
// Intercept exceptions that occur during the first tick and rekey them
819-
// on error instance rather than module instance (which will immediately be
820-
// garbage collected).
821-
if (getSourceMapsEnabled()) {
822-
try {
823-
module.load(filename);
824-
} catch (err) {
825-
rekeySourceMap(Module._cache[filename], err);
826-
throw err; /* node-do-not-add-exception-line */
827-
}
828-
} else {
829-
module.load(filename);
830-
}
816+
module.load(filename);
831817
threw = false;
832818
} finally {
833819
if (threw) {
Collapse file

‎lib/internal/source_map/source_map_cache.js‎

Copy file name to clipboardExpand all lines: lib/internal/source_map/source_map_cache.js
-9Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -172,14 +172,6 @@ function sourcesToAbsolute(baseURL, data) {
172172
return data;
173173
}
174174

175-
// Move source map from garbage collected module to alternate key.
176-
function rekeySourceMap(cjsModuleInstance, newInstance) {
177-
const sourceMap = cjsSourceMapCache.get(cjsModuleInstance);
178-
if (sourceMap) {
179-
cjsSourceMapCache.set(newInstance, sourceMap);
180-
}
181-
}
182-
183175
// WARNING: The `sourceMapCacheToObject` and `appendCJSCache` run during
184176
// shutdown. In particular, they also run when Workers are terminated, making
185177
// it important that they do not call out to any user-provided code, including
@@ -240,6 +232,5 @@ module.exports = {
240232
findSourceMap,
241233
getSourceMapsEnabled,
242234
maybeCacheSourceMap,
243-
rekeySourceMap,
244235
sourceMapCacheToObject,
245236
};
Collapse file
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/*
2+
* comments dropped by uglify.
3+
*/
4+
function Hello() {
5+
throw 'goodbye';
6+
}
7+
8+
Hello();
Collapse file

‎test/fixtures/source-map/throw-string.js‎

Copy file name to clipboardExpand all lines: test/fixtures/source-map/throw-string.js
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/parallel/test-source-map-enable.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-source-map-enable.js
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,23 @@ function nextdir() {
307307
assert.ok(sourceMap);
308308
}
309309

310+
// Does not throw TypeError when primitive value is thrown.
311+
{
312+
const coverageDirectory = nextdir();
313+
const output = spawnSync(process.execPath, [
314+
'--enable-source-maps',
315+
require.resolve('../fixtures/source-map/throw-string.js'),
316+
], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
317+
const sourceMap = getSourceMapFromCache(
318+
'throw-string.js',
319+
coverageDirectory
320+
);
321+
// Original stack trace.
322+
assert.match(output.stderr.toString(), /goodbye/);
323+
// Source map should have been serialized.
324+
assert.ok(sourceMap);
325+
}
326+
310327
function getSourceMapFromCache(fixtureFile, coverageDirectory) {
311328
const jsonFiles = fs.readdirSync(coverageDirectory);
312329
for (const jsonFile of jsonFiles) {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.