From 19a197966c49f37e870401ea230dfd75b3274c77 Mon Sep 17 00:00:00 2001 From: bcoe Date: Tue, 31 Dec 2019 09:24:43 -0800 Subject: [PATCH 1/4] errors: support prepareSourceMap with source-maps --- .../source_map/prepare_stack_trace.js | 6 ++++++ .../test-error-prepare-stack-trace.js | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 test/parallel/test-error-prepare-stack-trace.js diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index d6c5fce60a29e3..e5df8951e15e1a 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -17,6 +17,12 @@ const prepareStackTrace = (globalThis, error, trace) => { return f(error, trace); } + // `globalThis` is the global that contains the constructor which + // created `error`. + if (typeof globalThis.Error.prepareStackTrace === 'function') { + return globalThis.Error.prepareStackTrace(error, trace); + } + const { SourceMap } = require('internal/source_map/source_map'); const errorString = ErrorToString.call(error); diff --git a/test/parallel/test-error-prepare-stack-trace.js b/test/parallel/test-error-prepare-stack-trace.js new file mode 100644 index 00000000000000..2ace9c8d71491d --- /dev/null +++ b/test/parallel/test-error-prepare-stack-trace.js @@ -0,0 +1,19 @@ +// Flags: --enable-source-maps +'use strict'; + +require('../common'); +const assert = require('assert'); + +// Error.prepareStackTrace() can be overridden with source maps enabled. +{ + let prepareCalled = false; + Error.prepareStackTrace = (_error, trace) => { + prepareCalled = true; + }; + try { + throw new Error('foo'); + } catch (err) { + err.stack; + } + assert(prepareCalled); +} From ece5a2f4c63e5a807963e859995356d7627f085f Mon Sep 17 00:00:00 2001 From: bcoe Date: Tue, 31 Dec 2019 14:05:34 -0800 Subject: [PATCH 2/4] chore: address code review --- lib/internal/errors.js | 30 ++++++++++++------- .../source_map/prepare_stack_trace.js | 13 ++++---- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8b0a7a0d7aab66..3ee3a2b464e468 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -52,6 +52,23 @@ const prepareStackTrace = (globalThis, error, trace) => { return f(error, trace); } + const globalOverride = + maybeOverridePrepareStackTrace(globalThis, error, trace); + if (globalOverride) return globalOverride; + + // Normal error formatting: + // + // Error: Message + // at function (file) + // at file + const errorString = ErrorToString.call(error); + if (trace.length === 0) { + return errorString; + } + return `${errorString}\n at ${trace.join('\n at ')}`; +}; + +const maybeOverridePrepareStackTrace = (globalThis, error, trace) => { // Polyfill of V8's Error.prepareStackTrace API. // https://crbug.com/v8/7848 // `globalThis` is the global that contains the constructor which @@ -66,19 +83,9 @@ const prepareStackTrace = (globalThis, error, trace) => { return MainContextError.prepareStackTrace(error, trace); } - // Normal error formatting: - // - // Error: Message - // at function (file) - // at file - const errorString = ErrorToString.call(error); - if (trace.length === 0) { - return errorString; - } - return `${errorString}\n at ${trace.join('\n at ')}`; + return null; }; - let excludedStackFn; // Lazily loaded @@ -692,6 +699,7 @@ module.exports = { // This is exported only to facilitate testing. E, prepareStackTrace, + maybeOverridePrepareStackTrace, overrideStackTrace, kEnhanceStackBeforeInspector, fatalExceptionStackEnhancers diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index e5df8951e15e1a..978c38a0cba5eb 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -2,7 +2,10 @@ const debug = require('internal/util/debuglog').debuglog('source_map'); const { findSourceMap } = require('internal/source_map/source_map_cache'); -const { overrideStackTrace } = require('internal/errors'); +const { + overrideStackTrace, + maybeOverridePrepareStackTrace +} = require('internal/errors'); // Create a prettified stacktrace, inserting context from source maps // if possible. @@ -17,11 +20,9 @@ const prepareStackTrace = (globalThis, error, trace) => { return f(error, trace); } - // `globalThis` is the global that contains the constructor which - // created `error`. - if (typeof globalThis.Error.prepareStackTrace === 'function') { - return globalThis.Error.prepareStackTrace(error, trace); - } + const globalOverride = + maybeOverridePrepareStackTrace(globalThis, error, trace); + if (globalOverride) return globalOverride; const { SourceMap } = require('internal/source_map/source_map'); const errorString = ErrorToString.call(error); From 052de05a92c0f4a8cc8f2d4db84914291ebe15bc Mon Sep 17 00:00:00 2001 From: bcoe Date: Tue, 31 Dec 2019 14:31:33 -0800 Subject: [PATCH 3/4] chore: address review --- lib/internal/errors.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 3ee3a2b464e468..226b8e2b352948 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -43,6 +43,7 @@ const { kMaxLength } = internalBinding('buffer'); const MainContextError = Error; const ErrorToString = Error.prototype.toString; const overrideStackTrace = new WeakMap(); +const kNoOverride = Symbol('kNoOverride'); const prepareStackTrace = (globalThis, error, trace) => { // API for node internals to override error stack formatting // without interfering with userland code. @@ -54,7 +55,7 @@ const prepareStackTrace = (globalThis, error, trace) => { const globalOverride = maybeOverridePrepareStackTrace(globalThis, error, trace); - if (globalOverride) return globalOverride; + if (globalOverride !== kNoOverride) return globalOverride; // Normal error formatting: // @@ -83,7 +84,7 @@ const maybeOverridePrepareStackTrace = (globalThis, error, trace) => { return MainContextError.prepareStackTrace(error, trace); } - return null; + return kNoOverride; }; let excludedStackFn; From c9394db3501d9919ffcbb201847c3aea8df8230c Mon Sep 17 00:00:00 2001 From: bcoe Date: Tue, 31 Dec 2019 15:38:02 -0800 Subject: [PATCH 4/4] chore: whoops, need to export kNoOverride too --- lib/internal/errors.js | 1 + lib/internal/source_map/prepare_stack_trace.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 226b8e2b352948..a0f3f26d5d34be 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -699,6 +699,7 @@ module.exports = { SystemError, // This is exported only to facilitate testing. E, + kNoOverride, prepareStackTrace, maybeOverridePrepareStackTrace, overrideStackTrace, diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 978c38a0cba5eb..91bedcb266be70 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -3,6 +3,7 @@ const debug = require('internal/util/debuglog').debuglog('source_map'); const { findSourceMap } = require('internal/source_map/source_map_cache'); const { + kNoOverride, overrideStackTrace, maybeOverridePrepareStackTrace } = require('internal/errors'); @@ -22,7 +23,7 @@ const prepareStackTrace = (globalThis, error, trace) => { const globalOverride = maybeOverridePrepareStackTrace(globalThis, error, trace); - if (globalOverride) return globalOverride; + if (globalOverride !== kNoOverride) return globalOverride; const { SourceMap } = require('internal/source_map/source_map'); const errorString = ErrorToString.call(error);