From d03cb185e8670ca51b7bc074fda3bcabbe08090b Mon Sep 17 00:00:00 2001 From: Momtchil Momtchev Date: Wed, 14 Oct 2020 17:39:21 +0200 Subject: [PATCH] errors: eliminate all overhead for hidden calls Eliminate all overhead for function calls that are to be hidden from the stack traces at the expense of reduced performance for the error case Fixes: https://github.com/nodejs/node/issues/35386 --- lib/internal/errors.js | 272 +++++++++++++++++++++-------------------- 1 file changed, 137 insertions(+), 135 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8a7e744c5fe667..00a01611ab9cd0 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -23,7 +23,6 @@ const { ArrayPrototypeSplice, ArrayPrototypeUnshift, Error, - ErrorCaptureStackTrace, ErrorPrototypeToString, JSONStringify, MathAbs, @@ -75,6 +74,7 @@ const kTypes = [ const MainContextError = Error; const overrideStackTrace = new SafeWeakMap(); const kNoOverride = Symbol('kNoOverride'); +let userStackTraceLimit = Error.stackTraceLimit; const prepareStackTrace = (globalThis, error, trace) => { // API for node internals to override error stack formatting // without interfering with userland code. @@ -84,6 +84,16 @@ 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; + } + } + if (trace.length > userStackTraceLimit) + trace.splice(userStackTraceLimit); + const globalOverride = maybeOverridePrepareStackTrace(globalThis, error, trace); if (globalOverride !== kNoOverride) return globalOverride; @@ -118,8 +128,6 @@ const maybeOverridePrepareStackTrace = (globalThis, error, trace) => { return kNoOverride; }; -let excludedStackFn; - // Lazily loaded let util; let assert; @@ -147,6 +155,27 @@ function lazyBuffer() { return buffer; } +const addCodeToName = hideStackFrames(function(err, name, code) { + // Set the stack + err = captureLargerStackTrace(err); + // Add the error code to the name to include it in the stack trace. + err.name = `${name} [${code}]`; + // Access the stack to generate the error message including the error code + // from the name. + err.stack; + // Reset the name to the actual name. + if (name === 'SystemError') { + ObjectDefineProperty(err, 'name', { + value: name, + enumerable: false, + writable: true, + configurable: true + }); + } else { + delete err.name; + } +}); + // A specialized Error that includes an additional info property with // additional information about the error condition. // It has the properties present in a UVException but with a custom error @@ -157,18 +186,14 @@ function lazyBuffer() { // and may have .path and .dest. class SystemError extends Error { constructor(key, context) { - if (excludedStackFn === undefined) { - super(); - } else { - const limit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - super(); - // Reset the limit and setting the name property. - Error.stackTraceLimit = limit; - } + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + super(); + // Reset the limit and setting the name property. + Error.stackTraceLimit = limit; const prefix = getMessage(key, [], this); let message = `${prefix}: ${context.syscall} returned ` + - `${context.code} (${context.message})`; + `${context.code} (${context.message})`; if (context.path !== undefined) message += ` ${context.path}`; @@ -273,16 +298,11 @@ function makeSystemErrorWithCode(key) { function makeNodeErrorWithCode(Base, key) { return function NodeError(...args) { - let error; - if (excludedStackFn === undefined) { - error = new Base(); - } else { - const limit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - error = new Base(); - // Reset the limit and setting the name property. - Error.stackTraceLimit = limit; - } + const limit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + const error = new Base(); + // Reset the limit and setting the name property. + Error.stackTraceLimit = limit; const message = getMessage(key, args, error); ObjectDefineProperty(error, 'message', { value: message, @@ -306,44 +326,9 @@ function makeNodeErrorWithCode(Base, key) { // This function removes unnecessary frames from Node.js core errors. function hideStackFrames(fn) { - return function hidden(...args) { - // Make sure the most outer `hideStackFrames()` function is used. - let setStackFn = false; - if (excludedStackFn === undefined) { - excludedStackFn = hidden; - setStackFn = true; - } - try { - return fn(...args); - } finally { - if (setStackFn === true) { - excludedStackFn = undefined; - } - } - }; -} - -function addCodeToName(err, name, code) { - // Set the stack - if (excludedStackFn !== undefined) { - ErrorCaptureStackTrace(err, excludedStackFn); - } - // Add the error code to the name to include it in the stack trace. - err.name = `${name} [${code}]`; - // Access the stack to generate the error message including the error code - // from the name. - err.stack; // eslint-disable-line no-unused-expressions - // Reset the name to the actual name. - if (name === 'SystemError') { - ObjectDefineProperty(err, 'name', { - value: name, - enumerable: false, - writable: true, - configurable: true - }); - } else { - delete err.name; - } + const hidden = '__node_internal_hidden_' + fn.name; + ObjectDefineProperty(fn, 'name', { value: hidden }); + return fn; } // Utility function for registering the error codes. Only used here. Exported @@ -413,6 +398,16 @@ function uvErrmapGet(name) { return uvBinding.errmap.get(name); } +function captureLargerStackTrace(err) { + userStackTraceLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 20; + // eslint-disable-next-line no-restricted-syntax + Error.captureStackTrace(err); + // Reset the limit and setting the name property. + Error.stackTraceLimit = userStackTraceLimit; + + return err; +} /** * This creates an error compatible with errors produced in the C++ @@ -423,8 +418,8 @@ function uvErrmapGet(name) { * @param {Object} ctx * @returns {Error} */ -function uvException(ctx) { - const [ code, uvmsg ] = uvErrmapGet(ctx.errno) || uvUnmappedError; +const uvException = hideStackFrames(function(ctx) { + const [code, uvmsg] = uvErrmapGet(ctx.errno) || uvUnmappedError; let message = `${code}: ${ctx.message || uvmsg}, ${ctx.syscall}`; let path; @@ -463,9 +458,10 @@ function uvException(ctx) { if (dest) { err.dest = dest; } - ErrorCaptureStackTrace(err, excludedStackFn || uvException); + + captureLargerStackTrace(err); return err; -} +}); /** * This creates an error compatible with errors produced in the C++ @@ -478,35 +474,37 @@ function uvException(ctx) { * @param {number} [port] * @returns {Error} */ -function uvExceptionWithHostPort(err, syscall, address, port) { - const [ code, uvmsg ] = uvErrmapGet(err) || uvUnmappedError; - const message = `${syscall} ${code}: ${uvmsg}`; - let details = ''; - - if (port && port > 0) { - details = ` ${address}:${port}`; - } else if (address) { - details = ` ${address}`; - } +const uvExceptionWithHostPort = + hideStackFrames(function(err, syscall, address, port) { + const [code, uvmsg] = uvErrmapGet(err) || uvUnmappedError; + const message = `${syscall} ${code}: ${uvmsg}`; + let details = ''; + + if (port && port > 0) { + details = ` ${address}:${port}`; + } else if (address) { + details = ` ${address}`; + } - // Reducing the limit improves the performance significantly. We do not lose - // the stack frames due to the `captureStackTrace()` function that is called - // later. - const tmpLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - // eslint-disable-next-line no-restricted-syntax - const ex = new Error(`${message}${details}`); - Error.stackTraceLimit = tmpLimit; - ex.code = code; - ex.errno = err; - ex.syscall = syscall; - ex.address = address; - if (port) { - ex.port = port; - } - ErrorCaptureStackTrace(ex, excludedStackFn || uvExceptionWithHostPort); - return ex; -} + // Reducing the limit improves the performance significantly. We do not + // loose the stack frames due to the `captureStackTrace()` function that + // is called later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + // eslint-disable-next-line no-restricted-syntax + const ex = new Error(`${message}${details}`); + Error.stackTraceLimit = tmpLimit; + ex.code = code; + ex.errno = err; + ex.syscall = syscall; + ex.address = address; + if (port) { + ex.port = port; + } + + captureLargerStackTrace(ex); + return ex; + }); /** * This used to be util._errnoException(). @@ -516,7 +514,7 @@ function uvExceptionWithHostPort(err, syscall, address, port) { * @param {string} [original] * @returns {Error} */ -function errnoException(err, syscall, original) { +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 @@ -526,14 +524,17 @@ function errnoException(err, syscall, original) { 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; - ErrorCaptureStackTrace(ex, excludedStackFn || errnoException); - return ex; -} + + return captureLargerStackTrace(ex); +}); /** * Deprecated, new function is `uvExceptionWithHostPort()` @@ -546,41 +547,42 @@ function errnoException(err, syscall, original) { * @param {string} [additional] * @returns {Error} */ -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 - // is no longer exposed to user land. - if (util === undefined) util = require('util'); - const code = util.getSystemErrorName(err); - let details = ''; - if (port && port > 0) { - details = ` ${address}:${port}`; - } else if (address) { - details = ` ${address}`; - } - if (additional) { - details += ` - Local (${additional})`; - } +const exceptionWithHostPort = + hideStackFrames(function(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 + // is no longer exposed to user land. + if (util === undefined) util = require('util'); + const code = util.getSystemErrorName(err); + let details = ''; + if (port && port > 0) { + details = ` ${address}:${port}`; + } else if (address) { + details = ` ${address}`; + } + if (additional) { + details += ` - Local (${additional})`; + } - // Reducing the limit improves the performance significantly. We do not lose - // the stack frames due to the `captureStackTrace()` function that is called - // later. - const tmpLimit = Error.stackTraceLimit; - Error.stackTraceLimit = 0; - // eslint-disable-next-line no-restricted-syntax - const ex = new Error(`${syscall} ${code}${details}`); - Error.stackTraceLimit = tmpLimit; - ex.errno = err; - ex.code = code; - ex.syscall = syscall; - ex.address = address; - if (port) { - ex.port = port; - } - ErrorCaptureStackTrace(ex, excludedStackFn || exceptionWithHostPort); - return ex; -} + // Reducing the limit improves the performance significantly. We do not + // loose the stack frames due to the `captureStackTrace()` function that + // is called later. + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + // eslint-disable-next-line no-restricted-syntax + const ex = new Error(`${syscall} ${code}${details}`); + Error.stackTraceLimit = tmpLimit; + ex.errno = err; + ex.code = code; + ex.syscall = syscall; + ex.address = address; + if (port) { + ex.port = port; + } + + return captureLargerStackTrace(ex); + }); /** * @param {number|string} code - A libuv error number or a c-ares error code @@ -588,7 +590,7 @@ function exceptionWithHostPort(err, syscall, address, port, additional) { * @param {string} [hostname] * @returns {Error} */ -function dnsException(code, syscall, hostname) { +const dnsException = hideStackFrames(function(code, syscall, hostname) { let errno; // If `code` is of type number, it is a libuv error number, else it is a // c-ares error code. @@ -622,9 +624,9 @@ function dnsException(code, syscall, hostname) { if (hostname) { ex.hostname = hostname; } - ErrorCaptureStackTrace(ex, excludedStackFn || dnsException); - return ex; -} + + return captureLargerStackTrace(ex); +}); function connResetException(msg) { // eslint-disable-next-line no-restricted-syntax