Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
errors: display original symbol name
If symbol names array has been populated in source map, include
original symbol name in error message.

Fixes #35325

PR-URL: #36042
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
bcoe authored and targos committed Jun 11, 2021
1 parent 6adec63 commit ee44447
Show file tree
Hide file tree
Showing 14 changed files with 178 additions and 66 deletions.
1 change: 1 addition & 0 deletions doc/api/module.md
Expand Up @@ -206,6 +206,7 @@ consists of the following keys:
* originalSource: {string}
* originalLine: {number}
* originalColumn: {number}
* name: {string}
[CommonJS]: modules.md
[ES Modules]: esm.md
Expand Down
124 changes: 81 additions & 43 deletions lib/internal/source_map/prepare_stack_trace.js
Expand Up @@ -47,45 +47,48 @@ const prepareStackTrace = (globalThis, error, trace) => {
}

let errorSource = '';
let firstLine;
let firstColumn;
let lastSourceMap;
let lastFileName;
const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => {
if (i === 0) {
firstLine = t.getLineNumber();
firstColumn = t.getColumnNumber();
}
let str = i !== 0 ? '\n at ' : '';
str = `${str}${t}`;
try {
const sm = findSourceMap(t.getFileName());
// A stack trace will often have several call sites in a row within the
// same file, cache the source map and file content accordingly:
const fileName = t.getFileName();
const sm = fileName === lastFileName ?
lastSourceMap :
findSourceMap(fileName);
lastSourceMap = sm;
lastFileName = fileName;
if (sm) {
// Source Map V3 lines/columns use zero-based offsets whereas, in
// stack traces, they start at 1/1.
// Source Map V3 lines/columns start at 0/0 whereas stack traces
// start at 1/1:
const {
originalLine,
originalColumn,
originalSource
originalSource,
} = sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
if (originalSource && originalLine !== undefined &&
originalColumn !== undefined) {
const name = getOriginalSymbolName(sm, trace, i);
if (i === 0) {
firstLine = originalLine + 1;
firstColumn = originalColumn + 1;

// Show error in original source context to help user pinpoint it:
errorSource = getErrorSource(
sm.payload,
sm,
originalSource,
firstLine,
firstColumn
originalLine,
originalColumn
);
}
// Show both original and transpiled stack trace information:
const prefix = (name && name !== t.getFunctionName()) ?
`\n -> at ${name}` :
'\n ->';
const originalSourceNoScheme =
StringPrototypeStartsWith(originalSource, 'file://') ?
fileURLToPath(originalSource) : originalSource;
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
`${originalColumn + 1}`;
str += `${prefix} (${originalSourceNoScheme}:${originalLine + 1}:` +
`${originalColumn + 1})`;
}
}
} catch (err) {
Expand All @@ -96,18 +99,69 @@ const prepareStackTrace = (globalThis, error, trace) => {
return `${errorSource}${errorString}\n at ${preparedTrace}`;
};

// Transpilers may have removed the original symbol name used in the stack
// trace, if possible restore it from the names field of the source map:
function getOriginalSymbolName(sourceMap, trace, curIndex) {
// First check for a symbol name associated with the enclosing function:
const enclosingEntry = sourceMap.findEntry(
trace[curIndex].getEnclosingLineNumber() - 1,
trace[curIndex].getEnclosingColumnNumber() - 1
);
if (enclosingEntry.name) return enclosingEntry.name;
// Fallback to using the symbol name attached to the next stack frame:
const currentFileName = trace[curIndex].getFileName();
const nextCallSite = trace[curIndex + 1];
if (nextCallSite && currentFileName === nextCallSite.getFileName()) {
const { name } = sourceMap.findEntry(
nextCallSite.getLineNumber() - 1,
nextCallSite.getColumnNumber() - 1
);
return name;
}
}

// Places a snippet of code from where the exception was originally thrown
// above the stack trace. This logic is modeled after GetErrorSource in
// node_errors.cc.
function getErrorSource(payload, originalSource, firstLine, firstColumn) {
function getErrorSource(
sourceMap,
originalSourcePath,
originalLine,
originalColumn
) {
let exceptionLine = '';
const originalSourceNoScheme =
StringPrototypeStartsWith(originalSource, 'file://') ?
fileURLToPath(originalSource) : originalSource;
const originalSourcePathNoScheme =
StringPrototypeStartsWith(originalSourcePath, 'file://') ?
fileURLToPath(originalSourcePath) : originalSourcePath;
const source = getOriginalSource(
sourceMap.payload,
originalSourcePathNoScheme
);
const lines = StringPrototypeSplit(source, /\r?\n/, originalLine + 1);
const line = lines[originalLine];
if (!line) return exceptionLine;

// Display ^ in appropriate position, regardless of whether tabs or
// spaces are used:
let prefix = '';
for (const character of StringPrototypeSlice(line, 0, originalColumn + 1)) {
prefix += (character === '\t') ? '\t' :
StringPrototypeRepeat(' ', getStringWidth(character));
}
prefix = StringPrototypeSlice(prefix, 0, -1); // The last character is '^'.

exceptionLine =
`${originalSourcePathNoScheme}:${originalLine + 1}\n${line}\n${prefix}^\n\n`;
return exceptionLine;
}

function getOriginalSource(payload, originalSourcePath) {
let source;
const originalSourcePathNoScheme =
StringPrototypeStartsWith(originalSourcePath, 'file://') ?
fileURLToPath(originalSourcePath) : originalSourcePath;
const sourceContentIndex =
ArrayPrototypeIndexOf(payload.sources, originalSource);
ArrayPrototypeIndexOf(payload.sources, originalSourcePath);
if (payload.sourcesContent?.[sourceContentIndex]) {
// First we check if the original source content was provided in the
// source map itself:
Expand All @@ -116,29 +170,13 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
// If no sourcesContent was found, attempt to load the original source
// from disk:
try {
source = readFileSync(originalSourceNoScheme, 'utf8');
source = readFileSync(originalSourcePathNoScheme, 'utf8');
} catch (err) {
debug(err);
return '';
source = '';
}
}

const lines = StringPrototypeSplit(source, /\r?\n/, firstLine);
const line = lines[firstLine - 1];
if (!line) return exceptionLine;

// Display ^ in appropriate position, regardless of whether tabs or
// spaces are used:
let prefix = '';
for (const character of StringPrototypeSlice(line, 0, firstColumn)) {
prefix += (character === '\t') ? '\t' :
StringPrototypeRepeat(' ', getStringWidth(character));
}
prefix = StringPrototypeSlice(prefix, 0, -1); // The last character is '^'.

exceptionLine =
`${originalSourceNoScheme}:${firstLine}\n${line}\n${prefix}^\n\n`;
return exceptionLine;
return source;
}

module.exports = {
Expand Down
16 changes: 10 additions & 6 deletions lib/internal/source_map/source_map.js
Expand Up @@ -203,7 +203,8 @@ class SourceMap {
generatedColumn: entry[1],
originalSource: entry[2],
originalLine: entry[3],
originalColumn: entry[4]
originalColumn: entry[4],
name: entry[5],
};
}

Expand All @@ -214,6 +215,7 @@ class SourceMap {
let sourceIndex = 0;
let sourceLineNumber = 0;
let sourceColumnNumber = 0;
let nameIndex = 0;

const sources = [];
const originalToCanonicalURLMap = {};
Expand All @@ -229,7 +231,6 @@ class SourceMap {

const stringCharIterator = new StringCharIterator(map.mappings);
let sourceURL = sources[sourceIndex];

while (true) {
if (stringCharIterator.peek() === ',')
stringCharIterator.next();
Expand All @@ -256,12 +257,15 @@ class SourceMap {
}
sourceLineNumber += decodeVLQ(stringCharIterator);
sourceColumnNumber += decodeVLQ(stringCharIterator);
if (!isSeparator(stringCharIterator.peek()))
// Unused index into the names list.
decodeVLQ(stringCharIterator);

let name;
if (!isSeparator(stringCharIterator.peek())) {
nameIndex += decodeVLQ(stringCharIterator);
name = map.names?.[nameIndex];
}

this.#mappings.push([lineNumber, columnNumber, sourceURL,
sourceLineNumber, sourceColumnNumber]);
sourceLineNumber, sourceColumnNumber, name]);
}
};
}
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/source-map/enclosing-call-site-min.js
@@ -0,0 +1,3 @@
var functionA=function(){functionB()};function functionB(){functionC()}var functionC=function(){functionD()},functionD=function(){if(0<Math.random())throw Error("an error!");},thrower=functionA;try{functionA()}catch(a){throw a;};
//# sourceMappingURL=enclosing-call-site.js.map

