From 74a69abd12fb6c906d8618ffd6c31293e69d2327 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Mon, 16 Sep 2019 15:32:15 -0500 Subject: [PATCH] lib: stop using prepareStackTrace PR-URL: https://github.com/nodejs/node/pull/29777 Reviewed-By: Ben Coe Reviewed-By: Colin Ihrig --- lib/assert.js | 21 +++++++------- lib/internal/errors.js | 19 +++++++++++-- lib/internal/util.js | 13 ++++----- lib/repl.js | 64 ++++++++++++++++++++---------------------- 4 files changed, 65 insertions(+), 52 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index a1b22d3e462e59..562c9628c9dacb 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -23,13 +23,16 @@ const { Object } = primordials; const { Buffer } = require('buffer'); -const { codes: { - ERR_AMBIGUOUS_ARGUMENT, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE, - ERR_INVALID_RETURN_VALUE, - ERR_MISSING_ARGS -} } = require('internal/errors'); +const { + codes: { + ERR_AMBIGUOUS_ARGUMENT, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, + ERR_INVALID_RETURN_VALUE, + ERR_MISSING_ARGS, + }, + overrideStackTrace, +} = require('internal/errors'); const AssertionError = require('internal/assert/assertion_error'); const { openSync, closeSync, readSync } = require('fs'); const { inspect } = require('internal/util/inspect'); @@ -261,10 +264,8 @@ function getErrMessage(message, fn) { Error.captureStackTrace(err, fn); Error.stackTraceLimit = tmpLimit; - const tmpPrepare = Error.prepareStackTrace; - Error.prepareStackTrace = (_, stack) => stack; + overrideStackTrace.set(err, (_, stack) => stack); const call = err.stack[0]; - Error.prepareStackTrace = tmpPrepare; const filename = call.getFileName(); const line = call.getLineNumber() - 1; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8090b4d66cb3d7..6c7c94c484add3 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -21,9 +21,18 @@ const { kMaxLength } = internalBinding('buffer'); const MainContextError = Error; const ErrorToString = Error.prototype.toString; -// Polyfill of V8's Error.prepareStackTrace API. -// https://crbug.com/v8/7848 +const overrideStackTrace = new WeakMap(); const prepareStackTrace = (globalThis, error, trace) => { + // API for node internals to override error stack formatting + // without interfering with userland code. + if (overrideStackTrace.has(error)) { + const f = overrideStackTrace.get(error); + overrideStackTrace.delete(error); + return f(error, trace); + } + + // Polyfill of V8's Error.prepareStackTrace API. + // https://crbug.com/v8/7848 // `globalThis` is the global that contains the constructor which // created `error`. if (typeof globalThis.Error.prepareStackTrace === 'function') { @@ -36,6 +45,11 @@ 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; @@ -675,6 +689,7 @@ module.exports = { // This is exported only to facilitate testing. E, prepareStackTrace, + overrideStackTrace, kEnhanceStackBeforeInspector, fatalExceptionStackEnhancers }; diff --git a/lib/internal/util.js b/lib/internal/util.js index e13cce92fff3eb..58502f3b7a7a93 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -7,7 +7,8 @@ const { ERR_NO_CRYPTO, ERR_UNKNOWN_SIGNAL }, - uvErrmapGet + uvErrmapGet, + overrideStackTrace, } = require('internal/errors'); const { signals } = internalBinding('constants').os; const { @@ -338,15 +339,13 @@ function isInsideNodeModules() { // side-effect-free. Since this is currently only used for a deprecated API, // the perf implications should be okay. getStructuredStack = runInNewContext(`(function() { - Error.prepareStackTrace = function(err, trace) { - return trace; - }; Error.stackTraceLimit = Infinity; - return function structuredStack() { - return new Error().stack; + const e = new Error(); + overrideStackTrace.set(e, (err, trace) => trace); + return e.stack; }; - })()`, {}, { filename: 'structured-stack' }); + })()`, { overrideStackTrace }, { filename: 'structured-stack' }); } const stack = getStructuredStack(); diff --git a/lib/repl.js b/lib/repl.js index ef19d257916ab6..e0f20ff5bf0447 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -69,12 +69,15 @@ const CJSModule = require('internal/modules/cjs/loader'); const domain = require('domain'); const debug = require('internal/util/debuglog').debuglog('repl'); const { - ERR_CANNOT_WATCH_SIGINT, - ERR_INVALID_ARG_TYPE, - ERR_INVALID_REPL_EVAL_CONFIG, - ERR_INVALID_REPL_INPUT, - ERR_SCRIPT_EXECUTION_INTERRUPTED -} = require('internal/errors').codes; + codes: { + ERR_CANNOT_WATCH_SIGINT, + ERR_INVALID_ARG_TYPE, + ERR_INVALID_REPL_EVAL_CONFIG, + ERR_INVALID_REPL_INPUT, + ERR_SCRIPT_EXECUTION_INTERRUPTED, + }, + overrideStackTrace, +} = require('internal/errors'); const { sendInspectorCommand } = require('internal/util/inspector'); const experimentalREPLAwait = require('internal/options').getOptionValue( '--experimental-repl-await' @@ -473,10 +476,29 @@ function REPLServer(prompt, let errStack = ''; if (typeof e === 'object' && e !== null) { - const pstrace = Error.prepareStackTrace; - Error.prepareStackTrace = prepareStackTrace(pstrace); + overrideStackTrace.set(e, (error, stackFrames) => { + let frames; + if (typeof stackFrames === 'object') { + // Search from the bottom of the call stack to + // find the first frame with a null function name + const idx = stackFrames + .reverse() + .findIndex((frame) => frame.getFunctionName() === null); + // If found, get rid of it and everything below it + frames = stackFrames.splice(idx + 1); + } else { + frames = stackFrames; + } + // FIXME(devsnek): this is inconsistent with the checks + // that the real prepareStackTrace dispatch uses in + // lib/internal/errors.js. + if (typeof Error.prepareStackTrace === 'function') { + return Error.prepareStackTrace(error, frames); + } + frames.push(error); + return frames.reverse().join('\n at '); + }); decorateErrorStack(e); - Error.prepareStackTrace = pstrace; if (e.domainThrown) { delete e.domain; @@ -590,30 +612,6 @@ function REPLServer(prompt, } } - function filterInternalStackFrames(structuredStack) { - // Search from the bottom of the call stack to - // find the first frame with a null function name - if (typeof structuredStack !== 'object') - return structuredStack; - const idx = structuredStack.reverse().findIndex( - (frame) => frame.getFunctionName() === null); - - // If found, get rid of it and everything below it - structuredStack = structuredStack.splice(idx + 1); - return structuredStack; - } - - function prepareStackTrace(fn) { - return (error, stackFrames) => { - const frames = filterInternalStackFrames(stackFrames); - if (fn) { - return fn(error, frames); - } - frames.push(error); - return frames.reverse().join('\n at '); - }; - } - function _parseREPLKeyword(keyword, rest) { const cmd = this.commands[keyword]; if (cmd) {