diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index c76add4621b614..3a6a25f05c3b6f 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -31,14 +31,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 223c6b21e39f48..f7ccc5302ad0b2 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -59,6 +59,7 @@ module.exports = { const { NativeModule } = require('internal/bootstrap/loaders'); const { + getSourceMapsEnabled, maybeCacheSourceMap, rekeySourceMap } = require('internal/source_map/source_map_cache'); @@ -81,7 +82,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'); // Do not eagerly grab .manifest, it may be in TDZ @@ -758,7 +758,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 8a10470d077fed..23e117b677b74a 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -7,6 +7,8 @@ const { let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => { debug = fn; }); +const { getStringWidth } = require('internal/util/inspect'); +const { readFileSync } = require('fs'); const { findSourceMap } = require('internal/source_map/source_map_cache'); const { kNoOverride, @@ -36,7 +38,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 { @@ -51,8 +63,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) { @@ -60,9 +82,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 8952940920b88f..db7a19e275951c 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -41,12 +41,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); @@ -250,6 +266,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 c1853f81b68bd2..e713ea64ef2b19 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -837,6 +837,14 @@ void Environment::set_filehandle_close_warning(bool on) { emit_filehandle_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 f89365a1aa7ffa..7c57a4b0017e50 100644 --- a/src/env.h +++ b/src/env.h @@ -1037,6 +1037,9 @@ class Environment : public MemoryRetainer { inline bool filehandle_close_warning() const; inline void set_filehandle_close_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); @@ -1257,6 +1260,8 @@ class Environment : public MemoryRetainer { bool emit_env_nonstring_warning_ = true; bool emit_err_name_warning_ = true; bool emit_filehandle_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 6961e748c06ad9..3f2cc4efef2df8 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -56,6 +56,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; @@ -801,6 +811,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); @@ -839,6 +854,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