27 changes: 27 additions & 0 deletions test/fixtures/source-map/enclosing-call-site.js
@@ -0,0 +1,27 @@
const functionA = () => {
functionB()
}

function functionB() {
functionC()
}

const functionC = () => {
functionD()
}

const functionD = () => {
(function functionE () {
if (Math.random() > 0) {
throw new Error('an error!')
}
})()
}

const thrower = functionA

try {
thrower()
} catch (err) {
throw err
}
8 changes: 8 additions & 0 deletions test/fixtures/source-map/enclosing-call-site.js.map

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

5 changes: 5 additions & 0 deletions test/message/source_map_enclosing_function.js
@@ -0,0 +1,5 @@
// Flags: --enable-source-maps

'use strict';
require('../common');
require('../fixtures/source-map/enclosing-call-site-min.js');
20 changes: 20 additions & 0 deletions test/message/source_map_enclosing_function.out
@@ -0,0 +1,20 @@
*enclosing-call-site.js:16
throw new Error('an error!')
^

Error: an error!
at functionD (*enclosing-call-site-min.js:1:156)
-> (*enclosing-call-site.js:16:17)
at functionC (*enclosing-call-site-min.js:1:97)
-> (*enclosing-call-site.js:10:3)
at functionB (*enclosing-call-site-min.js:1:60)
-> (*enclosing-call-site.js:6:3)
at functionA (*enclosing-call-site-min.js:1:26)
-> (*enclosing-call-site.js:2:3)
at Object.<anonymous> (*enclosing-call-site-min.js:1:199)
-> (*enclosing-call-site.js:24:3)
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Module.require (internal/modules/cjs/loader.js:*)
4 changes: 2 additions & 2 deletions test/message/source_map_reference_error_tabs.out
Expand Up @@ -4,9 +4,9 @@

