diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index 3d5e0061daa8d1..f60814d2dc9e28 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -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(); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 026c0fb1489b4c..a128cb2adaff50 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -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) { diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 037a8dc53e0855..df0d79a204b29c 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -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, }; diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 06b1a2a5f52289..04926acfc1b5b2 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -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, diff --git a/src/env-inl.h b/src/env-inl.h index 3bc59acfc8beb5..97ccc24f809e5e 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -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_; } diff --git a/src/env.h b/src/env.h index ce9fb95b0c3a25..1ef085ee867f38 100644 --- a/src/env.h +++ b/src/env.h @@ -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 destroy_async_id_list_; diff --git a/src/node_errors.cc b/src/node_errors.cc index 4e13c24e15e1d0..22bc4d994b8859 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -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& args) { env->set_prepare_stack_trace_callback(args[0].As()); } +static void EnableSourceMaps(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + env->set_source_maps_enabled(true); +} + static void SetEnhanceStackForFatalException( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -840,6 +855,7 @@ void Initialize(Local target, Environment* env = Environment::GetCurrent(context); env->SetMethod( target, "setPrepareStackTraceCallback", SetPrepareStackTraceCallback); + env->SetMethod(target, "enableSourceMaps", EnableSourceMaps); env->SetMethod(target, "setEnhanceStackForFatalException", SetEnhanceStackForFatalException); diff --git a/test/fixtures/source-map/icu.js b/test/fixtures/source-map/icu.js new file mode 100644 index 00000000000000..a29f188ae86189 --- /dev/null +++ b/test/fixtures/source-map/icu.js @@ -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== diff --git a/test/fixtures/source-map/icu.jsx b/test/fixtures/source-map/icu.jsx new file mode 100644 index 00000000000000..45325058b51222 --- /dev/null +++ b/test/fixtures/source-map/icu.jsx @@ -0,0 +1,12 @@ +const React = { + createElement: () => { + ("あ 🐕 🐕", throw Error("an error")); + } +}; + +const profile = ( +
+ +

{["hello"]}

+
+); diff --git a/test/fixtures/source-map/tabs.coffee b/test/fixtures/source-map/tabs.coffee new file mode 100644 index 00000000000000..abc5baab636e8a --- /dev/null +++ b/test/fixtures/source-map/tabs.coffee @@ -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) diff --git a/test/fixtures/source-map/tabs.js b/test/fixtures/source-map/tabs.js new file mode 100644 index 00000000000000..4e8ebf149dfd16 --- /dev/null +++ b/test/fixtures/source-map/tabs.js @@ -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 diff --git a/test/message/source_map_reference_error_tabs.js b/test/message/source_map_reference_error_tabs.js new file mode 100644 index 00000000000000..f6a39d9d497449 --- /dev/null +++ b/test/message/source_map_reference_error_tabs.js @@ -0,0 +1,5 @@ +// Flags: --enable-source-maps + +'use strict'; +require('../common'); +require('../fixtures/source-map/tabs.js'); diff --git a/test/message/source_map_reference_error_tabs.out b/test/message/source_map_reference_error_tabs.out new file mode 100644 index 00000000000000..b02370ad34b6d3 --- /dev/null +++ b/test/message/source_map_reference_error_tabs.out @@ -0,0 +1,17 @@ +*tabs.coffee:26 + alert "I knew it!" + ^ + +ReferenceError: alert is not defined + at Object. (*tabs.coffee:39:5) + -> *tabs.coffee:26:2 + at Object. (*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. (*source_map_reference_error_tabs.js:* + at Module._compile (internal/modules/cjs/loader.js:* diff --git a/test/message/source_map_throw_catch.out b/test/message/source_map_throw_catch.out index 63e3eca3eff0ce..a8e71a934f5e47 100644 --- a/test/message/source_map_throw_catch.out +++ b/test/message/source_map_throw_catch.out @@ -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 diff --git a/test/message/source_map_throw_first_tick.out b/test/message/source_map_throw_first_tick.out index 7f11d9fbd969be..2532c38d6f722c 100644 --- a/test/message/source_map_throw_first_tick.out +++ b/test/message/source_map_throw_first_tick.out @@ -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 diff --git a/test/message/source_map_throw_icu.js b/test/message/source_map_throw_icu.js new file mode 100644 index 00000000000000..00298edc5ed81a --- /dev/null +++ b/test/message/source_map_throw_icu.js @@ -0,0 +1,5 @@ +// Flags: --enable-source-maps + +'use strict'; +require('../common'); +require('../fixtures/source-map/icu'); diff --git a/test/message/source_map_throw_icu.out b/test/message/source_map_throw_icu.out new file mode 100644 index 00000000000000..25043d62c1a1ea --- /dev/null +++ b/test/message/source_map_throw_icu.out @@ -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. (*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. (*source_map_throw_icu.js:* + at Module._compile (internal/modules/cjs/loader.js:* diff --git a/test/message/source_map_throw_set_immediate.out b/test/message/source_map_throw_set_immediate.out index 6b169d7e025f7e..488d5381d5510b 100644 --- a/test/message/source_map_throw_set_immediate.out +++ b/test/message/source_map_throw_set_immediate.out @@ -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