lib: refactor Error.captureStackTrace() usage

When using `Errors.captureStackFrames` the error's stack property
is set again. This adds a helper function that wraps this functionality
in a simple API that does not only set the stack including the `code`
property but it also improves the performance to create the error.
The helper works for thrown errors and errors returned from wrapped
functions in case they are Node.js core errors.

PR-URL: https://github.com/nodejs/node/pull/26738
Fixes: https://github.com/nodejs/node/issues/26669
Fixes: https://github.com/nodejs/node/issues/20253
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

lib/_http_outgoing.js

@@ -34,16 +34,19 @@ const {
   symbols: { async_id_symbol }
 } = require('internal/async_hooks');
 const {
-  ERR_HTTP_HEADERS_SENT,
+  codes: {
-  ERR_HTTP_INVALID_HEADER_VALUE,
+    ERR_HTTP_HEADERS_SENT,
-  ERR_HTTP_TRAILER_INVALID,
+    ERR_HTTP_INVALID_HEADER_VALUE,
-  ERR_INVALID_HTTP_TOKEN,
+    ERR_HTTP_TRAILER_INVALID,
-  ERR_INVALID_ARG_TYPE,
+    ERR_INVALID_HTTP_TOKEN,
-  ERR_INVALID_CHAR,
+    ERR_INVALID_ARG_TYPE,
-  ERR_METHOD_NOT_IMPLEMENTED,
+    ERR_INVALID_CHAR,
-  ERR_STREAM_CANNOT_PIPE,
+    ERR_METHOD_NOT_IMPLEMENTED,
-  ERR_STREAM_WRITE_AFTER_END
+    ERR_STREAM_CANNOT_PIPE,
-} = require('internal/errors').codes;
+    ERR_STREAM_WRITE_AFTER_END
+  },
+  hideStackFrames
+} = require('internal/errors');
 const { validateString } = require('internal/validators');
 
 const { CRLF, debug } = common;
@@ -443,39 +446,21 @@ function matchHeader(self, state, field, value) {
   }
 }
 
-function validateHeaderName(name) {
+const validateHeaderName = hideStackFrames((name) => {
   if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) {
-    // Reducing the limit improves the performance significantly. We do not
+    throw new ERR_INVALID_HTTP_TOKEN('Header name', name);
-    // lose the stack frames due to the `captureStackTrace()` function that is
-    // called later.
-    const tmpLimit = Error.stackTraceLimit;
-    Error.stackTraceLimit = 0;
-    const err = new ERR_INVALID_HTTP_TOKEN('Header name', name);
-    Error.stackTraceLimit = tmpLimit;
-    Error.captureStackTrace(err, validateHeaderName);
-    throw err;
   }
-}
+});
 