ReferenceError: alert is not defined
at Object.<anonymous> (*tabs.coffee:39:5)
-> *tabs.coffee:26:2
-> *tabs.coffee:26:2*
at Object.<anonymous> (*tabs.coffee:53:4)
-> *tabs.coffee:1:14
-> *tabs.coffee:1:14*
at Module._compile (internal/modules/cjs/loader.js:*
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*
at Module.load (internal/modules/cjs/loader.js:*
Expand Down
4 changes: 2 additions & 2 deletions test/message/source_map_throw_catch.out
Expand Up @@ -4,9 +4,9 @@ reachable
^
Error: an exception
at branch (*typescript-throw.js:20:15)
-> *typescript-throw.ts:18:11
-> *typescript-throw.ts:18:11*
at Object.<anonymous> (*typescript-throw.js:26:1)
-> *typescript-throw.ts:24:1
-> *typescript-throw.ts:24:1*
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
Expand Down
4 changes: 2 additions & 2 deletions test/message/source_map_throw_first_tick.out
Expand Up @@ -4,9 +4,9 @@ reachable
^
Error: an exception
at branch (*typescript-throw.js:20:15)
-> *typescript-throw.ts:18:11
-> *typescript-throw.ts:18:11*
at Object.<anonymous> (*typescript-throw.js:26:1)
-> *typescript-throw.ts:24:1
-> *typescript-throw.ts:24:1*
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
Expand Down
4 changes: 2 additions & 2 deletions test/message/source_map_throw_icu.out
Expand Up @@ -4,9 +4,9 @@

Error: an error
at Object.createElement (*icu.js:5:7)
-> *icu.jsx:3:23
-> *icu.jsx:3:23*
at Object.<anonymous> (*icu.js:8:82)
-> *icu.jsx:9:5
-> *icu.jsx:9:5*
at Module._compile (internal/modules/cjs/loader.js:*
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*
at Module.load (internal/modules/cjs/loader.js:*
Expand Down
4 changes: 2 additions & 2 deletions test/message/source_map_throw_set_immediate.out
Expand Up @@ -4,7 +4,7 @@

Error: goodbye
at *uglify-throw.js:1:43
-> *uglify-throw-original.js:5:9
-> at Hello *uglify-throw-original.js:5:9*
at Immediate.<anonymous> (*uglify-throw.js:1:60)
-> *uglify-throw-original.js:9:3
-> *uglify-throw-original.js:9:3*
at processImmediate (internal/timers.js:*)
20 changes: 13 additions & 7 deletions test/parallel/test-source-map-enable.js
Expand Up @@ -170,12 +170,15 @@ function nextdir() {
'--enable-source-maps',
require.resolve('../fixtures/source-map/uglify-throw.js')
]);
assert.ok(
output.stderr.toString().match(/->.*uglify-throw-original\.js:5:9/)
assert.match(
output.stderr.toString(),
/->.*uglify-throw-original\.js:5:9/
);
assert.ok(
output.stderr.toString().match(/->.*uglify-throw-original\.js:9:3/)
assert.match(
output.stderr.toString(),
/->.*uglify-throw-original\.js:9:3/
);
assert.match(output.stderr.toString(), /at Hello/);
}

// Applies source-maps generated by tsc to stack trace.
Expand Down Expand Up @@ -276,11 +279,14 @@ function nextdir() {
require.resolve('../fixtures/source-map/webpack.js')
]);
// Error in original context of source content:
assert.ok(
output.stderr.toString().match(/throw new Error\('oh no!'\)\r?\n.*\^/)
assert.match(
output.stderr.toString(),
/throw new Error\('oh no!'\)\r?\n.*\^/
);
// Rewritten stack trace:
assert.ok(output.stderr.toString().includes('webpack:///webpack.js:14:9'));
assert.match(output.stderr.toString(), /webpack:\/\/\/webpack\.js:14:9/);
assert.match(output.stderr.toString(), /at functionD.*14:9/);
assert.match(output.stderr.toString(), /at functionC.*10:3/);
}

// Stores and applies source map associated with file that throws while
Expand Down

0 comments on commit ee44447

Please sign in to comment.