Skip to content

Commit

Permalink
errors: print original exception context
Browse files Browse the repository at this point in the history
When --enable-source-maps is set, the error context displayed
above the stack trace now shows original source rather than
transpiled.

PR-URL: #33491
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
bcoe committed May 25, 2020
1 parent 8f10bb2 commit 458677f
Show file tree
Hide file tree
Showing 18 changed files with 270 additions and 21 deletions.
8 changes: 0 additions & 8 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const {

const { NativeModule } = require('internal/bootstrap/loaders');
const {
getSourceMapsEnabled,
maybeCacheSourceMap,
rekeySourceMap
} = require('internal/source_map/source_map_cache');
Expand All @@ -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') ?
Expand Down Expand Up @@ -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) {
Expand Down
57 changes: 54 additions & 3 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -49,18 +61,57 @@ 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) {
debug(err.stack);
}
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,
};
27 changes: 22 additions & 5 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -248,6 +264,7 @@ function findSourceMap(uri, error) {

module.exports = {
findSourceMap,
getSourceMapsEnabled,
maybeCacheSourceMap,
rekeySourceMap,
sourceMapCacheToObject,
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
}
Expand Down
5 changes: 5 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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_;

Expand Down
16 changes: 16 additions & 0 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 13 additions & 0 deletions test/fixtures/source-map/icu.js

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

12 changes: 12 additions & 0 deletions test/fixtures/source-map/icu.jsx
Original file line number Diff line number Diff line change
@@ -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 changes: 29 additions & 0 deletions test/fixtures/source-map/tabs.coffee
Original file line number Diff line number Diff line change
@@ -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 changes: 56 additions & 0 deletions test/fixtures/source-map/tabs.js

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_reference_error_tabs.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/tabs.js');
17 changes: 17 additions & 0 deletions test/message/source_map_reference_error_tabs.out
Original file line number Diff line number Diff line change
@@ -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:*
3 changes: 3 additions & 0 deletions test/message/source_map_throw_catch.out
Original file line number Diff line number Diff line change
@@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/message/source_map_throw_first_tick.out
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading

0 comments on commit 458677f

Please sign in to comment.