Skip to content

Commit efe28e3

Browse files
bcoebcoe
committed
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. Backport-PR-URL: nodejs#37700 PR-URL: nodejs#33491 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 6a9ec8d commit efe28e3

18 files changed

+270
-21
lines changed

‎lib/internal/bootstrap/pre_execution.js

-8
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,6 @@ function prepareMainThreadExecution(expandArgv1 = false) {
3131
setupCoverageHooks(process.env.NODE_V8_COVERAGE);
3232
}
3333

34-
// If source-map support has been enabled, we substitute in a new
35-
// prepareStackTrace method, replacing the default in errors.js.
36-
if (getOptionValue('--enable-source-maps')) {
37-
const { prepareStackTrace } =
38-
require('internal/source_map/prepare_stack_trace');
39-
const { setPrepareStackTraceCallback } = internalBinding('errors');
40-
setPrepareStackTraceCallback(prepareStackTrace);
41-
}
4234

4335
setupDebugEnv();
4436

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ module.exports = {
5959

6060
const { NativeModule } = require('internal/bootstrap/loaders');
6161
const {
62+
getSourceMapsEnabled,
6263
maybeCacheSourceMap,
6364
rekeySourceMap
6465
} = require('internal/source_map/source_map_cache');
@@ -81,7 +82,6 @@ const {
8182
loadNativeModule
8283
} = require('internal/modules/cjs/helpers');
8384
const { getOptionValue } = require('internal/options');
84-
const enableSourceMaps = getOptionValue('--enable-source-maps');
8585
const preserveSymlinks = getOptionValue('--preserve-symlinks');
8686
const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
8787
// Do not eagerly grab .manifest, it may be in TDZ
@@ -758,7 +758,7 @@ Module._load = function(request, parent, isMain) {
758758
// Intercept exceptions that occur during the first tick and rekey them
759759
// on error instance rather than module instance (which will immediately be
760760
// garbage collected).
761-
if (enableSourceMaps) {
761+
if (getSourceMapsEnabled()) {
762762
try {
763763
module.load(filename);
764764
} catch (err) {

‎lib/internal/source_map/prepare_stack_trace.js

+54-3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ const {
77
let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
88
debug = fn;
99
});
10+
const { getStringWidth } = require('internal/util/inspect');
11+
const { readFileSync } = require('fs');
1012
const { findSourceMap } = require('internal/source_map/source_map_cache');
1113
const {
1214
kNoOverride,
@@ -36,7 +38,17 @@ const prepareStackTrace = (globalThis, error, trace) => {
3638
if (trace.length === 0) {
3739
return errorString;
3840
}
41+
42+
let errorSource = '';
43+
let firstSource;
44+
let firstLine;
45+
let firstColumn;
3946
const preparedTrace = trace.map((t, i) => {
47+
if (i === 0) {
48+
firstLine = t.getLineNumber();
49+
firstColumn = t.getColumnNumber();
50+
firstSource = t.getFileName();
51+
}
4052
let str = i !== 0 ? '\n at ' : '';
4153
str = `${str}${t}`;
4254
try {
@@ -51,18 +63,57 @@ const prepareStackTrace = (globalThis, error, trace) => {
5163
} = sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
5264
if (originalSource && originalLine !== undefined &&
5365
originalColumn !== undefined) {
54-
str +=
55-
`\n -> ${originalSource.replace('file://', '')}:${originalLine + 1}:${originalColumn + 1}`;
66+
const originalSourceNoScheme = originalSource
67+
.replace(/^file:\/\//, '');
68+
if (i === 0) {
69+
firstLine = originalLine + 1;
70+
firstColumn = originalColumn + 1;
71+
firstSource = originalSourceNoScheme;
72+
// Show error in original source context to help user pinpoint it:
73+
errorSource = getErrorSource(firstSource, firstLine, firstColumn);
74+
}
75+
// Show both original and transpiled stack trace information:
76+
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
77+
`${originalColumn + 1}`;
5678
}
5779
}
5880
} catch (err) {
5981
debug(err.stack);
6082
}
6183
return str;
6284
});
63-
return `${errorString}\n at ${preparedTrace.join('')}`;
85+
return `${errorSource}${errorString}\n at ${preparedTrace.join('')}`;
6486
};
6587

88+
// Places a snippet of code from where the exception was originally thrown
89+
// above the stack trace. This logic is modeled after GetErrorSource in
90+
// node_errors.cc.
91+
function getErrorSource(firstSource, firstLine, firstColumn) {
92+
let exceptionLine = '';
93+
let source;
94+
try {
95+
source = readFileSync(firstSource, 'utf8');
96+
} catch (err) {
97+
debug(err);
98+
return exceptionLine;
99+
}
100+
const lines = source.split(/\r?\n/, firstLine);
101+
const line = lines[firstLine - 1];
102+
if (!line) return exceptionLine;
103+
104+
// Display ^ in appropriate position, regardless of whether tabs or
105+
// spaces are used:
106+
let prefix = '';
107+
for (const character of line.slice(0, firstColumn)) {
108+
prefix += (character === '\t') ? '\t' :
109+
' '.repeat(getStringWidth(character));
110+
}
111+
prefix = prefix.slice(0, -1); // The last character is the '^'.
112+
113+
exceptionLine = `${firstSource}:${firstLine}\n${line}\n${prefix}^\n\n`;
114+
return exceptionLine;
115+
}
116+
66117
module.exports = {
67118
prepareStackTrace,
68119
};

‎lib/internal/source_map/source_map_cache.js

+22-5
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,28 @@ const { fileURLToPath, URL } = require('url');
4141
let Module;
4242
let SourceMap;
4343

44-
let experimentalSourceMaps;
45-
function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
46-
if (experimentalSourceMaps === undefined) {
47-
experimentalSourceMaps = getOptionValue('--enable-source-maps');
44+
let sourceMapsEnabled;
45+
function getSourceMapsEnabled() {
46+
if (sourceMapsEnabled === undefined) {
47+
sourceMapsEnabled = getOptionValue('--enable-source-maps');
48+
if (sourceMapsEnabled) {
49+
const {
50+
enableSourceMaps,
51+
setPrepareStackTraceCallback
52+
} = internalBinding('errors');
53+
const {
54+
prepareStackTrace
55+
} = require('internal/source_map/prepare_stack_trace');
56+
setPrepareStackTraceCallback(prepareStackTrace);
57+
enableSourceMaps();
58+
}
4859
}
49-
if (!(process.env.NODE_V8_COVERAGE || experimentalSourceMaps)) return;
60+
return sourceMapsEnabled;
61+
}
62+
63+
function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
64+
const sourceMapsEnabled = getSourceMapsEnabled();
65+
if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return;
5066
let basePath;
5167
try {
5268
filename = normalizeReferrerURL(filename);
@@ -250,6 +266,7 @@ function findSourceMap(uri, error) {
250266

251267
module.exports = {
252268
findSourceMap,
269+
getSourceMapsEnabled,
253270
maybeCacheSourceMap,
254271
rekeySourceMap,
255272
sourceMapCacheToObject,

‎src/env-inl.h

+8
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,14 @@ void Environment::set_filehandle_close_warning(bool on) {
837837
emit_filehandle_warning_ = on;
838838
}
839839

840+
void Environment::set_source_maps_enabled(bool on) {
841+
source_maps_enabled_ = on;
842+
}
843+
844+
bool Environment::source_maps_enabled() const {
845+
return source_maps_enabled_;
846+
}
847+
840848
inline uint64_t Environment::thread_id() const {
841849
return thread_id_;
842850
}

‎src/env.h

+5
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,9 @@ class Environment : public MemoryRetainer {
10371037
inline bool filehandle_close_warning() const;
10381038
inline void set_filehandle_close_warning(bool on);
10391039

1040+
inline void set_source_maps_enabled(bool on);
1041+
inline bool source_maps_enabled() const;
1042+
10401043
inline void ThrowError(const char* errmsg);
10411044
inline void ThrowTypeError(const char* errmsg);
10421045
inline void ThrowRangeError(const char* errmsg);
@@ -1257,6 +1260,8 @@ class Environment : public MemoryRetainer {
12571260
bool emit_env_nonstring_warning_ = true;
12581261
bool emit_err_name_warning_ = true;
12591262
bool emit_filehandle_warning_ = true;
1263+
bool source_maps_enabled_ = false;
1264+
12601265
size_t async_callback_scope_depth_ = 0;
12611266
std::vector<double> destroy_async_id_list_;
12621267

‎src/node_errors.cc

+16
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ static std::string GetErrorSource(Isolate* isolate,
5656
node::Utf8Value encoded_source(isolate, source_line_maybe.ToLocalChecked());
5757
std::string sourceline(*encoded_source, encoded_source.length());
5858

59+
// If source maps have been enabled, the exception line will instead be
60+
// added in the JavaScript context:
61+
Environment* env = Environment::GetCurrent(isolate);
62+
const bool has_source_map_url =
63+
!message->GetScriptOrigin().SourceMapUrl().IsEmpty();
64+
if (has_source_map_url && env->source_maps_enabled()) {
65+
*added_exception_line = false;
66+
return sourceline;
67+
}
68+
5969
if (sourceline.find("node-do-not-add-exception-line") != std::string::npos) {
6070
*added_exception_line = false;
6171
return sourceline;
@@ -801,6 +811,11 @@ void SetPrepareStackTraceCallback(const FunctionCallbackInfo<Value>& args) {
801811
env->set_prepare_stack_trace_callback(args[0].As<Function>());
802812
}
803813

814+
static void EnableSourceMaps(const FunctionCallbackInfo<Value>& args) {
815+
Environment* env = Environment::GetCurrent(args);
816+
env->set_source_maps_enabled(true);
817+
}
818+
804819
static void SetEnhanceStackForFatalException(
805820
const FunctionCallbackInfo<Value>& args) {
806821
Environment* env = Environment::GetCurrent(args);
@@ -839,6 +854,7 @@ void Initialize(Local<Object> target,
839854
Environment* env = Environment::GetCurrent(context);
840855
env->SetMethod(
841856
target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback);
857+
env->SetMethod(target, "enableSourceMaps", EnableSourceMaps);
842858
env->SetMethod(target,
843859
"setEnhanceStackForFatalException",
844860
SetEnhanceStackForFatalException);

‎test/fixtures/source-map/icu.js

+13
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/source-map/icu.jsx

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const React = {
2+
createElement: () => {
3+
("あ 🐕 🐕", throw Error("an error"));
4+
}
5+
};
6+
7+
const profile = (
8+
<div>
9+
<img src="avatar.png" className="profile" />
10+
<h3>{["hello"]}</h3>
11+
</div>
12+
);

‎test/fixtures/source-map/tabs.coffee

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Assignment:
2+
number = 42
3+
opposite = true
4+
5+
# Conditions:
6+
number = -42 if opposite
7+
8+
# Functions:
9+
square = (x) -> x * x
10+
11+
# Arrays:
12+
list = [1, 2, 3, 4, 5]
13+
14+
# Objects:
15+
math =
16+
root: Math.sqrt
17+
square: square
18+
cube: (x) -> x * square x
19+
20+
# Splats:
21+
race = (winner, runners...) ->
22+
print winner, runners
23+
24+
# Existence:
25+
if true
26+
alert "I knew it!"
27+
28+
# Array comprehensions:
29+
cubes = (math.cube num for num in list)

‎test/fixtures/source-map/tabs.js

+56
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/message/source_map_reference_error_tabs.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// Flags: --enable-source-maps
2+
3+
'use strict';
4+
require('../common');
5+
require('../fixtures/source-map/tabs.js');

‎test/message/source_map_reference_error_tabs.out

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
*tabs.coffee:26
2+
alert "I knew it!"
3+
^
4+
5+
ReferenceError: alert is not defined
6+
at Object.<anonymous> (*tabs.coffee:39:5)
7+
-> *tabs.coffee:26:2
8+
at Object.<anonymous> (*tabs.coffee:53:4)
9+
-> *tabs.coffee:1:14
10+
at Module._compile (internal/modules/cjs/loader.js:*
11+
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*
12+
at Module.load (internal/modules/cjs/loader.js:*
13+
at Function.Module._load (internal/modules/cjs/loader.js:*
14+
at Module.require (internal/modules/cjs/loader.js:*
15+
at require (internal/modules/cjs/helpers.js:*
16+
at Object.<anonymous> (*source_map_reference_error_tabs.js:*
17+
at Module._compile (internal/modules/cjs/loader.js:*

‎test/message/source_map_throw_catch.out

+3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
reachable
2+
*typescript-throw.ts:18
3+
throw Error('an exception');
4+
^
25
Error: an exception
36
at branch (*typescript-throw.js:20:15)
47
-> *typescript-throw.ts:18:11

‎test/message/source_map_throw_first_tick.out

+3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
reachable
2+
*typescript-throw.ts:18
3+
throw Error('an exception');
4+
^
25
Error: an exception
36
at branch (*typescript-throw.js:20:15)
47
-> *typescript-throw.ts:18:11

0 commit comments

Comments
 (0)