From c205a80e9194153f3bb4203a21ef5b3f61c8df49 Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Thu, 15 Oct 2020 16:49:03 +0200 Subject: [PATCH] errors: slights improvements for hideStackFrames As suggested by @puzpuzpuz and @BridgeAR * Do not run the full loop for user-generated exceptions * Reduce the prefix * Add function names to the functions in errors.js so that they keep their names in the debugger/profiler Refs: https://github.com/nodejs/node/pull/35644 --- lib/internal/errors.js | 92 ++++++++++++++++++++++-------------------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7688f7ca50079a..0aae8f7088c896 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -56,6 +56,7 @@ const MainContextError = Error; const overrideStackTrace = new WeakMap(); const kNoOverride = Symbol('kNoOverride'); let userStackTraceLimit = Error.stackTraceLimit; +const nodeInternalPrefix = '__node_internal_'; const prepareStackTrace = (globalThis, error, trace) => { // API for node internals to override error stack formatting // without interfering with userland code. @@ -65,15 +66,18 @@ const prepareStackTrace = (globalThis, error, trace) => { return f(error, trace); } - for (let l = trace.length - 1; l >= 0; l--) { - const fn = trace[l].getFunctionName(); - if (fn != null && fn.startsWith('__node_internal_hidden_')) { - trace.splice(0, l + 1); - break; + const firstFrame = trace[0]?.getFunctionName(); + if (firstFrame && firstFrame.startsWith(nodeInternalPrefix)) { + for (let l = trace.length - 1; l >= 0; l--) { + const fn = trace[l].getFunctionName(); + if (fn != null && fn.startsWith(nodeInternalPrefix)) { + trace.splice(0, l + 1); + break; + } } + if (trace.length > userStackTraceLimit) + trace.splice(userStackTraceLimit); } - if (trace.length > userStackTraceLimit) - trace.splice(userStackTraceLimit); const globalOverride = maybeOverridePrepareStackTrace(globalThis, error, trace); @@ -136,7 +140,7 @@ function lazyBuffer() { return buffer; } -const addCodeToName = hideStackFrames(function(err, name, code) { +const addCodeToName = hideStackFrames(function addCodeToName(err, name, code) { // Set the stack err = captureLargerStackTrace(err); // Add the error code to the name to include it in the stack trace. @@ -309,7 +313,7 @@ function makeNodeErrorWithCode(Base, key) { function hideStackFrames(fn) { // We rename the functions that will be hidden to cut off the stacktrace // at the outermost one - const hidden = '__node_internal_hidden_' + fn.name; + const hidden = nodeInternalPrefix + fn.name; ObjectDefineProperty(fn, 'name', { value: hidden }); return fn; } @@ -380,16 +384,17 @@ function uvErrmapGet(name) { return uvBinding.errmap.get(name); } -function captureLargerStackTrace(err) { - userStackTraceLimit = Error.stackTraceLimit; - Error.stackTraceLimit = Infinity; - // eslint-disable-next-line no-restricted-syntax - Error.captureStackTrace(err); - // Reset the limit and setting the name property. - Error.stackTraceLimit = userStackTraceLimit; +const captureLargerStackTrace = hideStackFrames( + function captureLargerStackTrace(err) { + userStackTraceLimit = Error.stackTraceLimit; + Error.stackTraceLimit = Infinity; + // eslint-disable-next-line no-restricted-syntax + Error.captureStackTrace(err); + // Reset the limit + Error.stackTraceLimit = userStackTraceLimit; - return err; -} + return err; + }); /** * This creates an error compatible with errors produced in the C++ @@ -400,7 +405,7 @@ function captureLargerStackTrace(err) { * @param {Object} ctx * @returns {Error} */ -const uvException = hideStackFrames(function(ctx) { +const uvException = hideStackFrames(function uvException(ctx) { const [code, uvmsg] = uvErrmapGet(ctx.errno) || uvUnmappedError; let message = `${code}: ${ctx.message || uvmsg}, ${ctx.syscall}`; @@ -455,8 +460,8 @@ const uvException = hideStackFrames(function(ctx) { * @param {number} [port] * @returns {Error} */ -const uvExceptionWithHostPort = - hideStackFrames(function(err, syscall, address, port) { +const uvExceptionWithHostPort = hideStackFrames( + function uvExceptionWithHostPort(err, syscall, address, port) { const [code, uvmsg] = uvErrmapGet(err) || uvUnmappedError; const message = `${syscall} ${code}: ${uvmsg}`; let details = ''; @@ -495,28 +500,29 @@ const uvExceptionWithHostPort = * @param {string} [original] * @returns {Error} */ -const errnoException = hideStackFrames(function(err, syscall, original) { - // TODO(joyeecheung): We have to use the type-checked - // getSystemErrorName(err) to guard against invalid arguments from users. - // This can be replaced with [ code ] = errmap.get(err) when this method - // is no longer exposed to user land. - if (util === undefined) util = require('util'); - const code = util.getSystemErrorName(err); - const message = original ? - `${syscall} ${code} ${original}` : `${syscall} ${code}`; +const errnoException = hideStackFrames( + function errnoException(err, syscall, original) { + // TODO(joyeecheung): We have to use the type-checked + // getSystemErrorName(err) to guard against invalid arguments from users. + // This can be replaced with [ code ] = errmap.get(err) when this method + // is no longer exposed to user land. + if (util === undefined) util = require('util'); + const code = util.getSystemErrorName(err); + const message = original ? + `${syscall} ${code} ${original}` : `${syscall} ${code}`; - const tmpLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - // eslint-disable-next-line no-restricted-syntax - const ex = new Error(message); - Error.stackTraceLimit = tmpLimit; - ex.errno = err; - ex.code = code; - ex.syscall = syscall; + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + // eslint-disable-next-line no-restricted-syntax + const ex = new Error(message); + Error.stackTraceLimit = tmpLimit; + ex.errno = err; + ex.code = code; + ex.syscall = syscall; - captureLargerStackTrace(ex); - return ex; -}); + captureLargerStackTrace(ex); + return ex; + }); /** * Deprecated, new function is `uvExceptionWithHostPort()` @@ -529,8 +535,8 @@ const errnoException = hideStackFrames(function(err, syscall, original) { * @param {string} [additional] * @returns {Error} */ -const exceptionWithHostPort = - hideStackFrames(function(err, syscall, address, port, additional) { +const exceptionWithHostPort = hideStackFrames( + function exceptionWithHostPort(err, syscall, address, port, additional) { // TODO(joyeecheung): We have to use the type-checked // getSystemErrorName(err) to guard against invalid arguments from users. // This can be replaced with [ code ] = errmap.get(err) when this method