errors: improve hideStackFrames

PR-URL: #49990
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>

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,87 @@
+'use strict';
+
+const common = require('../common.js');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  type: [
+    'hide-stackframes',
+    'direct-call',
+  ],
+  double: ['true', 'false'],
+  n: [1e5],
+}, {
+  flags: ['--expose-internals'],
+});
+
+function main({ n, type, double }) {
+  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);
+    }
+  });
+
+  function doubleTestfn(value) {
+    testfn(value);
+  }
+
+  const doubleHideStackFramesTestfn = hideStackFrames((value) => {
+    hideStackFramesTestfn.withoutStackTrace(value);
+  });
+
+  const fn = type === 'hide-stackframe' ?
+    double === 'true' ?
+      doubleHideStackFramesTestfn :
+      hideStackFramesTestfn :
+    double === 'true' ?
+      doubleTestfn :
+      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

@@ -19,7 +19,7 @@ rules:
     - selector: ThrowStatement > CallExpression[callee.name=/Error$/]
       message: Use new keyword when throwing an Error.
     # Config specific to lib
-    - selector: NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError)$/])
+    - selector: NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError|NodeAggregateError)$/])
       message: Use an error exported by the internal/errors module.
     - selector: CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']
       message: Please use `require('internal/errors').hideStackFrames()` instead.

lib/.eslintrc.yaml

@@ -72,7 +72,7 @@ const {
   traceEnd,
   getNextTraceEventId,
 } = require('internal/http');
-const { connResetException, codes } = require('internal/errors');
+const { ConnResetException, codes } = require('internal/errors');
 const {
   ERR_HTTP_HEADERS_SENT,
   ERR_INVALID_ARG_TYPE,
@@ -452,7 +452,7 @@ function socketCloseListener() {
   if (res) {
     // Socket closed before we emitted 'end' below.
     if (!res.complete) {
-      res.destroy(connResetException('aborted'));
+      res.destroy(new ConnResetException('aborted'));
     }
     req._closed = true;
     req.emit('close');
@@ -465,7 +465,7 @@ function socketCloseListener() {
       // receive a response. The error needs to
       // fire on the request.
       req.socket._hadError = true;
-      req.emit('error', connResetException('socket hang up'));
+      req.emit('error', new ConnResetException('socket hang up'));
     }
     req._closed = true;
     req.emit('close');
@@ -516,7 +516,7 @@ function socketOnEnd() {
     // If we don't have a response then we know that the socket
     // ended prematurely and we need to emit an error on the request.
     req.socket._hadError = true;
-    req.emit('error', connResetException('socket hang up'));
+    req.emit('error', new ConnResetException('socket hang up'));
   }
   if (parser) {
     parser.finish();
@@ -869,7 +869,7 @@ function onSocketNT(req, socket, err) {
 
     function _destroy(req, err) {
       if (!req.aborted && !err) {
-        err = connResetException('socket hang up');
+        err = new ConnResetException('socket hang up');
       }
       if (err) {
         req.emit('error', err);

lib/_http_client.js

@@ -663,17 +663,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

@@ -69,7 +69,7 @@ const {
 } = require('internal/async_hooks');
 const { IncomingMessage } = require('_http_incoming');
 const {
-  connResetException,
+  ConnResetException,
   codes,
 } = require('internal/errors');
 const {
@@ -790,7 +790,7 @@ function socketOnClose(socket, state) {
 function abortIncoming(incoming) {
   while (incoming.length) {
     const req = incoming.shift();
-    req.destroy(connResetException('aborted'));
+    req.destroy(new ConnResetException('aborted'));
   }
   // Abort socket._httpMessage ?
 }

lib/_http_server.js

@@ -66,7 +66,7 @@ const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');
 const { owner_symbol } = require('internal/async_hooks').symbols;
 const { isArrayBufferView } = require('internal/util/types');
 const { SecureContext: NativeSecureContext } = internalBinding('crypto');
-const { connResetException, codes } = require('internal/errors');
+const { ConnResetException, codes } = require('internal/errors');
 const {
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
@@ -1218,7 +1218,7 @@ function onSocketClose(err) {
   // Emit ECONNRESET
   if (!this._controlReleased && !this[kErrorEmitted]) {
     this[kErrorEmitted] = true;
-    const connReset = connResetException('socket hang up');
+    const connReset = new ConnResetException('socket hang up');
     this._tlsOptions.server.emit('tlsClientError', connReset, this);
   }
 }
@@ -1724,9 +1724,9 @@ function onConnectEnd() {
   if (!this._hadError) {
     const options = this[kConnectOptions];
     this._hadError = true;
-    const error = connResetException('Client network socket disconnected ' +
-                                     'before secure TLS connection was ' +
-                                     'established');
+    const error = new ConnResetException('Client network socket disconnected ' +
+                                         'before secure TLS connection was ' +
+                                         'established');
     error.path = options.path;
     error.host = options.host;
     error.port = options.port;

lib/_tls_wrap.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);