Skip to content

Commit

Permalink
errors: display original symbol name
Browse files Browse the repository at this point in the history
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.