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

PR-URL: https://github.com/nodejs/node/pull/35644
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

3 files changed, 205 insertions, 150 deletions

diff --git a/benchmark/misc/hidestackframes.js b/benchmark/misc/hidestackframes.js
new file mode 100644
index 0000000000..5b14f2d95b
--- /dev/null
+++ b/benchmark/misc/hidestackframes.js
@@ -0,0 +1,45 @@
+'use strict';
+
+const common = require('../common.js');
+
+const bench = common.createBenchmark(main, {
+  type: ['hide-stackframes-throw', 'direct-call-throw',
+         'hide-stackframes-noerr', 'direct-call-noerr'],
+  n: [10e4]
+}, {
+  flags: ['--expose-internals']
+});
+
+function main({ n, type }) {
+  const {
+    hideStackFrames,
+    codes: {
+      ERR_INVALID_ARG_TYPE,
+    },
+  } = require('internal/errors');
+
+  const testfn = (value) => {
+    if (typeof value !== 'number') {
+      throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value);
+    }
+  };
+
+  let fn = testfn;
+  if (type.startsWith('hide-stackframe'))
+    fn = hideStackFrames(testfn);
+  let value = 42;
+  if (type.endsWith('-throw'))
+    value = 'err';
+
+  bench.start();
+
+  for (let i = 0; i < n; i++) {
+    try {
+      fn(value);
+    } catch {
+      // No-op
+    }
+  }
+
+  bench.end(n);
+}
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 8a7e744c5f..0b6a95d761 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -75,6 +75,8 @@ const kTypes = [
 const MainContextError = Error;
 const overrideStackTrace = new SafeWeakMap();
 const kNoOverride = Symbol('kNoOverride');
+let userStackTraceLimit;
+const nodeInternalPrefix = '__node_internal_';
 const prepareStackTrace = (globalThis, error, trace) => {
   // API for node internals to override error stack formatting
   // without interfering with userland code.
@@ -84,6 +86,21 @@ const prepareStackTrace = (globalThis, error, trace) => {
     return f(error, trace);
   }
 
+  const firstFrame = trace[0]?.getFunctionName();
+  if (firstFrame && StringPrototypeStartsWith(firstFrame, nodeInternalPrefix)) {
+    for (let l = trace.length - 1; l >= 0; l--) {
+      const fn = trace[l]?.getFunctionName();
+      if (fn && StringPrototypeStartsWith(fn, nodeInternalPrefix)) {
+        ArrayPrototypeSplice(trace, 0, l + 1);
+        break;
+      }
+    }
+    // `userStackTraceLimit` is the user value for `Error.stackTraceLimit`,
+    // it is updated at every new exception in `captureLargerStackTrace`.
+    if (trace.length > userStackTraceLimit)
+      ArrayPrototypeSplice(trace, userStackTraceLimit);
+  }
+
   const globalOverride =
     maybeOverridePrepareStackTrace(globalThis, error, trace);
   if (globalOverride !== kNoOverride) return globalOverride;
@@ -118,8 +135,6 @@ const maybeOverridePrepareStackTrace = (globalThis, error, trace) => {
   return kNoOverride;
 };
 
-let excludedStackFn;
-
 // Lazily loaded
 let util;
 let assert;
@@ -147,6 +162,27 @@ function lazyBuffer() {
   return buffer;
 }
 
+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.
+  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;
+  }
+});
+
 // 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,15 +193,11 @@ 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})`;
@@ -273,16 +305,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 +333,11 @@ 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;
-  }
+  // We rename the functions that will be hidden to cut off the stacktrace
+  // at the outermost one
+  const hidden = nodeInternalPrefix + fn.name;
+  ObjectDefineProperty(fn, 'name', { value: hidden });
+  return fn;
 }
 
 // Utility function for registering the error codes. Only used here. Exported
@@ -413,6 +407,16 @@ function uvErrmapGet(name) {
   return uvBinding.errmap.get(name);
 }
 
+const captureLargerStackTrace = hideStackFrames(
+  function captureLargerStackTrace(err) {
+    userStackTraceLimit = Error.stackTraceLimit;
+    Error.stackTraceLimit = Infinity;
+    ErrorCaptureStackTrace(err);
+    // Reset the limit
+    Error.stackTraceLimit = userStackTraceLimit;
+
+    return err;
+  });
 
 /**
  * This creates an error compatible with errors produced in the C++
@@ -423,8 +427,8 @@ function uvErrmapGet(name) {
  * @param {Object} ctx
  * @returns {Error}
  */
-function uvException(ctx) {
-  const [ code, uvmsg ] = uvErrmapGet(ctx.errno) || uvUnmappedError;
+const uvException = hideStackFrames(function uvException(ctx) {
+  const [code, uvmsg] = uvErrmapGet(ctx.errno) || uvUnmappedError;
   let message = `${code}: ${ctx.message || uvmsg}, ${ctx.syscall}`;
 
   let path;
@@ -463,9 +467,9 @@ function uvException(ctx) {
   if (dest) {
     err.dest = dest;
   }
-  ErrorCaptureStackTrace(err, excludedStackFn || uvException);
-  return err;
-}
+
+  return captureLargerStackTrace(err);
+});
 
 /**
  * This creates an error compatible with errors produced in the C++
@@ -478,35 +482,36 @@ 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 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}`;
+    }
 
-  // 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
+    // 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;
+    }
+
+    return captureLargerStackTrace(ex);
+  });
 
 /**
  * This used to be util._errnoException().
@@ -516,24 +521,28 @@ function uvExceptionWithHostPort(err, syscall, address, port) {
  * @param {string} [original]
  * @returns {Error}
  */
-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}`;
-
-  // eslint-disable-next-line no-restricted-syntax
-  const ex = new Error(message);
-  ex.errno = err;
-  ex.code = code;
-  ex.syscall = syscall;
-  ErrorCaptureStackTrace(ex, excludedStackFn || errnoException);
-  return ex;
-}
+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;
+
+    return captureLargerStackTrace(ex);
+  });
 
 /**
  * Deprecated, new function is `uvExceptionWithHostPort()`
@@ -546,41 +555,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 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})`;
+    }
 
-  // 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
+    // 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;
+    }
+
+    return captureLargerStackTrace(ex);
+  });
 
 /**
  * @param {number|string} code - A libuv error number or a c-ares error code
@@ -588,7 +598,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 +632,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
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
index 885df198a0..34aa897c33 100644
--- a/lib/internal/fs/promises.js
+++ b/lib/internal/fs/promises.js
@@ -276,7 +276,7 @@ async function writeFileHandle(filehandle, data, signal) {
   if (remaining === 0) return;
   do {
     if (signal?.aborted) {
-      throw new lazyDOMException('The operation was aborted', 'AbortError');
+      throw lazyDOMException('The operation was aborted', 'AbortError');
     }
     const { bytesWritten } =
       await write(filehandle, data, 0,
@@ -670,7 +670,7 @@ async function writeFile(path, data, options) {
 
   const fd = await open(path, flag, options.mode);
   if (options.signal?.aborted) {
-    throw new lazyDOMException('The operation was aborted', 'AbortError');
+    throw lazyDOMException('The operation was aborted', 'AbortError');
   }
   return PromisePrototypeFinally(writeFileHandle(fd, data), fd.close);
 }