errors: improve hideStackFrames

Author: Uzlopak

benchmark/error/hidestackframes-noerr.js

@@ -0,0 +1,62 @@
+'use strict';
+
+const common = require('../common.js');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  type: [
+    'hide-stackframes',
+    'direct-call',
+  ],
+  n: [1e7],
+}, {
+  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);
+    }
+  };
+
+  const hideStackFramesTestfn = hideStackFrames((value) => {
+    if (typeof value !== 'number') {
+      throw new ERR_INVALID_ARG_TYPE.HideStackFrameError('Benchmark', 'number', value);
+    }
+  });
+
+  const fn = type === 'hide-stackframe' ? hideStackFramesTestfn : testfn;
+
+  const value = 42;
+
+  const length = 1024;
+  const array = [];
+  const errCase = false;
+
+  for (let i = 0; i < length; ++i) {
+    array.push(fn(value));
+  }
+
+  bench.start();
+
+  for (let i = 0; i < n; i++) {
+    const index = i % length;
+    array[index] = fn(value);
+  }
+
+  bench.end(n);
+
+  // Verify the entries to prevent dead code elimination from making
+  // the benchmark invalid.
+  for (let i = 0; i < length; ++i) {
+    assert.strictEqual(typeof array[i], errCase ? 'object' : 'undefined');
+  }
+}

benchmark/error/hidestackframes-throw.js

@@ -0,0 +1,72 @@
+'use strict';
+
+const common = require('../common.js');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  type: [
+    'hide-stackframes',
+    'direct-call',
+  ],
+  n: [1e5],
+}, {
+  flags: ['--expose-internals'],
+});
+
+function main({ n, type }) {
+  const {
+    hideStackFrames,
+    codes: {
+      ERR_INVALID_ARG_TYPE,
+    },
+  } = require('internal/errors');
+
+  const value = 'err';
+
+  const testfn = (value) => {
+    if (typeof value !== 'number') {
+      throw new ERR_INVALID_ARG_TYPE('Benchmark', 'number', value);
+    }
+  };
+
+  const hideStackFramesTestfn = hideStackFrames((value) => {
+    if (typeof value !== 'number') {
+      throw new ERR_INVALID_ARG_TYPE.HideStackFrameError('Benchmark', 'number', value);
+    }
+  });
+
+  const fn = type === 'hide-stackframe' ? hideStackFramesTestfn : testfn;
+
+  const length = 1024;
+  const array = [];
+  let errCase = false;
+
+  // Warm up.
+  for (let i = 0; i < length; ++i) {
+    try {
+      fn(value);
+    } catch (e) {
+      errCase = true;
+      array.push(e);
+    }
+  }
+
+  bench.start();
+
+  for (let i = 0; i < n; i++) {
+    const index = i % length;
+    try {
+      fn(value);
+    } catch (e) {
+      array[index] = e;
+    }
+  }
+
+  bench.end(n);
+
+  // Verify the entries to prevent dead code elimination from making
+  // the benchmark invalid.
+  for (let i = 0; i < length; ++i) {
+    assert.strictEqual(typeof array[i], errCase ? 'object' : 'undefined');
+  }
+}

benchmark/error/hidestackframes.js

@@ -619,17 +619,17 @@ function matchHeader(self, state, field, value) {
 
 const validateHeaderName = hideStackFrames((name, label) => {
   if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) {
-    throw new ERR_INVALID_HTTP_TOKEN(label || 'Header name', name);
+    throw new ERR_INVALID_HTTP_TOKEN.HideStackFramesError(label || 'Header name', name);
   }
 });
 
 const validateHeaderValue = hideStackFrames((name, value) => {
   if (value === undefined) {
-    throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
+    throw new ERR_HTTP_INVALID_HEADER_VALUE.HideStackFramesError(value, name);
   }
   if (checkInvalidHeaderChar(value)) {
     debug('Header "%s" contains invalid characters', name);
-    throw new ERR_INVALID_CHAR('header content', name);
+    throw new ERR_INVALID_CHAR.HideStackFramesError('header content', name);
   }
 });
 

lib/_http_outgoing.js

@@ -108,7 +108,6 @@ const {
     ERR_UNKNOWN_ENCODING,
   },
   genericNodeError,