-function validateHeaderValue(name, value) {
+const validateHeaderValue = hideStackFrames((name, value) => {
-  let err;
-  // 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;
   if (value === undefined) {
-    err = new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
+    throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
-  } else if (checkInvalidHeaderChar(value)) {
+  }
+  if (checkInvalidHeaderChar(value)) {
     debug('Header "%s" contains invalid characters', name);
-    err = new ERR_INVALID_CHAR('header content', name);
+    throw new ERR_INVALID_CHAR('header content', name);
   }
-  Error.stackTraceLimit = tmpLimit;
+});
-  if (err !== undefined) {
-    Error.captureStackTrace(err, validateHeaderValue);
-    throw err;
-  }
-}
 
 OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
   if (this._header) {

lib/buffer.js

@@ -62,15 +62,18 @@ const {
 } = require('internal/util/inspect');
 
 const {
-  ERR_BUFFER_OUT_OF_BOUNDS,
+  codes: {
-  ERR_OUT_OF_RANGE,
+    ERR_BUFFER_OUT_OF_BOUNDS,
-  ERR_INVALID_ARG_TYPE,
+    ERR_OUT_OF_RANGE,
-  ERR_INVALID_ARG_VALUE,
+    ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_BUFFER_SIZE,
+    ERR_INVALID_ARG_VALUE,
-  ERR_INVALID_OPT_VALUE,
+    ERR_INVALID_BUFFER_SIZE,
-  ERR_NO_LONGER_SUPPORTED,
+    ERR_INVALID_OPT_VALUE,
-  ERR_UNKNOWN_ENCODING
+    ERR_NO_LONGER_SUPPORTED,
-} = require('internal/errors').codes;
+    ERR_UNKNOWN_ENCODING
+  },
+  hideStackFrames
+} = require('internal/errors');
 const { validateString } = require('internal/validators');
 
 const {
@@ -247,20 +250,14 @@ Object.setPrototypeOf(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.
-function assertSize(size) {
+const assertSize = hideStackFrames((size) => {
-  let err = null;
-
   if (typeof size !== 'number') {
-    err = new ERR_INVALID_ARG_TYPE('size', 'number', size);
+    throw new ERR_INVALID_ARG_TYPE('size', 'number', size);
-  } else if (!(size >= 0 && size <= kMaxLength)) {
-    err = new ERR_INVALID_OPT_VALUE.RangeError('size', size);
   }
-
+  if (!(size >= 0 && size <= kMaxLength)) {
-  if (err !== null) {
+    throw new ERR_INVALID_OPT_VALUE.RangeError('size', size);
-    Error.captureStackTrace(err, assertSize);
-    throw err;
   }
-}
+});
 
 /**
  * Creates a new filled Buffer instance.

lib/fs.js

@@ -43,13 +43,15 @@ const pathModule = require('path');
 const { isArrayBufferView } = require('internal/util/types');
 const binding = internalBinding('fs');
 const { Buffer, kMaxLength } = require('buffer');
-const errors = require('internal/errors');
 const {
-  ERR_FS_FILE_TOO_LARGE,
+  codes: {
-  ERR_INVALID_ARG_VALUE,
+    ERR_FS_FILE_TOO_LARGE,
-  ERR_INVALID_ARG_TYPE,
+    ERR_INVALID_ARG_VALUE,
-  ERR_INVALID_CALLBACK
+    ERR_INVALID_ARG_TYPE,
-} = errors.codes;
+    ERR_INVALID_CALLBACK
+  },
+  uvException
+} = require('internal/errors');
 
 const { FSReqCallback, statValues } = binding;
 const { toPathIfFileURL } = require('internal/url');
@@ -114,10 +116,11 @@ function showTruncateDeprecation() {
 
 function handleErrorFromBinding(ctx) {
   if (ctx.errno !== undefined) {  // libuv error numbers
-    const err = errors.uvException(ctx);
+    const err = uvException(ctx);
     Error.captureStackTrace(err, handleErrorFromBinding);
     throw err;
-  } else if (ctx.error !== undefined) {  // errors created in C++ land.
+  }
+  if (ctx.error !== undefined) {  // errors created in C++ land.
     // TODO(joyeecheung): currently, ctx.error are encoding errors
     // usually caused by memory problems. We need to figure out proper error
     // code(s) for this.
@@ -310,7 +313,7 @@ function tryStatSync(fd, isUserFd) {
   const stats = binding.fstat(fd, false, undefined, ctx);
   if (ctx.errno !== undefined && !isUserFd) {
     fs.closeSync(fd);
-    throw errors.uvException(ctx);
+    throw uvException(ctx);
   }
   return stats;
 }

lib/internal/crypto/pbkdf2.js

@@ -65,8 +65,8 @@ function check(password, salt, iterations, keylen, digest) {
 
   password = validateArrayBufferView(password, 'password');
   salt = validateArrayBufferView(salt, 'salt');
-  iterations = validateUint32(iterations, 'iterations', 0);
+  validateUint32(iterations, 'iterations', 0);
-  keylen = validateUint32(keylen, 'keylen', 0);
+  validateUint32(keylen, 'keylen', 0);
 
   return { password, salt, iterations, keylen, digest };
 }

lib/internal/errors.js

@@ -18,7 +18,7 @@ const codes = {};
 const { kMaxLength } = internalBinding('buffer');
 const { defineProperty } = Object;
 
-let useOriginalName = false;
+let excludedStackFn;
 
 // Lazily loaded
 let util;
@@ -49,7 +49,15 @@ function lazyBuffer() {
 // and may have .path and .dest.
 class SystemError extends Error {
   constructor(key, context) {
-    super();
+    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 prefix = getMessage(key, [], this);
     let message = `${prefix}: ${context.syscall} returned ` +
                   `${context.code} (${context.message})`;
@@ -148,7 +156,15 @@ function makeSystemErrorWithCode(key) {
 function makeNodeErrorWithCode(Base, key) {
   return class NodeError extends Base {
     constructor(...args) {
-      super();
+      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 message = getMessage(key, args, this);
       Object.defineProperty(this, 'message', {
         value: message,
@@ -178,9 +194,30 @@ 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) {
-  if (useOriginalName) {
+  // Set the stack
-    return;
+  if (excludedStackFn !== undefined) {
+    // eslint-disable-next-line no-restricted-syntax
+    Error.captureStackTrace(err, excludedStackFn);
   }
   // Add the error code to the name to include it in the stack trace.
   err.name = `${name} [${code}]`;
@@ -308,6 +345,7 @@ function uvException(ctx) {
     err[prop] = ctx[prop];
   }
 
+  // TODO(BridgeAR): Show the `code` property as part of the stack.
   err.code = code;
   if (path) {
     err.path = path;
@@ -316,7 +354,7 @@ function uvException(ctx) {
     err.dest = dest;
   }
 
-  Error.captureStackTrace(err, uvException);
+  Error.captureStackTrace(err, excludedStackFn || uvException);
   return err;
 }
 
@@ -358,7 +396,7 @@ function uvExceptionWithHostPort(err, syscall, address, port) {
     ex.port = port;
   }
 
-  Error.captureStackTrace(ex, uvExceptionWithHostPort);
+  Error.captureStackTrace(ex, excludedStackFn || uvExceptionWithHostPort);
   return ex;
 }
 
@@ -386,7 +424,7 @@ function errnoException(err, syscall, original) {
   ex.code = ex.errno = code;
   ex.syscall = syscall;
 
-  Error.captureStackTrace(ex, errnoException);
+  Error.captureStackTrace(ex, excludedStackFn || errnoException);
   return ex;
 }
 
@@ -434,7 +472,7 @@ function exceptionWithHostPort(err, syscall, address, port, additional) {
     ex.port = port;
   }
 
-  Error.captureStackTrace(ex, exceptionWithHostPort);
+  Error.captureStackTrace(ex, excludedStackFn || exceptionWithHostPort);
   return ex;
 }
 
@@ -473,7 +511,8 @@ function dnsException(code, syscall, hostname) {
   if (hostname) {
     ex.hostname = hostname;
   }
-  Error.captureStackTrace(ex, dnsException);
+
+  Error.captureStackTrace(ex, excludedStackFn || dnsException);
   return ex;
 }
 
@@ -523,21 +562,19 @@ function oneOf(expected, thing) {
 }
 
 module.exports = {
+  addCodeToName, // Exported for NghttpError
+  codes,
   dnsException,
   errnoException,
   exceptionWithHostPort,
+  getMessage,
+  hideStackFrames,
+  isStackOverflowError,
   uvException,
   uvExceptionWithHostPort,
-  isStackOverflowError,
-  getMessage,
   SystemError,
-  codes,
   // This is exported only to facilitate testing.
-  E,
+  E
-  // This allows us to tell the type of the errors without using
-  // instanceof, which is necessary in WPT harness.
-  get useOriginalName() { return useOriginalName; },
-  set useOriginalName(value) { useOriginalName = value; }
 };
 
 // To declare an error message, use the E(sym, val, def) function above. The sym
@@ -556,7 +593,6 @@ module.exports = {
 // Note: Please try to keep these in alphabetical order
 //
 // Note: Node.js specific errors must begin with the prefix ERR_
-
 E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
 E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
 E('ERR_ASSERTION', '%s', Error);
@@ -630,7 +666,10 @@ E('ERR_ENCODING_INVALID_ENCODED_DATA', function(encoding, ret) {
 }, TypeError);
 E('ERR_ENCODING_NOT_SUPPORTED', 'The "%s" encoding is not supported',
   RangeError);
-E('ERR_FALSY_VALUE_REJECTION', 'Promise was rejected with falsy value', Error);
+E('ERR_FALSY_VALUE_REJECTION', function(reason) {
+  this.reason = reason;
+  return 'Promise was rejected with falsy value';
+}, Error);
 E('ERR_FS_FILE_TOO_LARGE', 'File size (%s) is greater than possible Buffer: ' +
     `${kMaxLength} bytes`,
   RangeError);

lib/internal/fs/utils.js

@@ -2,13 +2,16 @@
 
 const { Buffer, kMaxLength } = require('buffer');
 const {
-  ERR_FS_INVALID_SYMLINK_TYPE,
+  codes: {
-  ERR_INVALID_ARG_TYPE,
+    ERR_FS_INVALID_SYMLINK_TYPE,
-  ERR_INVALID_ARG_VALUE,
+    ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_OPT_VALUE,
+    ERR_INVALID_ARG_VALUE,
-  ERR_INVALID_OPT_VALUE_ENCODING,
+    ERR_INVALID_OPT_VALUE,
-  ERR_OUT_OF_RANGE
+    ERR_INVALID_OPT_VALUE_ENCODING,
-} = require('internal/errors').codes;
+    ERR_OUT_OF_RANGE
+  },
+  hideStackFrames
+} = require('internal/errors');
 const { isUint8Array, isArrayBufferView } = require('internal/util/types');
 const { once } = require('internal/util');
 const pathModule = require('path');
@@ -185,7 +188,7 @@ function getOptions(options, defaultOptions) {
 
 // Check if the path contains null types if it is a string nor Uint8Array,
 // otherwise return silently.
-function nullCheck(path, propName, throwError = true) {
+const nullCheck = hideStackFrames((path, propName, throwError = true) => {
   const pathIsString = typeof path === 'string';
   const pathIsUint8Array = isUint8Array(path);
 
@@ -196,26 +199,16 @@ function nullCheck(path, propName, throwError = true) {
     return;
   }
 
-  // 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;
-  if (throwError) {
-    Error.stackTraceLimit = 0;
-  }
   const err = new ERR_INVALID_ARG_VALUE(
     propName,
     path,
     'must be a string or Uint8Array without null bytes'
   );
-
   if (throwError) {
-    Error.stackTraceLimit = tmpLimit;
-    Error.captureStackTrace(err, nullCheck);
     throw err;
   }
   return err;
-}
+});
 
 function preprocessSymlinkDestination(path, type, linkPath) {
   if (!isWindows) {
@@ -359,7 +352,7 @@ function stringToFlags(flags) {
   throw new ERR_INVALID_OPT_VALUE('flags', flags);
 }
 
-function stringToSymlinkType(type) {
+const stringToSymlinkType = hideStackFrames((type) => {
   let flags = 0;
   if (typeof type === 'string') {
     switch (type) {
@@ -372,13 +365,11 @@ function stringToSymlinkType(type) {
       case 'file':
         break;
       default:
-        const err = new ERR_FS_INVALID_SYMLINK_TYPE(type);
+        throw new ERR_FS_INVALID_SYMLINK_TYPE(type);
-        Error.captureStackTrace(err, stringToSymlinkType);
-        throw err;
     }
   }
   return flags;
-}
+});
 
 // converts Date or number to a fractional UNIX timestamp
 function toUnixTimestamp(time, name = 'time') {
@@ -399,65 +390,51 @@ function toUnixTimestamp(time, name = 'time') {
   throw new ERR_INVALID_ARG_TYPE(name, ['Date', 'Time in seconds'], time);
 }
 
-function validateBuffer(buffer) {
+const validateBuffer = hideStackFrames((buffer) => {
   if (!isArrayBufferView(buffer)) {
-    const err = new ERR_INVALID_ARG_TYPE('buffer',
+    throw new ERR_INVALID_ARG_TYPE('buffer',
-                                         ['Buffer', 'TypedArray', 'DataView'],
+                                   ['Buffer', 'TypedArray', 'DataView'],
-                                         buffer);
+                                   buffer);
-    Error.captureStackTrace(err, validateBuffer);
-    throw err;
   }
-}
+});
 
-function validateOffsetLengthRead(offset, length, bufferLength) {
+const validateOffsetLengthRead = hideStackFrames(
-  let err;
+  (offset, length, bufferLength) => {
-
+    if (offset < 0 || offset >= bufferLength) {
-  if (offset < 0 || offset >= bufferLength) {
+      throw new ERR_OUT_OF_RANGE('offset',
-    err = new ERR_OUT_OF_RANGE('offset',
+                                 `>= 0 && <= ${bufferLength}`, offset);
-                               `>= 0 && <= ${bufferLength}`, offset);
+    }
-  } else if (length < 0 || offset + length > bufferLength) {
+    if (length < 0 || offset + length > bufferLength) {
-    err = new ERR_OUT_OF_RANGE('length',
+      throw new ERR_OUT_OF_RANGE('length',
-                               `>= 0 && <= ${bufferLength - offset}`, length);
+                                 `>= 0 && <= ${bufferLength - offset}`, length);
-  }
-
-  if (err !== undefined) {
-    Error.captureStackTrace(err, validateOffsetLengthRead);
-    throw err;
-  }
-}
-
-function validateOffsetLengthWrite(offset, length, byteLength) {
-  let err;
-
-  if (offset > byteLength) {
-    err = new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset);
-  } else {
-    const max = byteLength > kMaxLength ? kMaxLength : byteLength;
-    if (length > max - offset) {
-      err = new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length);
     }
   }
+);
 
-  if (err !== undefined) {
+const validateOffsetLengthWrite = hideStackFrames(
-    Error.captureStackTrace(err, validateOffsetLengthWrite);
+  (offset, length, byteLength) => {
-    throw err;
+    if (offset > byteLength) {
+      throw new ERR_OUT_OF_RANGE('offset', `<= ${byteLength}`, offset);
+    }
+
+    const max = byteLength > kMaxLength ? kMaxLength : byteLength;
+    if (length > max - offset) {
+      throw new ERR_OUT_OF_RANGE('length', `<= ${max - offset}`, length);
+    }
   }
-}
+);
-
-function validatePath(path, propName = 'path') {
-  let err;
 
+const validatePath = hideStackFrames((path, propName = 'path') => {
   if (typeof path !== 'string' && !isUint8Array(path)) {
-    err = new ERR_INVALID_ARG_TYPE(propName, ['string', 'Buffer', 'URL'], path);
+    throw new ERR_INVALID_ARG_TYPE(propName, ['string', 'Buffer', 'URL'], path);
-  } else {
-    err = nullCheck(path, propName, false);
   }
 
+  const err = nullCheck(path, propName, false);
+
   if (err !== undefined) {
-    Error.captureStackTrace(err, validatePath);
     throw err;
   }
-}
+});
 
 module.exports = {
   assertEncoding,

lib/internal/http2/compat.js

@@ -6,17 +6,20 @@ const Readable = Stream.Readable;
 const binding = internalBinding('http2');
 const constants = binding.constants;
 const {
-  ERR_HTTP2_HEADERS_SENT,
+  codes: {
-  ERR_HTTP2_INFO_STATUS_NOT_ALLOWED,
+    ERR_HTTP2_HEADERS_SENT,
-  ERR_HTTP2_INVALID_HEADER_VALUE,
+    ERR_HTTP2_INFO_STATUS_NOT_ALLOWED,
-  ERR_HTTP2_INVALID_STREAM,
+    ERR_HTTP2_INVALID_HEADER_VALUE,
-  ERR_HTTP2_NO_SOCKET_MANIPULATION,
+    ERR_HTTP2_INVALID_STREAM,
-  ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED,
+    ERR_HTTP2_NO_SOCKET_MANIPULATION,
-  ERR_HTTP2_STATUS_INVALID,
+    ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED,
-  ERR_INVALID_ARG_VALUE,
+    ERR_HTTP2_STATUS_INVALID,
-  ERR_INVALID_CALLBACK,
+    ERR_INVALID_ARG_VALUE,
-  ERR_INVALID_HTTP_TOKEN
+    ERR_INVALID_CALLBACK,
-} = require('internal/errors').codes;
+    ERR_INVALID_HTTP_TOKEN
+  },
+  hideStackFrames
+} = require('internal/errors');
 const { validateString } = require('internal/validators');
 const { kSocket, kRequest, kProxySocket } = require('internal/http2/util');
 
@@ -51,22 +54,20 @@ let statusConnectionHeaderWarned = false;
 // HTTP/2 implementation, intended to provide an interface that is as
 // close as possible to the current require('http') API
 
-function assertValidHeader(name, value) {
+const assertValidHeader = hideStackFrames((name, value) => {
-  let err;
   if (name === '' || typeof name !== 'string') {
-    err = new ERR_INVALID_HTTP_TOKEN('Header name', name);
+    throw new ERR_INVALID_HTTP_TOKEN('Header name', name);
-  } else if (isPseudoHeader(name)) {
+  }
-    err = new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED();
+  if (isPseudoHeader(name)) {
-  } else if (value === undefined || value === null) {
+    throw new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED();
-    err = new ERR_HTTP2_INVALID_HEADER_VALUE(value, name);
+  }
-  } else if (!isConnectionHeaderAllowed(name, value)) {
+  if (value === undefined || value === null) {
+    throw new ERR_HTTP2_INVALID_HEADER_VALUE(value, name);
+  }
+  if (!isConnectionHeaderAllowed(name, value)) {
     connectionHeaderMessageWarn();
   }
-  if (err !== undefined) {
+});
-    Error.captureStackTrace(err, assertValidHeader);
-    throw err;
-  }
-}
 
 function isPseudoHeader(name) {
   switch (name) {

lib/internal/http2/core.js

@@ -76,7 +76,8 @@ const {
     ERR_INVALID_OPT_VALUE,
     ERR_OUT_OF_RANGE,
     ERR_SOCKET_CLOSED
-  }
+  },
+  hideStackFrames
 } = require('internal/errors');
 const { validateNumber, validateString } = require('internal/validators');
 const { utcDate } = require('internal/http');
@@ -606,37 +607,31 @@ function requestOnConnect(headers, options) {
 // 4. if specified, options.silent must be a boolean
 //
 // Also sets the default priority options if they are not set.
-function validatePriorityOptions(options) {
+const validatePriorityOptions = hideStackFrames((options) => {
-  let err;
   if (options.weight === undefined) {
     options.weight = NGHTTP2_DEFAULT_WEIGHT;
   } else if (typeof options.weight !== 'number') {
-    err = new ERR_INVALID_OPT_VALUE('weight', options.weight);
+    throw new ERR_INVALID_OPT_VALUE('weight', options.weight);
   }
 
   if (options.parent === undefined) {
     options.parent = 0;
   } else if (typeof options.parent !== 'number' || options.parent < 0) {
-    err = new ERR_INVALID_OPT_VALUE('parent', options.parent);
+    throw new ERR_INVALID_OPT_VALUE('parent', options.parent);
   }
 
   if (options.exclusive === undefined) {
     options.exclusive = false;
   } else if (typeof options.exclusive !== 'boolean') {
-    err = new ERR_INVALID_OPT_VALUE('exclusive', options.exclusive);
+    throw new ERR_INVALID_OPT_VALUE('exclusive', options.exclusive);
   }
 
   if (options.silent === undefined) {
     options.silent = false;
   } else if (typeof options.silent !== 'boolean') {
-    err = new ERR_INVALID_OPT_VALUE('silent', options.silent);
+    throw new ERR_INVALID_OPT_VALUE('silent', options.silent);
   }
-
+});
-  if (err) {
-    Error.captureStackTrace(err, validatePriorityOptions);
-    throw err;
-  }
-}
 
 // When an error occurs internally at the binding level, immediately
 // destroy the session.
@@ -788,7 +783,7 @@ function pingCallback(cb) {
 // 5. maxHeaderListSize must be a number in the range 0 <= n <= kMaxInt
 // 6. enablePush must be a boolean
 // All settings are optional and may be left undefined
-function validateSettings(settings) {
+const validateSettings = hideStackFrames((settings) => {
   settings = { ...settings };
   assertWithinRange('headerTableSize',
                     settings.headerTableSize,
@@ -807,13 +802,11 @@ function validateSettings(settings) {
                     0, kMaxInt);
   if (settings.enablePush !== undefined &&
       typeof settings.enablePush !== 'boolean') {
-    const err = new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush',
+    throw new ERR_HTTP2_INVALID_SETTING_VALUE('enablePush',
-                                                    settings.enablePush);
+                                              settings.enablePush);
-    Error.captureStackTrace(err, 'validateSettings');
-    throw err;
   }
   return settings;
-}
+});
 
 // Creates the internal binding.Http2Session handle for an Http2Session
 // instance. This occurs only after the socket connection has been

lib/internal/http2/util.js

@@ -2,12 +2,16 @@
 
 const binding = internalBinding('http2');
 const {
-  ERR_HTTP2_HEADER_SINGLE_VALUE,
+  codes: {
-  ERR_HTTP2_INVALID_CONNECTION_HEADERS,
+    ERR_HTTP2_HEADER_SINGLE_VALUE,
-  ERR_HTTP2_INVALID_PSEUDOHEADER,
+    ERR_HTTP2_INVALID_CONNECTION_HEADERS,
-  ERR_HTTP2_INVALID_SETTING_VALUE,
+    ERR_HTTP2_INVALID_PSEUDOHEADER,
-  ERR_INVALID_ARG_TYPE
+    ERR_HTTP2_INVALID_SETTING_VALUE,
-} = require('internal/errors').codes;
+    ERR_INVALID_ARG_TYPE
+  },
+  addCodeToName,
+  hideStackFrames
+} = require('internal/errors');
 
 const kSocket = Symbol('socket');
 const kProxySocket = Symbol('proxySocket');
@@ -404,27 +408,21 @@ function isIllegalConnectionSpecificHeader(name, value) {
   }
 }
 
-function assertValidPseudoHeader(key) {
+const assertValidPseudoHeader = hideStackFrames((key) => {
   if (!kValidPseudoHeaders.has(key)) {
-    const err = new ERR_HTTP2_INVALID_PSEUDOHEADER(key);
+    throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key);
-    Error.captureStackTrace(err, assertValidPseudoHeader);
-    throw err;
   }
-}
+});
 
-function assertValidPseudoHeaderResponse(key) {
+const assertValidPseudoHeaderResponse = hideStackFrames((key) => {
   if (key !== ':status') {
-    const err = new ERR_HTTP2_INVALID_PSEUDOHEADER(key);
+    throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key);
-    Error.captureStackTrace(err, assertValidPseudoHeaderResponse);
-    throw err;
   }
-}
+});
 
-function assertValidPseudoHeaderTrailer(key) {
+const assertValidPseudoHeaderTrailer = hideStackFrames((key) => {
-  const err = new ERR_HTTP2_INVALID_PSEUDOHEADER(key);
+  throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key);
-  Error.captureStackTrace(err, assertValidPseudoHeaderTrailer);
+});
-  throw err;
-}
 
 function mapToHeaders(map,
                       assertValuePseudoHeader = assertValidPseudoHeader) {
@@ -496,10 +494,8 @@ class NghttpError extends Error {
   constructor(ret) {
     super(binding.nghttp2ErrorString(ret));
     this.code = 'ERR_HTTP2_ERROR';
-    this.name = 'Error [ERR_HTTP2_ERROR]';
     this.errno = ret;
-    this.stack;
+    addCodeToName(this, super.name, 'ERR_HTTP2_ERROR');
-    delete this.name;
   }
 
   toString() {
@@ -507,26 +503,24 @@ class NghttpError extends Error {
   }
 }
 
-function assertIsObject(value, name, types) {
+const assertIsObject = hideStackFrames((value, name, types) => {
   if (value !== undefined &&
       (value === null ||
        typeof value !== 'object' ||
        Array.isArray(value))) {
-    const err = new ERR_INVALID_ARG_TYPE(name, types || 'Object', value);
+    throw new ERR_INVALID_ARG_TYPE(name, types || 'Object', value);
-    Error.captureStackTrace(err, assertIsObject);
-    throw err;
   }
-}
+});
 
-function assertWithinRange(name, value, min = 0, max = Infinity) {
+const assertWithinRange = hideStackFrames(
-  if (value !== undefined &&
+  (name, value, min = 0, max = Infinity) => {
+    if (value !== undefined &&
       (typeof value !== 'number' || value < min || value > max)) {
-    const err = new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError(
+      throw new ERR_HTTP2_INVALID_SETTING_VALUE.RangeError(
-      name, value, min, max);
+        name, value, min, max);
-    Error.captureStackTrace(err, assertWithinRange);
+    }
-    throw err;
   }
-}
+);
 
 function toHeaderObject(headers) {
   const obj = Object.create(null);

lib/internal/util/inspect.js

@@ -993,6 +993,7 @@ function formatPrimitive(fn, value, ctx) {
 }
 
 function formatError(value) {
+  // TODO(BridgeAR): Always show the error code if present.
   return value.stack || errorToString(value);
 }
 

lib/internal/validators.js

@@ -1,10 +1,13 @@
 'use strict';
 
 const {
-  ERR_INVALID_ARG_TYPE,
+  hideStackFrames,
-  ERR_INVALID_ARG_VALUE,
+  codes: {
-  ERR_OUT_OF_RANGE
+    ERR_INVALID_ARG_TYPE,
-} = require('internal/errors').codes;
+    ERR_INVALID_ARG_VALUE,
+    ERR_OUT_OF_RANGE
+  }
+} = require('internal/errors');
 
 function isInt32(value) {
   return value === (value | 0);
@@ -52,66 +55,52 @@ function validateMode(value, name, def) {
   throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
 }
 
-function validateInteger(value, name) {
+const validateInteger = hideStackFrames((value, name) => {
-  let err;
-
   if (typeof value !== 'number')
-    err = new ERR_INVALID_ARG_TYPE(name, 'number', value);
+    throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
-  else if (!Number.isSafeInteger(value))
+  if (!Number.isSafeInteger(value))
-    err = new ERR_OUT_OF_RANGE(name, 'an integer', value);
+    throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
-
-  if (err) {
-    Error.captureStackTrace(err, validateInteger);
-    throw err;
-  }
-
   return value;
-}
+});
 
-function validateInt32(value, name, min = -2147483648, max = 2147483647) {
+const validateInt32 = hideStackFrames(
-  // The defaults for min and max correspond to the limits of 32-bit integers.
+  (value, name, min = -2147483648, max = 2147483647) => {
-  if (!isInt32(value)) {
+    // The defaults for min and max correspond to the limits of 32-bit integers.
-    let err;
+    if (!isInt32(value)) {
-    if (typeof value !== 'number') {
+      if (typeof value !== 'number') {
-      err = new ERR_INVALID_ARG_TYPE(name, 'number', value);
+        throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
-    } else if (!Number.isInteger(value)) {
+      }
-      err = new ERR_OUT_OF_RANGE(name, 'an integer', value);
+      if (!Number.isInteger(value)) {
-    } else {
+        throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
-      err = new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
+      }
+      throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
     }
-    Error.captureStackTrace(err, validateInt32);
+    if (value < min || value > max) {
-    throw err;
+      throw new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
-  } else if (value < min || value > max) {
+    }
-    const err = new ERR_OUT_OF_RANGE(name, `>= ${min} && <= ${max}`, value);
+    return value;
-    Error.captureStackTrace(err, validateInt32);
-    throw err;
   }
+);
 
-  return value;
+const validateUint32 = hideStackFrames((value, name, positive) => {
-}
-
-function validateUint32(value, name, positive) {
   if (!isUint32(value)) {
-    let err;
     if (typeof value !== 'number') {
-      err = new ERR_INVALID_ARG_TYPE(name, 'number', value);
+      throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
-    } else if (!Number.isInteger(value)) {
-      err = new ERR_OUT_OF_RANGE(name, 'an integer', value);
-    } else {
-      const min = positive ? 1 : 0;
-      // 2 ** 32 === 4294967296
-      err = new ERR_OUT_OF_RANGE(name, `>= ${min} && < 4294967296`, value);
     }
-    Error.captureStackTrace(err, validateUint32);
+    if (!Number.isInteger(value)) {
-    throw err;
+      throw new ERR_OUT_OF_RANGE(name, 'an integer', value);
-  } else if (positive && value === 0) {
+    }
-    const err = new ERR_OUT_OF_RANGE(name, '>= 1 && < 4294967296', value);
+    const min = positive ? 1 : 0;
-    Error.captureStackTrace(err, validateUint32);
+    // 2 ** 32 === 4294967296
-    throw err;
+    throw new ERR_OUT_OF_RANGE(name, `>= ${min} && < 4294967296`, value);
   }
-
+  if (positive && value === 0) {
+    throw new ERR_OUT_OF_RANGE(name, '>= 1 && < 4294967296', value);
+  }
+  // TODO(BridgeAR): Remove return values from validation functions and
+  // especially reduce side effects caused by validation functions.
   return value;
-}
+});
 
 function validateString(value, name) {
   if (typeof value !== 'string')

lib/os.js

@@ -26,7 +26,12 @@ const constants = internalBinding('constants').os;
 const { deprecate } = require('internal/util');
 const isWindows = process.platform === 'win32';
 
-const { codes: { ERR_SYSTEM_ERROR } } = require('internal/errors');
+const {
+  codes: {
+    ERR_SYSTEM_ERROR
+  },
+  hideStackFrames
+} = require('internal/errors');
 const { validateInt32 } = require('internal/validators');
 
 const {
@@ -47,16 +52,14 @@ const {
 } = internalBinding('os');
 
 function getCheckedFunction(fn) {
-  return function checkError(...args) {
+  return hideStackFrames(function checkError(...args) {
     const ctx = {};
     const ret = fn(...args, ctx);
     if (ret === undefined) {
-      const err = new ERR_SYSTEM_ERROR(ctx);
+      throw new ERR_SYSTEM_ERROR(ctx);
-      Error.captureStackTrace(err, checkError);
-      throw err;
     }
     return ret;
-  };
+  });
 }
 
 const getHomeDirectory = getCheckedFunction(_getHomeDirectory);

lib/util.js

@@ -21,18 +21,22 @@
 
 'use strict';
 
-const errors = require('internal/errors');
+const {
+  codes: {
+    ERR_FALSY_VALUE_REJECTION,
+    ERR_INVALID_ARG_TYPE,
+    ERR_OUT_OF_RANGE
+  },
+  errnoException,
+  exceptionWithHostPort,
+  hideStackFrames
+} = require('internal/errors');
 const {
   format,
   formatWithOptions,
   inspect
 } = require('internal/util/inspect');
 const { debuglog } = require('internal/util/debuglog');
-const {
-  ERR_FALSY_VALUE_REJECTION,
-  ERR_INVALID_ARG_TYPE,
-  ERR_OUT_OF_RANGE
-} = errors.codes;
 const { validateNumber } = require('internal/validators');
 const { TextDecoder, TextEncoder } = require('internal/encoding');
 const { isBuffer } = require('buffer').Buffer;
@@ -158,19 +162,16 @@ function _extend(target, source) {
   return target;
 }
 
-function callbackifyOnRejected(reason, cb) {
+const callbackifyOnRejected = hideStackFrames((reason, cb) => {
   // `!reason` guard inspired by bluebird (Ref: https://goo.gl/t5IS6M).
   // Because `null` is a special error value in callbacks which means "no error
   // occurred", we error-wrap so the callback consumer can distinguish between
   // "the promise rejected with null" or "the promise fulfilled with undefined".
   if (!reason) {
-    const newReason = new ERR_FALSY_VALUE_REJECTION();
+    reason = new ERR_FALSY_VALUE_REJECTION(reason);
-    newReason.reason = reason;
-    reason = newReason;
-    Error.captureStackTrace(reason, callbackifyOnRejected);
   }
   return cb(reason);
-}
+});
 
 function callbackify(original) {
   if (typeof original !== 'function') {
@@ -209,8 +210,8 @@ function getSystemErrorName(err) {
 
 // Keep the `exports =` so that various functions can still be monkeypatched
 module.exports = exports = {
-  _errnoException: errors.errnoException,
+  _errnoException: errnoException,
-  _exceptionWithHostPort: errors.exceptionWithHostPort,
+  _exceptionWithHostPort: exceptionWithHostPort,
   _extend,
   callbackify,
   debuglog,

lib/zlib.js

@@ -22,12 +22,15 @@
 'use strict';
 
 const {
-  ERR_BROTLI_INVALID_PARAM,
+  codes: {
-  ERR_BUFFER_TOO_LARGE,
+    ERR_BROTLI_INVALID_PARAM,
-  ERR_INVALID_ARG_TYPE,
+    ERR_BUFFER_TOO_LARGE,
-  ERR_OUT_OF_RANGE,
+    ERR_INVALID_ARG_TYPE,
-  ERR_ZLIB_INITIALIZATION_FAILED,
+    ERR_OUT_OF_RANGE,
-} = require('internal/errors').codes;
+    ERR_ZLIB_INITIALIZATION_FAILED,
+  },
+  hideStackFrames
+} = require('internal/errors');
 const Transform = require('_stream_transform');
 const {
   deprecate,
@@ -170,7 +173,7 @@ function zlibOnError(message, errno, code) {
 // 2. Returns true for finite numbers
 // 3. Throws ERR_INVALID_ARG_TYPE for non-numbers
 // 4. Throws ERR_OUT_OF_RANGE for infinite numbers
-function checkFiniteNumber(number, name) {
+const checkFiniteNumber = hideStackFrames((number, name) => {
   // Common case
   if (number === undefined) {
     return false;
@@ -186,33 +189,29 @@ function checkFiniteNumber(number, name) {
 
   // Other non-numbers
   if (typeof number !== 'number') {
-    const err = new ERR_INVALID_ARG_TYPE(name, 'number', number);
+    throw new ERR_INVALID_ARG_TYPE(name, 'number', number);
-    Error.captureStackTrace(err, checkFiniteNumber);
-    throw err;
   }
 
   // Infinite numbers
-  const err = new ERR_OUT_OF_RANGE(name, 'a finite number', number);
+  throw new ERR_OUT_OF_RANGE(name, 'a finite number', number);
-  Error.captureStackTrace(err, checkFiniteNumber);
+});
-  throw err;
-}
 
 // 1. Returns def for number when it's undefined or NaN
 // 2. Returns number for finite numbers >= lower and <= upper
 // 3. Throws ERR_INVALID_ARG_TYPE for non-numbers
 // 4. Throws ERR_OUT_OF_RANGE for infinite numbers or numbers > upper or < lower
-function checkRangesOrGetDefault(number, name, lower, upper, def) {
+const checkRangesOrGetDefault = hideStackFrames(
-  if (!checkFiniteNumber(number, name)) {
+  (number, name, lower, upper, def) => {
-    return def;
+    if (!checkFiniteNumber(number, name)) {
+      return def;
+    }
+    if (number < lower || number > upper) {
+      throw new ERR_OUT_OF_RANGE(name,
+                                 `>= ${lower} and <= ${upper}`, number);
+    }
+    return number;
   }
-  if (number < lower || number > upper) {
+);
-    const err = new ERR_OUT_OF_RANGE(name,
-                                     `>= ${lower} and <= ${upper}`, number);
-    Error.captureStackTrace(err, checkRangesOrGetDefault);
-    throw err;
-  }
-  return number;
-}
 
 // The base class for all Zlib-style streams.
 function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) {

test/common/wpt.js

@@ -287,11 +287,6 @@ class WPTRunner {
   // TODO(joyeecheung): work with the upstream to port more tests in .html
   // to .js.
   runJsTests() {
-    // TODO(joyeecheung): it's still under discussion whether we should leave
-    // err.name alone. See https://github.com/nodejs/node/issues/20253
-    const internalErrors = require('internal/errors');
-    internalErrors.useOriginalName = true;
-
     let queue = [];
 
     // If the tests are run as `node test/wpt/test-something.js subset.any.js`,