errors: print original exception context

When --enable-source-maps is set, the error context displayed
above the stack trace now shows original source rather than
transpiled.

PR-URL: https://github.com/nodejs/node/pull/33491
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
bcoe 2020-05-24 16:00:46 -07:00 committed by Benjamin Coe
parent 8f10bb2da5
commit 458677f5ef
No known key found for this signature in database
GPG Key ID: 84D97FAF3C07DF69
18 changed files with 270 additions and 21 deletions

View File

@ -26,14 +26,6 @@ function prepareMainThreadExecution(expandArgv1 = false) {
setupCoverageHooks(process.env.NODE_V8_COVERAGE);
}
// If source-map support has been enabled, we substitute in a new
// prepareStackTrace method, replacing the default in errors.js.
if (getOptionValue('--enable-source-maps')) {
const { prepareStackTrace } =
require('internal/source_map/prepare_stack_trace');
const { setPrepareStackTraceCallback } = internalBinding('errors');
setPrepareStackTraceCallback(prepareStackTrace);
}
setupDebugEnv();

View File

@ -50,6 +50,7 @@ const {
const { NativeModule } = require('internal/bootstrap/loaders');
const {
getSourceMapsEnabled,
maybeCacheSourceMap,
rekeySourceMap
} = require('internal/source_map/source_map_cache');
@ -71,7 +72,6 @@ const {
loadNativeModule
} = require('internal/modules/cjs/helpers');
const { getOptionValue } = require('internal/options');
const enableSourceMaps = getOptionValue('--enable-source-maps');
const preserveSymlinks = getOptionValue('--preserve-symlinks');
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
const manifest = getOptionValue('--experimental-policy') ?
@ -941,7 +941,7 @@ Module._load = function(request, parent, isMain) {
// Intercept exceptions that occur during the first tick and rekey them
// on error instance rather than module instance (which will immediately be
// garbage collected).
if (enableSourceMaps) {
if (getSourceMapsEnabled()) {
try {
module.load(filename);
} catch (err) {

View File

@ -5,6 +5,8 @@ const {
} = primordials;
const debug = require('internal/util/debuglog').debuglog('source_map');
const { getStringWidth } = require('internal/util/inspect');
const { readFileSync } = require('fs');
const { findSourceMap } = require('internal/source_map/source_map_cache');
const {
kNoOverride,
@ -34,7 +36,17 @@ const prepareStackTrace = (globalThis, error, trace) => {
if (trace.length === 0) {
return errorString;
}
let errorSource = '';
let firstSource;
let firstLine;
let firstColumn;
const preparedTrace = trace.map((t, i) => {
if (i === 0) {
firstLine = t.getLineNumber();
firstColumn = t.getColumnNumber();
firstSource = t.getFileName();
}
let str = i !== 0 ? '\n at ' : '';
str = `${str}${t}`;
try {
@ -49,8 +61,18 @@ const prepareStackTrace = (globalThis, error, trace) => {
} = sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
if (originalSource && originalLine !== undefined &&
originalColumn !== undefined) {
str +=
`\n -> ${originalSource.replace('file://', '')}:${originalLine + 1}:${originalColumn + 1}`;
const originalSourceNoScheme = originalSource
.replace(/^file:\/\//, '');
if (i === 0) {
firstLine = originalLine + 1;
firstColumn = originalColumn + 1;
firstSource = originalSourceNoScheme;
// Show error in original source context to help user pinpoint it:
errorSource = getErrorSource(firstSource, firstLine, firstColumn);
}
// Show both original and transpiled stack trace information:
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
`${originalColumn + 1}`;
}
}
} catch (err) {
@ -58,9 +80,38 @@ const prepareStackTrace = (globalThis, error, trace) => {
}
return str;
});
return `${errorString}\n at ${preparedTrace.join('')}`;
return `${errorSource}${errorString}\n at ${preparedTrace.join('')}`;
};
// 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(firstSource, firstLine, firstColumn) {
let exceptionLine = '';
let source;
try {
source = readFileSync(firstSource, 'utf8');
} catch (err) {
debug(err);
return exceptionLine;
}
const lines = source.split(/\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 line.slice(0, firstColumn)) {
prefix += (character === '\t') ? '\t' :
' '.repeat(getStringWidth(character));
}
prefix = prefix.slice(0, -1); // The last character is the '^'.
exceptionLine = `${firstSource}:${firstLine}\n${line}\n${prefix}^\n\n`;
return exceptionLine;
}
module.exports = {
prepareStackTrace,
};

View File

@ -39,12 +39,28 @@ const { fileURLToPath, URL } = require('url');
let Module;
let SourceMap;
let experimentalSourceMaps;
function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
if (experimentalSourceMaps === undefined) {
experimentalSourceMaps = getOptionValue('--enable-source-maps');
let sourceMapsEnabled;
function getSourceMapsEnabled() {
if (sourceMapsEnabled === undefined) {
sourceMapsEnabled = getOptionValue('--enable-source-maps');
if (sourceMapsEnabled) {
const {
enableSourceMaps,
setPrepareStackTraceCallback
} = internalBinding('errors');
const {
prepareStackTrace
} = require('internal/source_map/prepare_stack_trace');
setPrepareStackTraceCallback(prepareStackTrace);
enableSourceMaps();
}
}
if (!(process.env.NODE_V8_COVERAGE || experimentalSourceMaps)) return;
return sourceMapsEnabled;
}
function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
const sourceMapsEnabled = getSourceMapsEnabled();
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
let basePath;
try {
filename = normalizeReferrerURL(filename);
@ -248,6 +264,7 @@ function findSourceMap(uri, error) {
module.exports = {
findSourceMap,
getSourceMapsEnabled,
maybeCacheSourceMap,
rekeySourceMap,
sourceMapCacheToObject,

View File

@ -795,6 +795,14 @@ void Environment::set_emit_insecure_umask_warning(bool on) {
emit_insecure_umask_warning_ = on;
}
void Environment::set_source_maps_enabled(bool on) {
source_maps_enabled_ = on;
}
bool Environment::source_maps_enabled() const {
return source_maps_enabled_;
}
inline uint64_t Environment::thread_id() const {
return thread_id_;
}

View File

@ -1059,6 +1059,9 @@ class Environment : public MemoryRetainer {
inline bool emit_insecure_umask_warning() const;
inline void set_emit_insecure_umask_warning(bool on);
inline void set_source_maps_enabled(bool on);
inline bool source_maps_enabled() const;
inline void ThrowError(const char* errmsg);
inline void ThrowTypeError(const char* errmsg);
inline void ThrowRangeError(const char* errmsg);
@ -1277,6 +1280,8 @@ class Environment : public MemoryRetainer {
bool emit_err_name_warning_ = true;
bool emit_filehandle_warning_ = true;
bool emit_insecure_umask_warning_ = true;
bool source_maps_enabled_ = false;
size_t async_callback_scope_depth_ = 0;
std::vector<double> destroy_async_id_list_;

View File

@ -57,6 +57,16 @@ static std::string GetErrorSource(Isolate* isolate,
node::Utf8Value encoded_source(isolate, source_line_maybe.ToLocalChecked());
std::string sourceline(*encoded_source, encoded_source.length());
// If source maps have been enabled, the exception line will instead be
// added in the JavaScript context:
Environment* env = Environment::GetCurrent(isolate);
const bool has_source_map_url =
!message->GetScriptOrigin().SourceMapUrl().IsEmpty();
if (has_source_map_url && env->source_maps_enabled()) {
*added_exception_line = false;
return sourceline;
}
if (sourceline.find("node-do-not-add-exception-line") != std::string::npos) {
*added_exception_line = false;
return sourceline;
@ -802,6 +812,11 @@ void SetPrepareStackTraceCallback(const FunctionCallbackInfo<Value>& args) {
env->set_prepare_stack_trace_callback(args[0].As<Function>());
}
static void EnableSourceMaps(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
env->set_source_maps_enabled(true);
}
static void SetEnhanceStackForFatalException(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
@ -840,6 +855,7 @@ void Initialize(Local<Object> target,
Environment* env = Environment::GetCurrent(context);
env->SetMethod(
target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback);
env->SetMethod(target, "enableSourceMaps", EnableSourceMaps);
env->SetMethod(target,
"setEnhanceStackForFatalException",
SetEnhanceStackForFatalException);

13
test/fixtures/source-map/icu.js vendored Normal file
View File

@ -0,0 +1,13 @@
const React = {
createElement: () => {
"あ 🐕 🐕", function (e) {
throw e;
}(Error("an error"));
}
};
const profile = /*#__PURE__*/React.createElement("div", null, /*#__PURE__*/React.createElement("img", {
src: "avatar.png",
className: "profile"
}), /*#__PURE__*/React.createElement("h3", null, ["hello"]));
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImljdS5qc3giXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IkFBQUEsTUFBTSxLQUFLLEdBQUc7QUFDWixFQUFBLGFBQWEsRUFBRSxNQUFNO0FBQ2xCO0FBQUE7QUFBQSxNQUFpQixLQUFLLENBQUMsVUFBRCxDQUF0QixDQUFEO0FBQ0Q7QUFIVyxDQUFkO0FBTUEsTUFBTSxPQUFPLGdCQUNYLDhDQUNFO0FBQUssRUFBQSxHQUFHLEVBQUMsWUFBVDtBQUFzQixFQUFBLFNBQVMsRUFBQztBQUFoQyxFQURGLGVBRUUsZ0NBQUssQ0FBQyxPQUFELENBQUwsQ0FGRixDQURGIiwiZmlsZSI6InN0ZG91dCIsInNvdXJjZXNDb250ZW50IjpbImNvbnN0IFJlYWN0ID0ge1xuICBjcmVhdGVFbGVtZW50OiAoKSA9PiB7XG4gICAgKFwi44GCIPCfkJUg8J+QlVwiLCB0aHJvdyBFcnJvcihcImFuIGVycm9yXCIpKTtcbiAgfVxufTtcblxuY29uc3QgcHJvZmlsZSA9IChcbiAgPGRpdj5cbiAgICA8aW1nIHNyYz1cImF2YXRhci5wbmdcIiBjbGFzc05hbWU9XCJwcm9maWxlXCIgLz5cbiAgICA8aDM+e1tcImhlbGxvXCJdfTwvaDM+XG4gIDwvZGl2PlxuKTtcbiJdfQ==

12
test/fixtures/source-map/icu.jsx vendored Normal file
View File

@ -0,0 +1,12 @@
const React = {
createElement: () => {
("あ 🐕 🐕", throw Error("an error"));
}
};
const profile = (
<div>
<img src="avatar.png" className="profile" />
<h3>{["hello"]}</h3>
</div>
);

29
test/fixtures/source-map/tabs.coffee vendored Normal file
View File

@ -0,0 +1,29 @@
# Assignment:
number = 42
opposite = true
# Conditions:
number = -42 if opposite
# Functions:
square = (x) -> x * x
# Arrays:
list = [1, 2, 3, 4, 5]
# Objects:
math =
root: Math.sqrt
square: square
cube: (x) -> x * square x
# Splats:
race = (winner, runners...) ->
print winner, runners
# Existence:
if true
alert "I knew it!"
# Array comprehensions:
cubes = (math.cube num for num in list)

56
test/fixtures/source-map/tabs.js vendored Normal file
View File

@ -0,0 +1,56 @@
// Generated by CoffeeScript 2.5.1
(function() {
// Assignment:
var cubes, list, math, num, number, opposite, race, square;
number = 42;
opposite = true;
if (opposite) {
// Conditions:
number = -42;
}
// Functions:
square = function(x) {
return x * x;
};
// Arrays:
list = [1, 2, 3, 4, 5];
// Objects:
math = {
root: Math.sqrt,
square: square,
cube: function(x) {
return x * square(x);
}
};
// Splats:
race = function(winner, ...runners) {
return print(winner, runners);
};
// Existence:
if (true) {
alert("I knew it!");
}
// Array comprehensions:
cubes = (function() {
var i, len, results;
results = [];
for (i = 0, len = list.length; i < len; i++) {
num = list[i];
results.push(math.cube(num));
}
return results;
})();
}).call(this);
//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoidGFicy5qcyIsInNvdXJjZVJvb3QiOiIiLCJzb3VyY2VzIjpbInRhYnMuY29mZmVlIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiI7QUFBYTtFQUFBO0FBQUEsTUFBQSxLQUFBLEVBQUEsSUFBQSxFQUFBLElBQUEsRUFBQSxHQUFBLEVBQUEsTUFBQSxFQUFBLFFBQUEsRUFBQSxJQUFBLEVBQUE7O0VBQ2IsTUFBQSxHQUFXOztFQUNYLFFBQUEsR0FBVzs7RUFHWCxJQUFnQixRQUFoQjs7SUFBQSxNQUFBLEdBQVMsQ0FBQyxHQUFWO0dBTGE7OztFQVFiLE1BQUEsR0FBUyxRQUFBLENBQUMsQ0FBRCxDQUFBO1dBQU8sQ0FBQSxHQUFJO0VBQVgsRUFSSTs7O0VBV2IsSUFBQSxHQUFPLENBQUMsQ0FBRCxFQUFJLENBQUosRUFBTyxDQUFQLEVBQVUsQ0FBVixFQUFhLENBQWIsRUFYTTs7O0VBY2IsSUFBQSxHQUNDO0lBQUEsSUFBQSxFQUFRLElBQUksQ0FBQyxJQUFiO0lBQ0EsTUFBQSxFQUFRLE1BRFI7SUFFQSxJQUFBLEVBQVEsUUFBQSxDQUFDLENBQUQsQ0FBQTthQUFPLENBQUEsR0FBSSxNQUFBLENBQU8sQ0FBUDtJQUFYO0VBRlIsRUFmWTs7O0VBb0JiLElBQUEsR0FBTyxRQUFBLENBQUMsTUFBRCxFQUFBLEdBQVMsT0FBVCxDQUFBO1dBQ04sS0FBQSxDQUFNLE1BQU4sRUFBYyxPQUFkO0VBRE0sRUFwQk07OztFQXdCYixJQUFHLElBQUg7SUFDQyxLQUFBLENBQU0sWUFBTixFQUREO0dBeEJhOzs7RUE0QmIsS0FBQTs7QUFBUztJQUFBLEtBQUEsc0NBQUE7O21CQUFBLElBQUksQ0FBQyxJQUFMLENBQVUsR0FBVjtJQUFBLENBQUE7OztBQTVCSSIsInNvdXJjZXNDb250ZW50IjpbIiMgQXNzaWdubWVudDpcbm51bWJlciAgID0gNDJcbm9wcG9zaXRlID0gdHJ1ZVxuXG4jIENvbmRpdGlvbnM6XG5udW1iZXIgPSAtNDIgaWYgb3Bwb3NpdGVcblxuIyBGdW5jdGlvbnM6XG5zcXVhcmUgPSAoeCkgLT4geCAqIHhcblxuIyBBcnJheXM6XG5saXN0ID0gWzEsIDIsIDMsIDQsIDVdXG5cbiMgT2JqZWN0czpcbm1hdGggPVxuXHRyb290OiAgIE1hdGguc3FydFxuXHRzcXVhcmU6IHNxdWFyZVxuXHRjdWJlOiAgICh4KSAtPiB4ICogc3F1YXJlIHhcblxuIyBTcGxhdHM6XG5yYWNlID0gKHdpbm5lciwgcnVubmVycy4uLikgLT5cblx0cHJpbnQgd2lubmVyLCBydW5uZXJzXG5cbiMgRXhpc3RlbmNlOlxuaWYgdHJ1ZVxuXHRhbGVydCBcIkkga25ldyBpdCFcIlxuXG4jIEFycmF5IGNvbXByZWhlbnNpb25zOlxuY3ViZXMgPSAobWF0aC5jdWJlIG51bSBmb3IgbnVtIGluIGxpc3QpXG4iXX0=
//# sourceURL=/Users/bencoe/oss/coffee-script-test/tabs.coffee

View File

@ -0,0 +1,5 @@
// Flags: --enable-source-maps
'use strict';
require('../common');
require('../fixtures/source-map/tabs.js');

View File

@ -0,0 +1,17 @@
*tabs.coffee:26
alert "I knew it!"
^
ReferenceError: alert is not defined
at Object.<anonymous> (*tabs.coffee:39:5)
-> *tabs.coffee:26:2
at Object.<anonymous> (*tabs.coffee:53:4)
-> *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:*
at Function.Module._load (internal/modules/cjs/loader.js:*
at Module.require (internal/modules/cjs/loader.js:*
at require (internal/modules/cjs/helpers.js:*
at Object.<anonymous> (*source_map_reference_error_tabs.js:*
at Module._compile (internal/modules/cjs/loader.js:*

View File

@ -1,4 +1,7 @@
reachable
*typescript-throw.ts:18
throw Error('an exception');
^
Error: an exception
at branch (*typescript-throw.js:20:15)
-> *typescript-throw.ts:18:11

View File

@ -1,4 +1,7 @@
reachable
*typescript-throw.ts:18
throw Error('an exception');
^
Error: an exception
at branch (*typescript-throw.js:20:15)
-> *typescript-throw.ts:18:11

View File

@ -0,0 +1,5 @@
// Flags: --enable-source-maps
'use strict';
require('../common');
require('../fixtures/source-map/icu');

View File

@ -0,0 +1,17 @@
*icu.jsx:3
("********", throw Error("an error"));
^
Error: an error
at Object.createElement (*icu.js:5:7)
-> *icu.jsx:3:23
at Object.<anonymous> (*icu.js:8:82)
-> *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:*
at Function.Module._load (internal/modules/cjs/loader.js:*
at Module.require (internal/modules/cjs/loader.js:*
at require (internal/modules/cjs/helpers.js:*
at Object.<anonymous> (*source_map_throw_icu.js:*
at Module._compile (internal/modules/cjs/loader.js:*

View File

@ -1,6 +1,6 @@
*uglify-throw.js:1
setImmediate(function(){!function(){throw Error("goodbye")}()});
^
*uglify-throw-original.js:5
throw Error('goodbye');
^
Error: goodbye
at *uglify-throw.js:1:43