-  hideStackFrames,
 } = require('internal/errors');
 const {
   validateArray,
@@ -386,19 +385,12 @@ Buffer.of = of;
 
 ObjectSetPrototypeOf(Buffer, Uint8Array);
 
-// The 'assertSize' method will remove itself from the callstack when an error
-// occurs. This is done simply to keep the internal details of the
-// implementation from bleeding out to users.
-const assertSize = hideStackFrames((size) => {
-  validateNumber(size, 'size', 0, kMaxLength);
-});
-
 /**
  * Creates a new filled Buffer instance.
  * alloc(size[, fill[, encoding]])
  */
 Buffer.alloc = function alloc(size, fill, encoding) {
-  assertSize(size);
+  validateNumber(size, 'size', 0, kMaxLength);
   if (fill !== undefined && fill !== 0 && size > 0) {
     const buf = createUnsafeBuffer(size);
     return _fill(buf, fill, 0, buf.length, encoding);
@@ -411,7 +403,7 @@ Buffer.alloc = function alloc(size, fill, encoding) {
  * instance. If `--zero-fill-buffers` is set, will zero-fill the buffer.
  */
 Buffer.allocUnsafe = function allocUnsafe(size) {
-  assertSize(size);
+  validateNumber(size, 'size', 0, kMaxLength);
   return allocate(size);
 };
 
@@ -421,15 +413,15 @@ Buffer.allocUnsafe = function allocUnsafe(size) {
  * If `--zero-fill-buffers` is set, will zero-fill the buffer.
  */
 Buffer.allocUnsafeSlow = function allocUnsafeSlow(size) {
-  assertSize(size);
+  validateNumber(size, 'size', 0, kMaxLength);
   return createUnsafeBuffer(size);
 };
 
 // If --zero-fill-buffers command line argument is set, a zero-filled
 // buffer is returned.
-function SlowBuffer(length) {
-  assertSize(length);
-  return createUnsafeBuffer(length);
+function SlowBuffer(size) {
+  validateNumber(size, 'size', 0, kMaxLength);
+  return createUnsafeBuffer(size);
 }
 
 ObjectSetPrototypeOf(SlowBuffer.prototype, Uint8ArrayPrototype);

lib/buffer.js

@@ -87,7 +87,6 @@ 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.
@@ -97,21 +96,6 @@ 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;
@@ -372,6 +356,29 @@ function makeSystemErrorWithCode(key) {
   };
 }
 
+function makeNodeErrorForHideStackFrame(Base, clazz) {
+  class HideStackFramesError extends Base {
+    constructor(...args) {
+      if (isErrorStackTraceLimitWritable()) {
+        const limit = Error.stackTraceLimit;
+        Error.stackTraceLimit = 0;
+        super(...args);
+        Error.stackTraceLimit = limit;
+      } else {
+        super(...args);
+      }
+    }
+
+    // This is a workaround for wpt tests that expect that the error
+    // constructor has a `name` property of the base class.
+    get ['constructor']() {
+      return clazz;
+    }
+  }
+
+  return HideStackFramesError;
+}
+
 function makeNodeErrorWithCode(Base, key) {
   const msg = messages.get(key);
   const expectedLength = typeof msg !== 'string' ? -1 : getExpectedArgumentLength(msg);
@@ -479,11 +486,14 @@ function makeNodeErrorWithCode(Base, key) {
  * @returns {T}
  */
 function hideStackFrames(fn) {
-  // 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', { __proto__: null, value: hidden });
-  return fn;
+  return function wrappedFn() {
+    try {
+      return ReflectApply(fn, fn, arguments);
+    } catch (error) {
+      Error.stackTraceLimit && ErrorCaptureStackTrace(error, wrappedFn);
+      throw error;
+    }
+  };
 }
 
 // Utility function for registering the error codes. Only used here. Exported
@@ -492,18 +502,20 @@ function E(sym, val, def, ...otherClasses) {
   // Special case for SystemError that formats the error message differently
   // The SystemErrors only have SystemError as their base classes.
   messages.set(sym, val);
-  if (def === SystemError) {
-    def = makeSystemErrorWithCode(sym);
-  } else {
-    def = makeNodeErrorWithCode(def, sym);
-  }
+
+  const ErrClass = def === SystemError ?
+    makeSystemErrorWithCode(sym) :
+    makeNodeErrorWithCode(def, sym);
 
   if (otherClasses.length !== 0) {
     otherClasses.forEach((clazz) => {
-      def[clazz.name] = makeNodeErrorWithCode(clazz, sym);
+      ErrClass[clazz.name] = makeNodeErrorWithCode(clazz, sym);
     });
   }
-  codes[sym] = def;
+
+  ErrClass.HideStackFramesError = makeNodeErrorForHideStackFrame(ErrClass, def);
+
+  codes[sym] = ErrClass;
 }
 
 function getExpectedArgumentLength(msg) {