From ed1a66430910036e6c47328e00811eeceeae611f Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 11 Feb 2023 13:32:34 +0100
Subject: [PATCH 01/15] errors: improve hideInternalStackFrames performance

Directly concat the necessary information to a string instead of
allocating intermediate arrays and iterating over the array more
than once.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 742d723155e7fe..b26edeecef08eb 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -14,7 +14,6 @@ const {
   AggregateError,
   ArrayFrom,
   ArrayIsArray,
-  ArrayPrototypeFilter,
   ArrayPrototypeIncludes,
   ArrayPrototypeIndexOf,
   ArrayPrototypeJoin,
@@ -89,18 +88,18 @@ const nodeInternalPrefix = '__node_internal_';
 const prepareStackTrace = (globalThis, error, trace) => {
   // API for node internals to override error stack formatting
   // without interfering with userland code.
-  if (overrideStackTrace.has(error)) {
-    const f = overrideStackTrace.get(error);
+  const fn = overrideStackTrace.get(error);
+  if (fn !== undefined) {
     overrideStackTrace.delete(error);
-    return f(error, trace);
+    return fn(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();
+    for (let i = trace.length - 1; i >= 0; i--) {
+      const fn = trace[i]?.getFunctionName();
       if (fn && StringPrototypeStartsWith(fn, nodeInternalPrefix)) {
-        ArrayPrototypeSplice(trace, 0, l + 1);
+        ArrayPrototypeSplice(trace, 0, i + 1);
         break;
       }
     }
@@ -828,16 +827,21 @@ function setArrowMessage(err, arrowMessage) {
 // Hide stack lines before the first user code line.
 function hideInternalStackFrames(error) {
   overrideStackTrace.set(error, (error, stackFrames) => {
-    let frames = stackFrames;
+    let result = '';
     if (typeof stackFrames === 'object') {
-      frames = ArrayPrototypeFilter(
-        stackFrames,
-        (frm) => !StringPrototypeStartsWith(frm.getFileName() || '',
-                                            'node:internal'),
-      );
+      for (const frame of stackFrames) {
+        const filename = frame.getFileName();
+        if (!filename || !StringPrototypeStartsWith(filename, 'node:internal')) {
+          result += `\n    at ${frame}`;
+        }
+      }
+    } else {
+      for (const frame of stackFrames) {
+        result += `\n    at ${frame}`;
+      }
     }
-    ArrayPrototypeUnshift(frames, error);
-    return ArrayPrototypeJoin(frames, '\n    at ');
+    result = error + result;
+    return result;
   });
 }
 

From d21f72be44c9a6e8b446ad588c5c06ad3236f079 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 11 Feb 2023 13:36:17 +0100
Subject: [PATCH 02/15] errors: reuse addNumericSeparator from internal inspect

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js       | 15 ++-------------
 lib/internal/util/inspect.js |  1 +
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index b26edeecef08eb..8881a8ea5f2f38 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -743,17 +743,6 @@ function isStackOverflowError(err) {
          err.message === maxStack_ErrorMessage;
 }
 
-// Only use this for integers! Decimal numbers do not work with this function.
-function addNumericalSeparator(val) {
-  let res = '';
-  let i = val.length;
-  const start = val[0] === '-' ? 1 : 0;
-  for (; i >= start + 4; i -= 3) {
-    res = `_${StringPrototypeSlice(val, i - 3, i)}${res}`;
-  }
-  return `${StringPrototypeSlice(val, 0, i)}${res}`;
-}
-
 // Used to enhance the stack that will be picked up by the inspector
 const kEnhanceStackBeforeInspector = Symbol('kEnhanceStackBeforeInspector');
 
@@ -1483,11 +1472,11 @@ E('ERR_OUT_OF_RANGE',
       `The value of "${str}" is out of range.`;
     let received;
     if (NumberIsInteger(input) && MathAbs(input) > 2 ** 32) {
-      received = addNumericalSeparator(String(input));
+      received = lazyInternalUtilInspect().addNumericSeparator(String(input));
     } else if (typeof input === 'bigint') {
       received = String(input);
       if (input > 2n ** 32n || input < -(2n ** 32n)) {
-        received = addNumericalSeparator(received);
+        received = lazyInternalUtilInspect().addNumericSeparator(received);
       }
       received += 'n';
     } else {
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index eb9c74e9146892..bb281bf801e984 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -2408,6 +2408,7 @@ function stripVTControlCharacters(str) {
 }
 
 module.exports = {
+  addNumericSeparator,
   identicalSequenceRange,
   inspect,
   inspectDefaultOptions,

From b2878526dbc95a2eaf9a6ec21e3a01d66a159ca3 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 11 Feb 2023 14:10:15 +0100
Subject: [PATCH 03/15] lib: make sure internal assertions use internal/assert

The assert module should only be used by users, not by Node.js
internally.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/error_serdes.js              | 2 +-
 lib/internal/http2/core.js                | 6 ++++--
 lib/internal/main/print_help.js           | 2 +-
 lib/internal/test_runner/reporter/spec.js | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/internal/error_serdes.js b/lib/internal/error_serdes.js
index 13f3f8b35fdab0..bc06357f0d09c4 100644
--- a/lib/internal/error_serdes.js
+++ b/lib/internal/error_serdes.js
@@ -146,7 +146,7 @@ function deserializeError(error) {
       return buf.toString('utf8');
     }
   }
-  require('assert').fail('This should not happen');
+  require('internal/assert').fail('This should not happen');
 }
 
 module.exports = { serializeError, deserializeError };
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 4fb27d9a52aa3b..dcd7619d37d627 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -42,7 +42,7 @@ const {
 
 assertCrypto();
 
-const assert = require('assert');
+const assert = require('internal/assert');
 const EventEmitter = require('events');
 const fs = require('fs');
 const http = require('http');
@@ -418,7 +418,9 @@ function onSessionHeaders(handle, id, cat, flags, headers, sensitiveHeaders) {
 function tryClose(fd) {
   // Try to close the file descriptor. If closing fails, assert because
   // an error really should not happen at this point.
-  fs.close(fd, assert.ifError);
+  fs.close(fd, (error) => {
+    if (error != null) require('assert').ifError(error);
+  });
 }
 
 // Called when the Http2Stream has finished sending data and is ready for
diff --git a/lib/internal/main/print_help.js b/lib/internal/main/print_help.js
index 4f07aedf1b6e82..631326edb91f8e 100644
--- a/lib/internal/main/print_help.js
+++ b/lib/internal/main/print_help.js
@@ -110,7 +110,7 @@ function getArgDescription(type) {
     case 'kStringList':
       return '...';
     default:
-      require('assert').fail(`unknown option type ${type}`);
+      require('internal/assert').fail(`unknown option type ${type}`);
   }
 }
 
diff --git a/lib/internal/test_runner/reporter/spec.js b/lib/internal/test_runner/reporter/spec.js
index 3637c74111c4b7..5bd3474971d276 100644
--- a/lib/internal/test_runner/reporter/spec.js
+++ b/lib/internal/test_runner/reporter/spec.js
@@ -10,7 +10,7 @@ const {
   SafeMap,
   StringPrototypeRepeat,
 } = primordials;
-const assert = require('assert');
+const assert = require('internal/assert');
 const Transform = require('internal/streams/transform');
 const { inspectWithNoCustomRetry } = require('internal/errors');
 const { green, blue, red, white, gray } = require('internal/util/colors');

From c4f35170d2f68bd26621b0cb998f405e419651b2 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 11 Feb 2023 14:20:04 +0100
Subject: [PATCH 04/15] net: reuse existing aggregateTwoErrors functionality

Instead of providing a similar functionality, just reuse the other
method.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js |  8 --------
 lib/net.js             | 16 +++++++++++-----
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 8881a8ea5f2f38..f0882669b16114 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -166,13 +166,6 @@ const aggregateTwoErrors = hideStackFrames((innerError, outerError) => {
   return innerError || outerError;
 });
 
-const aggregateErrors = hideStackFrames((errors, message, code) => {
-  // eslint-disable-next-line no-restricted-syntax
-  const err = new AggregateError(new SafeArrayIterator(errors), message);
-  err.code = errors[0]?.code;
-  return err;
-});
-
 // Lazily loaded
 let util;
 let assert;
@@ -903,7 +896,6 @@ function formatList(array, type = 'and') {
 module.exports = {
   AbortError,
   aggregateTwoErrors,
-  aggregateErrors,
   captureLargerStackTrace,
   codes,
   connResetException,
diff --git a/lib/net.js b/lib/net.js
index 7ac8a0bdc649c5..d4f3a5fa3f8377 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -101,7 +101,7 @@ const {
     ERR_SOCKET_CLOSED_BEFORE_CONNECTION,
     ERR_MISSING_ARGS,
   },
-  aggregateErrors,
+  aggregateTwoErrors,
   errnoException,
   exceptionWithHostPort,
   genericNodeError,
@@ -1092,7 +1092,7 @@ function internalConnectMultiple(context, canceled) {
 
   // All connections have been tried without success, destroy with error
   if (canceled || context.current === context.addresses.length) {
-    self.destroy(aggregateErrors(context.errors));
+    self.destroy(context.error);
     return;
   }
 
@@ -1118,7 +1118,10 @@ function internalConnectMultiple(context, canceled) {
 
     err = checkBindError(err, localPort, handle);
     if (err) {
-      ArrayPrototypePush(context.errors, exceptionWithHostPort(err, 'bind', localAddress, localPort));
+      context.error = aggregateTwoErrors(
+        exceptionWithHostPort(err, 'bind', localAddress, localPort),
+        context.error,
+      );
       internalConnectMultiple(context);
       return;
     }
@@ -1149,7 +1152,10 @@ function internalConnectMultiple(context, canceled) {
       details = sockname.address + ':' + sockname.port;
     }
 
-    ArrayPrototypePush(context.errors, exceptionWithHostPort(err, 'connect', address, port, details));
+    context.error = aggregateTwoErrors(
+      exceptionWithHostPort(err, 'connect', address, port, details),
+      context.error,
+    );
     internalConnectMultiple(context);
     return;
   }
@@ -1564,7 +1570,7 @@ function afterConnectMultiple(context, status, handle, req, readable, writable)
       ex.localPort = req.localPort;
     }
 
-    ArrayPrototypePush(context.errors, ex);
+    context.error = aggregateTwoErrors(ex, context.error);
 
     // Try the next address
     internalConnectMultiple(context, status === UV_ECANCELED);

From 0d72d4038532637e3b57c878c0b7fdb42438e164 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Sat, 11 Feb 2023 14:35:10 +0100
Subject: [PATCH 05/15] errors: minor performance improvement

The arguments check is moved to the error class setup time instead
of error instance creation time.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js                   | 21 ++++++++++++++-------
 test/parallel/test-errors-systemerror.js |  8 --------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index f0882669b16114..f8a9298dedbdab 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -21,7 +21,6 @@ const {
   ArrayPrototypePush,
   ArrayPrototypeSlice,
   ArrayPrototypeSplice,
-  ArrayPrototypeUnshift,
   Error,
   ErrorCaptureStackTrace,
   ErrorPrototypeToString,
@@ -63,6 +62,7 @@ const kIsNodeError = Symbol('kIsNodeError');
 const isWindows = process.platform === 'win32';
 
 const messages = new SafeMap();
+const messageArguments = new SafeMap();
 const codes = {};
 
 const classRegExp = /^([A-Z][a-z0-9]*)+$/;
@@ -413,6 +413,14 @@ function hideStackFrames(fn) {
 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.
+  if (typeof val === 'string') {
+    const regex = /%[dfijoOs]/g;
+    let expectedLength = 0;
+    while (RegExpPrototypeExec(regex, val) !== null) expectedLength++;
+    messageArguments.set(sym, expectedLength);
+  } else {
+    messageArguments.set(sym, val.length);
+  }
   messages.set(sym, val);
   if (def === SystemError) {
     def = makeSystemErrorWithCode(sym);
@@ -433,28 +441,27 @@ function getMessage(key, args, self) {
 
   assert ??= require('internal/assert');
 
+  const expectedLength = messageArguments.get(key);
+
   if (typeof msg === 'function') {
     assert(
-      msg.length <= args.length, // Default options do not count.
+      expectedLength <= args.length,
       `Code: ${key}; The provided arguments length (${args.length}) does not ` +
         `match the required ones (${msg.length}).`,
     );
     return ReflectApply(msg, self, args);
   }
 
-  const regex = /%[dfijoOs]/g;
-  let expectedLength = 0;
-  while (RegExpPrototypeExec(regex, msg) !== null) expectedLength++;
   assert(
     expectedLength === args.length,
     `Code: ${key}; The provided arguments length (${args.length}) does not ` +
       `match the required ones (${expectedLength}).`,
   );
+
   if (args.length === 0)
     return msg;
 
-  ArrayPrototypeUnshift(args, msg);
-  return ReflectApply(lazyInternalUtilInspect().format, null, args);
+  return lazyInternalUtilInspect().format(msg, ...args);
 }
 
 let uvBinding;
diff --git a/test/parallel/test-errors-systemerror.js b/test/parallel/test-errors-systemerror.js
index 38afbd4aa7d164..73f86a75c6db01 100644
--- a/test/parallel/test-errors-systemerror.js
+++ b/test/parallel/test-errors-systemerror.js
@@ -5,14 +5,6 @@ require('../common');
 const assert = require('assert');
 const { E, SystemError, codes } = require('internal/errors');
 
-assert.throws(
-  () => { new SystemError(); },
-  {
-    name: 'TypeError',
-    message: "Cannot read properties of undefined (reading 'syscall')",
-  }
-);
-
 E('ERR_TEST', 'custom message', SystemError);
 const { ERR_TEST } = codes;
 

From 2d8a4a32b723d42805cba3b503696149f20c22b3 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Mon, 13 Feb 2023 22:12:04 +0100
Subject: [PATCH 06/15] errors: improve NodeError creation performance

This improves the performance by removing the extra stack trace
collection step. Instead, just change the maximum stack trace limit
to the current limit plus 25.
The arbitrary 25 is used as arbitrary upper bound to prevent huge
overhead in recursive methods. The limit should normally not be
reached and should therefore suffice.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js     | 43 +++++++++++---------------------------
 lib/internal/http2/util.js |  6 ++++--
 2 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index f8a9298dedbdab..c5132a17cdb2af 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -83,7 +83,6 @@ 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
@@ -226,7 +225,7 @@ function inspectWithNoCustomRetry(obj, options) {
 class SystemError extends Error {
   constructor(key, context) {
     const limit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit + 25;
     super();
     // Reset the limit and setting the name property.
     if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
@@ -239,8 +238,6 @@ class SystemError extends Error {
     if (context.dest !== undefined)
       message += ` => ${context.dest}`;
 
-    captureLargerStackTrace(this);
-
     this.code = key;
 
     ObjectDefineProperties(this, {
@@ -358,7 +355,7 @@ function makeSystemErrorWithCode(key) {
 function makeNodeErrorWithCode(Base, key) {
   return function NodeError(...args) {
     const limit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit + 25;
     const error = new Base();
     // Reset the limit and setting the name property.
     if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
@@ -388,7 +385,6 @@ function makeNodeErrorWithCode(Base, key) {
         configurable: true,
       },
     });
-    captureLargerStackTrace(error);
     error.code = key;
     return error;
   };
@@ -479,20 +475,6 @@ function uvErrmapGet(name) {
   return MapPrototypeGet(uvBinding.errmap, name);
 }
 
-const captureLargerStackTrace = hideStackFrames(
-  function captureLargerStackTrace(err) {
-    const stackTraceLimitIsWritable = isErrorStackTraceLimitWritable();
-    if (stackTraceLimitIsWritable) {
-      userStackTraceLimit = Error.stackTraceLimit;
-      Error.stackTraceLimit = Infinity;
-    }
-    ErrorCaptureStackTrace(err);
-    // Reset the limit
-    if (stackTraceLimitIsWritable) Error.stackTraceLimit = userStackTraceLimit;
-
-    return err;
-  });
-
 /**
  * This creates an error compatible with errors produced in the C++
  * function UVException using a context object with data assembled in C++.
@@ -521,7 +503,7 @@ const uvException = hideStackFrames(function uvException(ctx) {
   // the stack frames due to the `captureStackTrace()` function that is called
   // later.
   const tmpLimit = Error.stackTraceLimit;
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
   // Pass the message to the constructor instead of setting it on the object
   // to make sure it is the same as the one created in C++
   // eslint-disable-next-line no-restricted-syntax
@@ -543,7 +525,7 @@ const uvException = hideStackFrames(function uvException(ctx) {
     err.dest = dest;
   }
 
-  return captureLargerStackTrace(err);
+  return err;
 });
 
 /**
@@ -573,7 +555,7 @@ const uvExceptionWithHostPort = hideStackFrames(
     // lose the stack frames due to the `captureStackTrace()` function that
     // is called later.
     const tmpLimit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(`${message}${details}`);
     if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
@@ -585,7 +567,7 @@ const uvExceptionWithHostPort = hideStackFrames(
       ex.port = port;
     }
 
-    return captureLargerStackTrace(ex);
+    return ex;
   });
 
 /**
@@ -608,7 +590,7 @@ const errnoException = hideStackFrames(
       `${syscall} ${code} ${original}` : `${syscall} ${code}`;
 
     const tmpLimit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(message);
     if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
@@ -616,7 +598,7 @@ const errnoException = hideStackFrames(
     ex.code = code;
     ex.syscall = syscall;
 
-    return captureLargerStackTrace(ex);
+    return ex;
   });
 
 /**
@@ -652,7 +634,7 @@ const exceptionWithHostPort = hideStackFrames(
     // lose the stack frames due to the `captureStackTrace()` function that
     // is called later.
     const tmpLimit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(`${syscall} ${code}${details}`);
     if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
@@ -664,7 +646,7 @@ const exceptionWithHostPort = hideStackFrames(
       ex.port = port;
     }
 
-    return captureLargerStackTrace(ex);
+    return ex;
   });
 
 /**
@@ -697,7 +679,7 @@ const dnsException = hideStackFrames(function(code, syscall, hostname) {
   // the stack frames due to the `captureStackTrace()` function that is called
   // later.
   const tmpLimit = Error.stackTraceLimit;
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
   // eslint-disable-next-line no-restricted-syntax
   const ex = new Error(message);
   if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
@@ -708,7 +690,7 @@ const dnsException = hideStackFrames(function(code, syscall, hostname) {
     ex.hostname = hostname;
   }
 
-  return captureLargerStackTrace(ex);
+  return ex;
 });
 
 function connResetException(msg) {
@@ -903,7 +885,6 @@ function formatList(array, type = 'and') {
 module.exports = {
   AbortError,
   aggregateTwoErrors,
-  captureLargerStackTrace,
   codes,
   connResetException,
   dnsException,
diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
index 850c8f74019048..badd0335ff04dd 100644
--- a/lib/internal/http2/util.js
+++ b/lib/internal/http2/util.js
@@ -28,9 +28,9 @@ const {
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_HTTP_TOKEN
   },
-  captureLargerStackTrace,
   getMessage,
   hideStackFrames,
+  isErrorStackTraceLimitWritable,
   kIsNodeError,
 } = require('internal/errors');
 
@@ -547,12 +547,14 @@ function mapToHeaders(map,
 
 class NghttpError extends Error {
   constructor(integerCode, customErrorCode) {
+    const { stackTraceLimit } = Error;
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = stackTraceLimit + 25;
     super(customErrorCode ?
       getMessage(customErrorCode, [], null) :
       binding.nghttp2ErrorString(integerCode));
+    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = stackTraceLimit;
     this.code = customErrorCode || 'ERR_HTTP2_ERROR';
     this.errno = integerCode;
-    captureLargerStackTrace(this);
     ObjectDefineProperty(this, kIsNodeError, {
       __proto__: null,
       value: true,

From 69df124d4a94700f5fbac035387546755b22f7af Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Mon, 13 Feb 2023 22:14:52 +0100
Subject: [PATCH 07/15] errors: do not remove already collected stack frames in
 NodeErrors

Currently, already collected user stack frames are removed during
stack frame serialization to adhere to the defined maximum stack
frame limit. This limit is mainly there to prevent extra cpu
overhead while gathering the stack frames. Removing stack frames
after already collecting them won't improve the performance and
only limit the debuggability for users later.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index c5132a17cdb2af..9a92cd94499b50 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -102,10 +102,6 @@ const prepareStackTrace = (globalThis, error, trace) => {
         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 =

From 4ddd43ffdc0663175920ce90956c6b1bddc178dd Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Mon, 13 Feb 2023 22:38:16 +0100
Subject: [PATCH 08/15] errors: format more numbers with numeric separator

This simplifies the implementation of the OUT_OF_RANGE error by
reusing the current inspect functionality.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js                           | 16 +---------------
 lib/internal/streams/readable.js                 |  4 ++--
 lib/internal/util/inspect.js                     |  1 -
 test/parallel/test-buffer-readint.js             |  4 +++-
 test/parallel/test-buffer-readuint.js            |  4 +++-
 test/parallel/test-buffer-writeint.js            | 10 +++++-----
 test/parallel/test-buffer-writeuint.js           | 13 +++++++------
 test/parallel/test-crypto-keygen.js              | 12 ++++++------
 test/parallel/test-crypto-random.js              | 13 +++++++++----
 test/parallel/test-fs-fchmod.js                  |  9 +++++----
 test/parallel/test-fs-fchown.js                  |  7 +++++--
 test/parallel/test-fs-lchmod.js                  |  4 +++-
 test/parallel/test-fs-write-buffer-large.js      |  2 +-
 test/parallel/test-http2-altsvc.js               |  4 +++-
 ...test-http2-client-rststream-before-connect.js |  2 +-
 .../test-http2-client-setLocalWindowSize.js      |  4 +++-
 .../test-http2-client-setNextStreamID-errors.js  |  4 +++-
 .../test-http2-invalidargtypes-errors.js         |  3 ++-
 .../test-inspector-open-port-integer-overflow.js |  2 +-
 test/parallel/test-streams-highwatermark.js      |  4 ++--
 test/parallel/test-zlib-flush-flags.js           |  4 ++--
 21 files changed, 67 insertions(+), 59 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 9a92cd94499b50..e3f768037655c6 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -26,10 +26,8 @@ const {
   ErrorPrototypeToString,
   JSONStringify,
   MapPrototypeGet,
-  MathAbs,
   MathMax,
   Number,
-  NumberIsInteger,
   ObjectAssign,
   ObjectDefineProperty,
   ObjectDefineProperties,
@@ -43,7 +41,6 @@ const {
   SafeArrayIterator,
   SafeMap,
   SafeWeakMap,
-  String,
   StringPrototypeEndsWith,
   StringPrototypeIncludes,
   StringPrototypeSlice,
@@ -1446,18 +1443,7 @@ E('ERR_OUT_OF_RANGE',
     assert(range, 'Missing "range" argument');
     let msg = replaceDefaultBoolean ? str :
       `The value of "${str}" is out of range.`;
-    let received;
-    if (NumberIsInteger(input) && MathAbs(input) > 2 ** 32) {
-      received = lazyInternalUtilInspect().addNumericSeparator(String(input));
-    } else if (typeof input === 'bigint') {
-      received = String(input);
-      if (input > 2n ** 32n || input < -(2n ** 32n)) {
-        received = lazyInternalUtilInspect().addNumericSeparator(received);
-      }
-      received += 'n';
-    } else {
-      received = lazyInternalUtilInspect().inspect(input);
-    }
+    const received = lazyInternalUtilInspect().inspect(input, { numericSeparator: true });
     msg += ` It must be ${range}. Received ${received}`;
     return msg;
   }, RangeError);
diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js
index 7a2aad41ee2baa..54eb8a9073ea7c 100644
--- a/lib/internal/streams/readable.js
+++ b/lib/internal/streams/readable.js
@@ -370,10 +370,10 @@ Readable.prototype.setEncoding = function(enc) {
 };
 
 // Don't raise the hwm > 1GB.
-const MAX_HWM = 0x40000000;
+const MAX_HWM = 1_073_741_824;
 function computeNewHighWaterMark(n) {
   if (n > MAX_HWM) {
-    throw new ERR_OUT_OF_RANGE('size', '<= 1GiB', n);
+    throw new ERR_OUT_OF_RANGE('size', `<= ${MAX_HWM}`, n);
   } else {
     // Get the next highest power of 2 to prevent increasing hwm excessively in
     // tiny amounts.
diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index bb281bf801e984..eb9c74e9146892 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -2408,7 +2408,6 @@ function stripVTControlCharacters(str) {
 }
 
 module.exports = {
-  addNumericSeparator,
   identicalSequenceRange,
   inspect,
   inspectDefaultOptions,
diff --git a/test/parallel/test-buffer-readint.js b/test/parallel/test-buffer-readint.js
index 8758652b28a4a4..fd083950cc1e39 100644
--- a/test/parallel/test-buffer-readint.js
+++ b/test/parallel/test-buffer-readint.js
@@ -2,6 +2,7 @@
 
 require('../common');
 const assert = require('assert');
+const { inspect } = require('util');
 
 // Test OOB
 {
@@ -178,7 +179,8 @@ const assert = require('assert');
             code: 'ERR_OUT_OF_RANGE',
             name: 'RangeError',
             message: 'The value of "offset" is out of range. ' +
-                     `It must be >= 0 and <= ${8 - i}. Received ${offset}`
+                     `It must be >= 0 and <= ${8 - i}. Received ` +
+                     inspect(offset, { numericSeparator: true })
           });
       });
 
diff --git a/test/parallel/test-buffer-readuint.js b/test/parallel/test-buffer-readuint.js
index 31e32bc3df14db..e7b49318de0673 100644
--- a/test/parallel/test-buffer-readuint.js
+++ b/test/parallel/test-buffer-readuint.js
@@ -2,6 +2,7 @@
 
 require('../common');
 const assert = require('assert');
+const { inspect } = require('util');
 
 // Test OOB
 {
@@ -146,7 +147,8 @@ const assert = require('assert');
             code: 'ERR_OUT_OF_RANGE',
             name: 'RangeError',
             message: 'The value of "offset" is out of range. ' +
-                     `It must be >= 0 and <= ${8 - i}. Received ${offset}`
+                     `It must be >= 0 and <= ${8 - i}. Received ` +
+                     inspect(offset, { numericSeparator: true })
           });
       });
 
diff --git a/test/parallel/test-buffer-writeint.js b/test/parallel/test-buffer-writeint.js
index e4b916b1cff27f..659de05a8c6c32 100644
--- a/test/parallel/test-buffer-writeint.js
+++ b/test/parallel/test-buffer-writeint.js
@@ -4,6 +4,7 @@
 
 require('../common');
 const assert = require('assert');
+const { inspect } = require('util');
 const errorOutOfBounds = {
   code: 'ERR_OUT_OF_RANGE',
   name: 'RangeError',
@@ -222,16 +223,14 @@ const errorOutOfBounds = {
         range = `>= -(2 ** ${i * 8 - 1}) and < 2 ** ${i * 8 - 1}`;
       }
       [min - 1, max + 1].forEach((val) => {
-        const received = i > 4 ?
-          String(val).replace(/(\d)(?=(\d\d\d)+(?!\d))/g, '$1_') :
-          val;
         assert.throws(() => {
           data[fn](val, 0, i);
         }, {
           code: 'ERR_OUT_OF_RANGE',
           name: 'RangeError',
           message: 'The value of "value" is out of range. ' +
-                   `It must be ${range}. Received ${received}`
+                   `It must be ${range}. Received ` +
+                   inspect(val, { numericSeparator: true })
         });
       });
 
@@ -251,7 +250,8 @@ const errorOutOfBounds = {
             code: 'ERR_OUT_OF_RANGE',
             name: 'RangeError',
             message: 'The value of "offset" is out of range. ' +
-                     `It must be >= 0 and <= ${8 - i}. Received ${offset}`
+                     `It must be >= 0 and <= ${8 - i}. Received ` +
+                     inspect(offset, { numericSeparator: true })
           });
       });
 
diff --git a/test/parallel/test-buffer-writeuint.js b/test/parallel/test-buffer-writeuint.js
index efacdb7ab48c3b..6260e080a6ae4a 100644
--- a/test/parallel/test-buffer-writeuint.js
+++ b/test/parallel/test-buffer-writeuint.js
@@ -1,6 +1,7 @@
 'use strict';
 
 require('../common');
+const { inspect } = require('util');
 const assert = require('assert');
 
 // We need to check the following things:
@@ -90,7 +91,8 @@ const assert = require('assert');
       {
         code: 'ERR_OUT_OF_RANGE',
         message: 'The value of "value" is out of range. ' +
-                 `It must be >= 0 and <= 65535. Received ${value}`
+                 'It must be >= 0 and <= 65535. Received ' +
+                 inspect(value, { numericSeparator: true })
       }
     );
   });
@@ -170,9 +172,6 @@ const assert = require('assert');
   // Test 1 to 6 bytes.
   for (let i = 1; i <= 6; i++) {
     const range = i < 5 ? `= ${val - 1}` : ` 2 ** ${i * 8}`;
-    const received = i > 4 ?
-      String(val).replace(/(\d)(?=(\d\d\d)+(?!\d))/g, '$1_') :
-      val;
     ['writeUIntBE', 'writeUIntLE'].forEach((fn) => {
       assert.throws(() => {
         data[fn](val, 0, i);
@@ -180,7 +179,8 @@ const assert = require('assert');
         code: 'ERR_OUT_OF_RANGE',
         name: 'RangeError',
         message: 'The value of "value" is out of range. ' +
-                 `It must be >= 0 and <${range}. Received ${received}`
+                 `It must be >= 0 and <${range}. Received ` +
+                 inspect(val, { numericSeparator: true })
       });
 
       ['', '0', null, {}, [], () => {}, true, false].forEach((o) => {
@@ -199,7 +199,8 @@ const assert = require('assert');
             code: 'ERR_OUT_OF_RANGE',
             name: 'RangeError',
             message: 'The value of "offset" is out of range. ' +
-                     `It must be >= 0 and <= ${8 - i}. Received ${offset}`
+                     `It must be >= 0 and <= ${8 - i}. Received ` +
+                     inspect(offset, { numericSeparator: true })
           });
       });
 
diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
index 1a77266b057041..29c5790890afa2 100644
--- a/test/parallel/test-crypto-keygen.js
+++ b/test/parallel/test-crypto-keygen.js
@@ -1147,7 +1147,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       message:
         'The value of "options.modulusLength" is out of range. ' +
         'It must be an integer. ' +
-        `Received ${inspect(modulusLength)}`
+        `Received ${inspect(modulusLength, { numericSeparator: true })}`
     });
   }
 
@@ -1273,7 +1273,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       message:
         'The value of "options.divisorLength" is out of range. ' +
         'It must be an integer. ' +
-        `Received ${inspect(divisorLength)}`
+        `Received ${inspect(divisorLength, { numericSeparator: true })}`
     });
   }
 
@@ -1288,7 +1288,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
       message:
         'The value of "options.divisorLength" is out of range. ' +
         'It must be >= 0 && <= 2147483647. ' +
-        `Received ${inspect(divisorLength)}`
+        `Received ${inspect(divisorLength, { numericSeparator: true })}`
     });
   }
 }
@@ -1413,7 +1413,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     code: 'ERR_OUT_OF_RANGE',
     message: 'The value of "options.primeLength" is out of range. ' +
              'It must be >= 0 && <= 2147483647. ' +
-             'Received 2147483648',
+             'Received 2_147_483_648',
   });
 
   assert.throws(() => {
@@ -1438,7 +1438,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     code: 'ERR_OUT_OF_RANGE',
     message: 'The value of "options.generator" is out of range. ' +
              'It must be >= 0 && <= 2147483647. ' +
-             'Received 2147483648',
+             'Received 2_147_483_648',
   });
 
   assert.throws(() => {
@@ -1529,7 +1529,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     code: 'ERR_OUT_OF_RANGE',
     message: 'The value of "options.saltLength" is out of range. ' +
              'It must be >= 0 && <= 2147483647. ' +
-             'Received 2147483648'
+             'Received 2_147_483_648'
   });
 
   assert.throws(() => {
diff --git a/test/parallel/test-crypto-random.js b/test/parallel/test-crypto-random.js
index ceaa859a0e0316..e9e796462aa404 100644
--- a/test/parallel/test-crypto-random.js
+++ b/test/parallel/test-crypto-random.js
@@ -28,6 +28,7 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const crypto = require('crypto');
+const { inspect } = require('util');
 const { kMaxLength } = require('buffer');
 
 const kMaxInt32 = 2 ** 31 - 1;
@@ -54,7 +55,8 @@ common.expectWarning('DeprecationWarning',
         code: 'ERR_OUT_OF_RANGE',
         name: 'RangeError',
         message: 'The value of "size" is out of range. It must be >= 0 && <= ' +
-                 `${kMaxPossibleLength}. Received ${value}`
+                 `${kMaxPossibleLength}. Received ` +
+                 inspect(value, { numericSeparator: true })
       };
       assert.throws(() => f(value), errObj);
       assert.throws(() => f(value, common.mustNotCall()), errObj);
@@ -252,7 +254,8 @@ common.expectWarning('DeprecationWarning',
         code: 'ERR_OUT_OF_RANGE',
         name: 'RangeError',
         message: 'The value of "offset" is out of range. ' +
-                 `It must be >= 0 && <= 10. Received ${offsetSize}`
+                 'It must be >= 0 && <= 10. Received ' +
+                 inspect(offsetSize, { numericSeparator: true })
       };
 
       assert.throws(() => crypto.randomFillSync(buf, offsetSize), errObj);
@@ -262,7 +265,8 @@ common.expectWarning('DeprecationWarning',
         errObj);
 
       errObj.message = 'The value of "size" is out of range. It must be >= ' +
-                       `0 && <= ${kMaxPossibleLength}. Received ${offsetSize}`;
+                       `0 && <= ${kMaxPossibleLength}. Received ` +
+                       inspect(offsetSize, { numericSeparator: true });
       assert.throws(() => crypto.randomFillSync(buf, 1, offsetSize), errObj);
 
       assert.throws(
@@ -295,7 +299,8 @@ assert.throws(
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
     message: 'The value of "size" is out of range. ' +
-             `It must be >= 0 && <= ${kMaxPossibleLength}. Received 4294967296`
+             `It must be >= 0 && <= ${kMaxPossibleLength}. ` +
+             'Received 4_294_967_296'
   }
 );
 
diff --git a/test/parallel/test-fs-fchmod.js b/test/parallel/test-fs-fchmod.js
index da37080ddeedd8..39c8a27dcfa5cb 100644
--- a/test/parallel/test-fs-fchmod.js
+++ b/test/parallel/test-fs-fchmod.js
@@ -2,6 +2,7 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
+const { inspect } = require('util');
 
 // This test ensures that input for fchmod is valid, testing for valid
 // inputs for fd and mode
@@ -35,8 +36,8 @@ assert.throws(() => fs.fchmod(1, '123x'), {
   const errObj = {
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
-    message: 'The value of "fd" is out of range. It must be >= 0 && <= ' +
-             `2147483647. Received ${input}`
+    message: 'The value of "fd" is out of range. It must be >= 0 && <= 2' +
+             `147483647. Received ${inspect(input, { numericSeparator: true })}`
   };
   assert.throws(() => fs.fchmod(input), errObj);
   assert.throws(() => fs.fchmodSync(input), errObj);
@@ -46,8 +47,8 @@ assert.throws(() => fs.fchmod(1, '123x'), {
   const errObj = {
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
-    message: 'The value of "mode" is out of range. It must be >= 0 && <= ' +
-             `4294967295. Received ${input}`
+    message: 'The value of "mode" is out of range. It must be >= 0 && <= 4' +
+             `294967295. Received ${inspect(input, { numericSeparator: true })}`
   };
 
   assert.throws(() => fs.fchmod(1, input), errObj);
diff --git a/test/parallel/test-fs-fchown.js b/test/parallel/test-fs-fchown.js
index 03a9ef3780ae6b..b234e4634ea664 100644
--- a/test/parallel/test-fs-fchown.js
+++ b/test/parallel/test-fs-fchown.js
@@ -3,6 +3,7 @@
 require('../common');
 const assert = require('assert');
 const fs = require('fs');
+const { inspect } = require('util');
 
 function testFd(input, errObj) {
   assert.throws(() => fs.fchown(input), errObj);
@@ -49,11 +50,13 @@ function testGid(input, errObj) {
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
     message: 'The value of "fd" is out of range. It must be ' +
-             `>= 0 && <= 2147483647. Received ${input}`
+             '>= 0 && <= 2147483647. Received ' +
+             inspect(input, { numericSeparator: true })
   };
   testFd(input, errObj);
   errObj.message = 'The value of "uid" is out of range. It must be >= -1 && ' +
-    `<= 4294967295. Received ${input}`;
+    '<= 4294967295. Received ' +
+    inspect(input, { numericSeparator: true });
   testUid(input, errObj);
   errObj.message = errObj.message.replace('uid', 'gid');
   testGid(input, errObj);
diff --git a/test/parallel/test-fs-lchmod.js b/test/parallel/test-fs-lchmod.js
index ae5cb3ba2a230a..b68c635ba50b3b 100644
--- a/test/parallel/test-fs-lchmod.js
+++ b/test/parallel/test-fs-lchmod.js
@@ -3,6 +3,7 @@
 const common = require('../common');
 const assert = require('assert');
 const fs = require('fs');
+const { inspect } = require('util');
 const { promises } = fs;
 const f = __filename;
 
@@ -58,7 +59,8 @@ assert.throws(() => fs.lchmodSync(f, '123x'), {
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
     message: 'The value of "mode" is out of range. It must be >= 0 && <= ' +
-             `4294967295. Received ${input}`
+             '4294967295. Received ' +
+             inspect(input, { numericSeparator: true })
   };
 
   assert.rejects(promises.lchmod(f, input, () => {}), errObj);
diff --git a/test/parallel/test-fs-write-buffer-large.js b/test/parallel/test-fs-write-buffer-large.js
index a80072354eaa0f..920835c27cd9a1 100644
--- a/test/parallel/test-fs-write-buffer-large.js
+++ b/test/parallel/test-fs-write-buffer-large.js
@@ -33,7 +33,7 @@ fs.open(filename, 'w', 0o644, common.mustSucceed((fd) => {
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
     message: 'The value of "length" is out of range. ' +
-      'It must be >= 0 && <= 2147483647. Received 2147483648'
+      'It must be >= 0 && <= 2147483647. Received 2_147_483_648'
   });
 
   fs.closeSync(fd);
diff --git a/test/parallel/test-http2-altsvc.js b/test/parallel/test-http2-altsvc.js
index c5abfc33265330..6d46dfc711d949 100644
--- a/test/parallel/test-http2-altsvc.js
+++ b/test/parallel/test-http2-altsvc.js
@@ -6,6 +6,7 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 const Countdown = require('../common/countdown');
 
 const server = http2.createServer();
@@ -34,7 +35,8 @@ server.on('session', common.mustCall((session) => {
         code: 'ERR_OUT_OF_RANGE',
         name: 'RangeError',
         message: 'The value of "originOrStream" is out of ' +
-                 `range. It must be > 0 && < 4294967296. Received ${input}`
+                 'range. It must be > 0 && < 4294967296. Received ' +
+                 inspect(input, { numericSeparator: true })
       }
     );
   });
diff --git a/test/parallel/test-http2-client-rststream-before-connect.js b/test/parallel/test-http2-client-rststream-before-connect.js
index bc0cb5ff619dc0..6184baf8641bf3 100644
--- a/test/parallel/test-http2-client-rststream-before-connect.js
+++ b/test/parallel/test-http2-client-rststream-before-connect.js
@@ -24,7 +24,7 @@ server.listen(0, common.mustCall(() => {
       name: 'RangeError',
       code: 'ERR_OUT_OF_RANGE',
       message: 'The value of "code" is out of range. It must be ' +
-               '>= 0 && <= 4294967295. Received 4294967296'
+               '>= 0 && <= 4294967295. Received 4_294_967_296'
     }
   );
   assert.strictEqual(req.closed, false);
diff --git a/test/parallel/test-http2-client-setLocalWindowSize.js b/test/parallel/test-http2-client-setLocalWindowSize.js
index 8e3b57ed0c1a6b..04501894dc651d 100644
--- a/test/parallel/test-http2-client-setLocalWindowSize.js
+++ b/test/parallel/test-http2-client-setLocalWindowSize.js
@@ -6,6 +6,7 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 {
   const server = http2.createServer();
@@ -34,7 +35,8 @@ const http2 = require('http2');
           name: 'RangeError',
           code: 'ERR_OUT_OF_RANGE',
           message: 'The value of "windowSize" is out of range.' +
-            ' It must be >= 0 && <= 2147483647. Received ' + outOfRangeNum
+            ' It must be >= 0 && <= 2147483647. Received ' +
+            inspect(outOfRangeNum, { numericSeparator: true })
         }
       );
 
diff --git a/test/parallel/test-http2-client-setNextStreamID-errors.js b/test/parallel/test-http2-client-setNextStreamID-errors.js
index aace5845252af6..948e0645dd7dd8 100644
--- a/test/parallel/test-http2-client-setNextStreamID-errors.js
+++ b/test/parallel/test-http2-client-setNextStreamID-errors.js
@@ -6,6 +6,7 @@ if (!common.hasCrypto)
 
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 const server = http2.createServer();
 server.on('stream', (stream) => {
@@ -34,7 +35,8 @@ server.listen(0, common.mustCall(() => {
         name: 'RangeError',
         code: 'ERR_OUT_OF_RANGE',
         message: 'The value of "id" is out of range.' +
-           ' It must be > 0 and <= 4294967295. Received ' + outOfRangeNum
+           ' It must be > 0 and <= 4294967295. Received ' +
+           inspect(outOfRangeNum, { numericSeparator: true })
       }
     );
 
diff --git a/test/parallel/test-http2-invalidargtypes-errors.js b/test/parallel/test-http2-invalidargtypes-errors.js
index 5f126b060a9f49..00077142823cce 100644
--- a/test/parallel/test-http2-invalidargtypes-errors.js
+++ b/test/parallel/test-http2-invalidargtypes-errors.js
@@ -5,6 +5,7 @@ if (!common.hasCrypto)
   common.skip('missing crypto');
 const assert = require('assert');
 const http2 = require('http2');
+const { inspect } = require('util');
 
 const server = http2.createServer();
 
@@ -35,7 +36,7 @@ server.on('stream', common.mustCall((stream) => {
         name: 'RangeError',
         message: 'The value of "code" is out of range. ' +
                  'It must be >= 0 && <= 4294967295. ' +
-                 `Received ${code}`
+                 `Received ${inspect(code, { numericSeparator: true })}`
       }
     );
   });
diff --git a/test/parallel/test-inspector-open-port-integer-overflow.js b/test/parallel/test-inspector-open-port-integer-overflow.js
index 0f9a4799d0642a..e9061a5d6cfe94 100644
--- a/test/parallel/test-inspector-open-port-integer-overflow.js
+++ b/test/parallel/test-inspector-open-port-integer-overflow.js
@@ -13,5 +13,5 @@ const inspector = require('inspector');
 assert.throws(() => inspector.open(99999), {
   name: 'RangeError',
   code: 'ERR_OUT_OF_RANGE',
-  message: 'The value of "port" is out of range. It must be >= 0 && <= 65535. Received 99999'
+  message: 'The value of "port" is out of range. It must be >= 0 && <= 65535. Received 99_999'
 });
diff --git a/test/parallel/test-streams-highwatermark.js b/test/parallel/test-streams-highwatermark.js
index 7018c5bf29ce08..50dc0e33237906 100644
--- a/test/parallel/test-streams-highwatermark.js
+++ b/test/parallel/test-streams-highwatermark.js
@@ -81,7 +81,7 @@ const { inspect } = require('util');
   assert.throws(() => readable.read(hwm), common.expectsError({
     code: 'ERR_OUT_OF_RANGE',
     message: 'The value of "size" is out of range.' +
-             ' It must be <= 1GiB. Received ' +
-             hwm,
+             ' It must be <= 1073741824. Received ' +
+             inspect(hwm, { numericSeparator: true }),
   }));
 }
diff --git a/test/parallel/test-zlib-flush-flags.js b/test/parallel/test-zlib-flush-flags.js
index 3d8e609adb6269..5c2652ca97598b 100644
--- a/test/parallel/test-zlib-flush-flags.js
+++ b/test/parallel/test-zlib-flush-flags.js
@@ -21,7 +21,7 @@ assert.throws(
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
     message: 'The value of "options.flush" is out of range. It must ' +
-             'be >= 0 and <= 5. Received 10000'
+             'be >= 0 and <= 5. Received 10_000'
   }
 );
 
@@ -43,6 +43,6 @@ assert.throws(
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError',
     message: 'The value of "options.finishFlush" is out of range. It must ' +
-             'be >= 0 and <= 5. Received 10000'
+             'be >= 0 and <= 5. Received 10_000'
   }
 );

From 087b0bc0f6565286dc8ba37e9f7f9385c2aca424 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 14 Feb 2023 01:30:42 +0100
Subject: [PATCH 09/15] errors: improve NodeError creation performance

The former implementation over allocated stack frames during error
creation to substitute for internal frames that should be hidden
to the user. This could have caused a significant performance
overhead in deeply nested code.

As side effect also simplify the setting of stackTraceLimits and
improve the error related benchmarks.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 benchmark/error/error.js                     | 14 ----
 benchmark/{misc => error}/hidestackframes.js | 26 +++++--
 benchmark/error/node-error.js                | 21 ++++-
 lib/assert.js                                | 13 ++--
 lib/internal/assert/assertion_error.js       |  8 +-
 lib/internal/debugger/inspect_client.js      | 13 ++--
 lib/internal/errors.js                       | 82 +++++++-------------
 lib/internal/http2/util.js                   |  4 -
 lib/internal/process/promises.js             |  8 +-
 lib/internal/process/warning.js              |  8 +-
 lib/repl.js                                  |  8 +-
 11 files changed, 94 insertions(+), 111 deletions(-)
 delete mode 100644 benchmark/error/error.js
 rename benchmark/{misc => error}/hidestackframes.js (66%)

diff --git a/benchmark/error/error.js b/benchmark/error/error.js
deleted file mode 100644
index c856f3e07f6bea..00000000000000
--- a/benchmark/error/error.js
+++ /dev/null
@@ -1,14 +0,0 @@
-'use strict';
-
-const common = require('../common.js');
-
-const bench = common.createBenchmark(main, {
-  n: [1e7],
-});
-
-function main({ n }) {
-  bench.start();
-  for (let i = 0; i < n; ++i)
-    new Error('test');
-  bench.end(n);
-}
diff --git a/benchmark/misc/hidestackframes.js b/benchmark/error/hidestackframes.js
similarity index 66%
rename from benchmark/misc/hidestackframes.js
rename to benchmark/error/hidestackframes.js
index b28be725a30969..c57b84df16242b 100644
--- a/benchmark/misc/hidestackframes.js
+++ b/benchmark/error/hidestackframes.js
@@ -5,12 +5,23 @@ 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],
+  n: [1e4],
+  nested: [1, 0],
 }, {
   flags: ['--expose-internals'],
 });
 
-function main({ n, type }) {
+function nestIt(fn, i = 25) {
+  const nested = (...args) => {
+    return fn(...args);
+  };
+  if (i === 0) {
+    return nested;
+  }
+  return nestIt(nested, i - 1);
+}
+
+function main({ n, type, nested }) {
   const {
     hideStackFrames,
     codes: {
@@ -24,9 +35,14 @@ function main({ n, type }) {
     }
   };
 
-  let fn = testfn;
-  if (type.startsWith('hide-stackframe'))
-    fn = hideStackFrames(testfn);
+  let fn = type.startsWith('hide-stackframe') ?
+    hideStackFrames(testfn) :
+    testfn;
+
+  if (nested) {
+    fn = nestIt(fn);
+  }
+
   let value = 42;
   if (type.endsWith('-throw'))
     value = 'err';
diff --git a/benchmark/error/node-error.js b/benchmark/error/node-error.js
index 3a0aef91f04a06..8f9cb5cad621fc 100644
--- a/benchmark/error/node-error.js
+++ b/benchmark/error/node-error.js
@@ -1,21 +1,34 @@
 'use strict';
 
 const common = require('../common');
+const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
-  n: [1e7],
+  n: [1e5],
+  type: ['node', 'regular'],
 }, {
   flags: ['--expose-internals'],
 });
 
-function main({ n }) {
+function main({ n, type }) {
   const {
     codes: {
       ERR_INVALID_STATE,
     },
   } = require('internal/errors');
+
+  const Clazz = type === 'node' ? ERR_INVALID_STATE.TypeError : (() => {
+    class Foo extends TypeError {}
+    Foo.prototype.constructor = TypeError;
+    return Foo;
+  })();
+
   bench.start();
-  for (let i = 0; i < n; ++i)
-    new ERR_INVALID_STATE.TypeError('test');
+  let length = 0;
+  for (let i = 0; i < n; i++) {
+    const error = new Clazz('test' + i);
+    length += error.name.length;
+  }
   bench.end(n);
+  assert(length);
 }
diff --git a/lib/assert.js b/lib/assert.js
index 04c2dd3bfcfdfb..cdf6fa3b4e9260 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -57,7 +57,7 @@ const {
     ERR_INVALID_RETURN_VALUE,
     ERR_MISSING_ARGS,
   },
-  isErrorStackTraceLimitWritable,
+  setStackTraceLimit,
   overrideStackTrace,
 } = require('internal/errors');
 const AssertionError = require('internal/assert/assertion_error');
@@ -282,16 +282,15 @@ function parseCode(code, offset) {
 }
 
 function getErrMessage(message, fn) {
-  const tmpLimit = Error.stackTraceLimit;
-  const errorStackTraceLimitIsWritable = isErrorStackTraceLimitWritable();
+  const { stackTraceLimit } = Error;
   // Make sure the limit is set to 1. Otherwise it could fail (<= 0) or it
   // does to much work.
-  if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 1;
+  setStackTraceLimit(1);
   // We only need the stack trace. To minimize the overhead use an object
   // instead of an error.
   const err = {};
   ErrorCaptureStackTrace(err, fn);
-  if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;
+  setStackTraceLimit(stackTraceLimit);
 
   overrideStackTrace.set(err, (_, stack) => stack);
   const call = err.stack[0];
@@ -323,7 +322,7 @@ function getErrMessage(message, fn) {
   try {
     // Set the stack trace limit to zero. This makes sure unexpected token
     // errors are handled faster.
-    if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 0;
+    setStackTraceLimit(0);
 
     if (filename) {
       if (decoder === undefined) {
@@ -368,7 +367,7 @@ function getErrMessage(message, fn) {
     errorCache.set(identifier, undefined);
   } finally {
     // Reset limit.
-    if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = tmpLimit;
+    setStackTraceLimit(stackTraceLimit);
     if (fd !== undefined)
       closeSync(fd);
   }
diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
index 851662fe4c5fa2..657ebdebf03cb1 100644
--- a/lib/internal/assert/assertion_error.js
+++ b/lib/internal/assert/assertion_error.js
@@ -24,7 +24,7 @@ const colors = require('internal/util/colors');
 const {
   validateObject,
 } = require('internal/validators');
-const { isErrorStackTraceLimitWritable } = require('internal/errors');
+const { setStackTraceLimit } = require('internal/errors');
 
 
 const kReadableOperator = {
@@ -337,8 +337,8 @@ class AssertionError extends Error {
       expected
     } = options;
 
-    const limit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+    const { stackTraceLimit } = Error;
+    setStackTraceLimit(0);
 
     if (message != null) {
       super(String(message));
@@ -421,7 +421,7 @@ class AssertionError extends Error {
       }
     }
 
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
+    setStackTraceLimit(stackTraceLimit);
 
     this.generatedMessage = !message;
     ObjectDefineProperty(this, 'name', {
diff --git a/lib/internal/debugger/inspect_client.js b/lib/internal/debugger/inspect_client.js
index e467899fb3e746..24a9c683c68f2f 100644
--- a/lib/internal/debugger/inspect_client.js
+++ b/lib/internal/debugger/inspect_client.js
@@ -2,7 +2,6 @@
 
 const {
   ArrayPrototypePush,
-  ErrorCaptureStackTrace,
   FunctionPrototypeBind,
   JSONParse,
   JSONStringify,
@@ -12,10 +11,15 @@ const {
 
 const Buffer = require('buffer').Buffer;
 const crypto = require('crypto');
-const { ERR_DEBUGGER_ERROR } = require('internal/errors').codes;
 const { EventEmitter } = require('events');
 const http = require('http');
 const URL = require('url');
+const {
+  hideStackFrames,
+  codes: {
+    ERR_DEBUGGER_ERROR,
+  },
+} = require('internal/errors');
 
 const debuglog = require('internal/util/debuglog').debuglog('inspect');
 
@@ -40,12 +44,11 @@ const kMaskingKeyWidthInBytes = 4;
 // https://tools.ietf.org/html/rfc6455#section-1.3
 const WEBSOCKET_HANDSHAKE_GUID = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11';
 
-function unpackError({ code, message }) {
+const unpackError = hideStackFrames(({ code, message }) => {
   const err = new ERR_DEBUGGER_ERROR(`${message}`);
   err.code = code;
-  ErrorCaptureStackTrace(err, unpackError);
   return err;
-}
+});
 
 function validateHandshake(requestKey, responseKey) {
   const expectedResponseKeyBase = requestKey + WEBSOCKET_HANDSHAKE_GUID;
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index e3f768037655c6..5948439abd36e4 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -19,10 +19,10 @@ const {
   ArrayPrototypeJoin,
   ArrayPrototypeMap,
   ArrayPrototypePush,
+  ArrayPrototypeShift,
   ArrayPrototypeSlice,
   ArrayPrototypeSplice,
   Error,
-  ErrorCaptureStackTrace,
   ErrorPrototypeToString,
   JSONStringify,
   MapPrototypeGet,
@@ -31,10 +31,7 @@ const {
   ObjectAssign,
   ObjectDefineProperty,
   ObjectDefineProperties,
-  ObjectIsExtensible,
-  ObjectGetOwnPropertyDescriptor,
   ObjectKeys,
-  ObjectPrototypeHasOwnProperty,
   RangeError,
   ReflectApply,
   RegExpPrototypeExec,
@@ -92,13 +89,17 @@ const prepareStackTrace = (globalThis, error, trace) => {
 
   const firstFrame = trace[0]?.getFunctionName();
   if (firstFrame && StringPrototypeStartsWith(firstFrame, nodeInternalPrefix)) {
-    for (let i = trace.length - 1; i >= 0; i--) {
+    let i = trace.length - 1;
+    for (; i > 0; i--) {
       const fn = trace[i]?.getFunctionName();
       if (fn && StringPrototypeStartsWith(fn, nodeInternalPrefix)) {
-        ArrayPrototypeSplice(trace, 0, i + 1);
+        trace = ArrayPrototypeSlice(trace, i + 1);
         break;
       }
     }
+    if (i === 0) {
+      ArrayPrototypeShift(trace);
+    }
   }
 
   const globalOverride =
@@ -180,21 +181,24 @@ function lazyBuffer() {
   return buffer;
 }
 
-function isErrorStackTraceLimitWritable() {
-  // Do no touch Error.stackTraceLimit as V8 would attempt to install
-  // it again during deserialization.
-  if (require('internal/v8/startup_snapshot').namespace.isBuildingSnapshot()) {
-    return false;
-  }
-
-  const desc = ObjectGetOwnPropertyDescriptor(Error, 'stackTraceLimit');
-  if (desc === undefined) {
-    return ObjectIsExtensible(Error);
+let stackTraceLimitWritable = 0;
+function setStackTraceLimit(limit) {
+  if (stackTraceLimitWritable === 1) {
+    try {
+      Error.stackTraceLimit = limit;
+    } catch {
+      stackTraceLimitWritable = -1;
+    }
+  } else if (stackTraceLimitWritable === 0) {
+    // Do no touch Error.stackTraceLimit as V8 would attempt to install
+    // it again during deserialization.
+    if (require('internal/v8/startup_snapshot').namespace.isBuildingSnapshot()) {
+      stackTraceLimitWritable = -1;
+      return;
+    }
+    stackTraceLimitWritable = 1;
+    setStackTraceLimit(limit);
   }
-
-  return ObjectPrototypeHasOwnProperty(desc, 'writable') ?
-    desc.writable :
-    desc.set !== undefined;
 }
 
 function inspectWithNoCustomRetry(obj, options) {
@@ -217,11 +221,7 @@ function inspectWithNoCustomRetry(obj, options) {
 // and may have .path and .dest.
 class SystemError extends Error {
   constructor(key, context) {
-    const limit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit + 25;
     super();
-    // Reset the limit and setting the name property.
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
     const prefix = getMessage(key, [], this);
     let message = `${prefix}: ${context.syscall} returned ` +
                   `${context.code} (${context.message})`;
@@ -347,11 +347,7 @@ function makeSystemErrorWithCode(key) {
 
 function makeNodeErrorWithCode(Base, key) {
   return function NodeError(...args) {
-    const limit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit + 25;
     const error = new Base();
-    // Reset the limit and setting the name property.
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = limit;
     const message = getMessage(key, args, error);
     ObjectDefineProperties(error, {
       [kIsNodeError]: {
@@ -492,16 +488,10 @@ const uvException = hideStackFrames(function uvException(ctx) {
     message += ` -> '${dest}'`;
   }
 
-  // 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;
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
   // Pass the message to the constructor instead of setting it on the object
   // to make sure it is the same as the one created in C++
   // eslint-disable-next-line no-restricted-syntax
   const err = new Error(message);
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
 
   for (const prop of ObjectKeys(ctx)) {
     if (prop === 'message' || prop === 'path' || prop === 'dest') {
@@ -544,14 +534,8 @@ const uvExceptionWithHostPort = hideStackFrames(
       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;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(`${message}${details}`);
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
     ex.code = code;
     ex.errno = err;
     ex.syscall = syscall;
@@ -582,11 +566,8 @@ const errnoException = hideStackFrames(
     const message = original ?
       `${syscall} ${code} ${original}` : `${syscall} ${code}`;
 
-    const tmpLimit = Error.stackTraceLimit;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(message);
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
     ex.errno = err;
     ex.code = code;
     ex.syscall = syscall;
@@ -623,14 +604,8 @@ const exceptionWithHostPort = hideStackFrames(
       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;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
     // eslint-disable-next-line no-restricted-syntax
     const ex = new Error(`${syscall} ${code}${details}`);
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
     ex.errno = err;
     ex.code = code;
     ex.syscall = syscall;
@@ -668,14 +643,9 @@ const dnsException = hideStackFrames(function(code, syscall, hostname) {
     }
   }
   const message = `${syscall} ${code}${hostname ? ` ${hostname}` : ''}`;
-  // 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;
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit + 25;
+
   // eslint-disable-next-line no-restricted-syntax
   const ex = new Error(message);
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpLimit;
   ex.errno = errno;
   ex.code = code;
   ex.syscall = syscall;
@@ -893,7 +863,7 @@ module.exports = {
   hideInternalStackFrames,
   hideStackFrames,
   inspectWithNoCustomRetry,
-  isErrorStackTraceLimitWritable,
+  setStackTraceLimit,
   isStackOverflowError,
   kEnhanceStackBeforeInspector,
   kIsNodeError,
diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
index badd0335ff04dd..b022481bc8bc44 100644
--- a/lib/internal/http2/util.js
+++ b/lib/internal/http2/util.js
@@ -30,7 +30,6 @@ const {
   },
   getMessage,
   hideStackFrames,
-  isErrorStackTraceLimitWritable,
   kIsNodeError,
 } = require('internal/errors');
 
@@ -547,12 +546,9 @@ function mapToHeaders(map,
 
 class NghttpError extends Error {
   constructor(integerCode, customErrorCode) {
-    const { stackTraceLimit } = Error;
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = stackTraceLimit + 25;
     super(customErrorCode ?
       getMessage(customErrorCode, [], null) :
       binding.nghttp2ErrorString(integerCode));
-    if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = stackTraceLimit;
     this.code = customErrorCode || 'ERR_HTTP2_ERROR';
     this.errno = integerCode;
     ObjectDefineProperty(this, kIsNodeError, {
diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js
index d80ce1ef764a00..0c96e52e66d289 100644
--- a/lib/internal/process/promises.js
+++ b/lib/internal/process/promises.js
@@ -36,7 +36,7 @@ const {
     trigger_async_id_symbol: kTriggerAsyncIdSymbol,
   },
 } = require('internal/async_hooks');
-const { isErrorStackTraceLimitWritable } = require('internal/errors');
+const { setStackTraceLimit } = require('internal/errors');
 
 // *Must* match Environment::TickInfo::Fields in src/env.h.
 const kHasRejectionToWarn = 1;
@@ -315,11 +315,11 @@ function processPromiseRejections() {
 
 function getErrorWithoutStack(name, message) {
   // Reset the stack to prevent any overhead.
-  const tmp = Error.stackTraceLimit;
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+  const { stackTraceLimit } = Error;
+  setStackTraceLimit(0);
   // eslint-disable-next-line no-restricted-syntax
   const err = new Error(message);
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmp;
+  setStackTraceLimit(stackTraceLimit);
   ObjectDefineProperty(err, 'name', {
     __proto__: null,
     value: name,
diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
index 3ce00004dab476..fb1b50b7e664bd 100644
--- a/lib/internal/process/warning.js
+++ b/lib/internal/process/warning.js
@@ -13,7 +13,7 @@ const {
   codes: {
     ERR_INVALID_ARG_TYPE,
   },
-  isErrorStackTraceLimitWritable,
+  setStackTraceLimit,
 } = require('internal/errors');
 const { validateString } = require('internal/validators');
 
@@ -174,11 +174,11 @@ function createWarningObject(warning, type, code, ctor, detail) {
   assert(typeof warning === 'string');
   // Improve error creation performance by skipping the error frames.
   // They are added in the `captureStackTrace()` function below.
-  const tmpStackLimit = Error.stackTraceLimit;
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+  const { stackTraceLimit } = Error;
+  setStackTraceLimit(0);
   // eslint-disable-next-line no-restricted-syntax
   warning = new Error(warning);
-  if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmpStackLimit;
+  setStackTraceLimit(stackTraceLimit);
   warning.name = String(type || 'Warning');
   if (code !== undefined) warning.code = code;
   if (detail !== undefined) warning.detail = detail;
diff --git a/lib/repl.js b/lib/repl.js
index 241e25f0f2095a..3356b88e48b605 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -143,7 +143,7 @@ const {
     ERR_INVALID_REPL_INPUT,
     ERR_SCRIPT_EXECUTION_INTERRUPTED,
   },
-  isErrorStackTraceLimitWritable,
+  setStackTraceLimit,
   overrideStackTrace,
 } = require('internal/errors');
 const { sendInspectorCommand } = require('internal/util/inspector');
@@ -601,10 +601,10 @@ function REPLServer(prompt,
         if (self.breakEvalOnSigint) {
           const interrupt = new Promise((resolve, reject) => {
             sigintListener = () => {
-              const tmp = Error.stackTraceLimit;
-              if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = 0;
+              const { stackTraceLimit } = Error;
+              setStackTraceLimit(0);
               const err = new ERR_SCRIPT_EXECUTION_INTERRUPTED();
-              if (isErrorStackTraceLimitWritable()) Error.stackTraceLimit = tmp;
+              setStackTraceLimit(stackTraceLimit);
               reject(err);
             };
             prioritizedSigintQueue.add(sigintListener);

From 6ec603c667aed8cdf81da881963659cd260a937a Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 14 Feb 2023 04:03:09 +0100
Subject: [PATCH 10/15] lib: add new to construct errors

This makes sure all error instances use `new` to call the
constructor.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/crypto/hkdf.js        | 2 +-
 lib/internal/fs/streams.js         | 4 ++--
 lib/internal/modules/esm/hooks.js  | 2 +-
 lib/internal/test_runner/runner.js | 2 ++
 lib/stream.js                      | 4 ++--
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js
index 7f0fe5534ee843..cf3c39e8d9da5a 100644
--- a/lib/internal/crypto/hkdf.js
+++ b/lib/internal/crypto/hkdf.js
@@ -57,7 +57,7 @@ const validateParameters = hideStackFrames((hash, key, salt, info, length) => {
   validateInteger(length, 'length', 0, kMaxLength);
 
   if (info.byteLength > 1024) {
-    throw ERR_OUT_OF_RANGE(
+    throw new ERR_OUT_OF_RANGE(
       'info',
       'must not contain more than 1024 bytes',
       info.byteLength);
diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js
index f75d0fba917241..5f65007a3b540e 100644
--- a/lib/internal/fs/streams.js
+++ b/lib/internal/fs/streams.js
@@ -141,8 +141,8 @@ function importFd(stream, options) {
     return options.fd.fd;
   }
 
-  throw ERR_INVALID_ARG_TYPE('options.fd',
-                             ['number', 'FileHandle'], options.fd);
+  throw new ERR_INVALID_ARG_TYPE('options.fd',
+                                 ['number', 'FileHandle'], options.fd);
 }
 
 function ReadStream(path, options) {
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
index 83e5038903df83..3ee2ac7798e4eb 100644
--- a/lib/internal/modules/esm/hooks.js
+++ b/lib/internal/modules/esm/hooks.js
@@ -500,7 +500,7 @@ class Hooks {
       !isAnyArrayBuffer(source) &&
       !isArrayBufferView(source)
     ) {
-      throw ERR_INVALID_RETURN_PROPERTY_VALUE(
+      throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
         'a string, an ArrayBuffer, or a TypedArray',
         hookErrIdentifier,
         'source',
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 5108fc3283f63b..c2fbecdb83aa23 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -314,6 +314,8 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
     if (code !== 0 || signal !== null) {
       if (!err) {
         const failureType = subtest.failedSubtests ? kSubtestsFailed : kTestCodeFailure;
+        // TODO(BridgeAR): The stack should not be created in the first place
+        // and the properties should be set inside of the function.
         err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
           __proto__: null,
           exitCode: code,
diff --git a/lib/stream.js b/lib/stream.js
index 4c105067bae1ca..49b698d05ebbd7 100644
--- a/lib/stream.js
+++ b/lib/stream.js
@@ -59,7 +59,7 @@ for (const key of ObjectKeys(streamReturningOperators)) {
   const op = streamReturningOperators[key];
   function fn(...args) {
     if (new.target) {
-      throw ERR_ILLEGAL_CONSTRUCTOR();
+      throw new ERR_ILLEGAL_CONSTRUCTOR();
     }
     return Stream.Readable.from(ReflectApply(op, this, args));
   }
@@ -77,7 +77,7 @@ for (const key of ObjectKeys(promiseReturningOperators)) {
   const op = promiseReturningOperators[key];
   function fn(...args) {
     if (new.target) {
-      throw ERR_ILLEGAL_CONSTRUCTOR();
+      throw new ERR_ILLEGAL_CONSTRUCTOR();
     }
     return ReflectApply(op, this, args);
   }

From 08e1c89b4efcdd065b44b64231eb8417b355e9db Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 14 Feb 2023 03:58:23 +0100
Subject: [PATCH 11/15] errors: significantly improve NodeError creation
 performance

The improvement comes from the following changes:

1. Less properties are defined on the error instances, especially
   no Symbol.
2. The check if something is a NodeError or not is not needed
   anymore.
3. The setup of NodeError now makes sure that all heavy lifting is
   done up front instead of doing that during instance creation.

To align with the WPT tests, the NodeErrors must have their
constructor set to their base class. This was not the case for
SystemErrors and is now aligned.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/assert/assertion_error.js        |   8 +-
 lib/internal/console/constructor.js           |   5 +-
 lib/internal/errors.js                        | 137 +++++++++---------
 lib/internal/http2/util.js                    |  13 +-
 .../source_map/prepare_stack_trace.js         |  39 +++--
 lib/internal/test_runner/utils.js             |   3 +-
 test/message/internal_assert.out              |   1 -
 test/message/internal_assert_fail.out         |   1 -
 test/message/util_inspect_error.js            |   8 +
 test/message/util_inspect_error.out           |   8 +
 test/parallel/test-repl-top-level-await.js    |   9 +-
 11 files changed, 124 insertions(+), 108 deletions(-)

diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
index 657ebdebf03cb1..9eb62afc9af369 100644
--- a/lib/internal/assert/assertion_error.js
+++ b/lib/internal/assert/assertion_error.js
@@ -424,13 +424,15 @@ class AssertionError extends Error {
     setStackTraceLimit(stackTraceLimit);
 
     this.generatedMessage = !message;
+
     ObjectDefineProperty(this, 'name', {
       __proto__: null,
-      value: 'AssertionError [ERR_ASSERTION]',
+      value: 'AssertionError',
       enumerable: false,
       writable: true,
       configurable: true
     });
+
     this.code = 'ERR_ASSERTION';
     if (details) {
       this.actual = undefined;
@@ -449,10 +451,6 @@ class AssertionError extends Error {
       this.operator = operator;
     }
     ErrorCaptureStackTrace(this, stackStartFn || stackStartFunction);
-    // Create error message including the error code in the name.
-    this.stack; // eslint-disable-line no-unused-expressions
-    // Reset the name.
-    this.name = 'AssertionError';
   }
 
   toString() {
diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js
index 1f0cc2d0a66ed9..ae9fcc942dc971 100644
--- a/lib/internal/console/constructor.js
+++ b/lib/internal/console/constructor.js
@@ -421,7 +421,10 @@ const consoleMethods = {
   trace: function trace(...args) {
     const err = {
       name: 'Trace',
-      message: this[kFormatForStderr](args)
+      message: this[kFormatForStderr](args),
+      toString() {
+        return `${this.name}: ${this.message}`;
+      }
     };
     ErrorCaptureStackTrace(err, trace);
     this.error(err.stack);
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 5948439abd36e4..fc6343822a1b6a 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -51,8 +51,6 @@ const {
   URIError,
 } = primordials;
 
-const kIsNodeError = Symbol('kIsNodeError');
-
 const isWindows = process.platform === 'win32';
 
 const messages = new SafeMap();
@@ -78,6 +76,7 @@ const MainContextError = Error;
 const overrideStackTrace = new SafeWeakMap();
 const kNoOverride = Symbol('kNoOverride');
 const nodeInternalPrefix = '__node_internal_';
+
 const prepareStackTrace = (globalThis, error, trace) => {
   // API for node internals to override error stack formatting
   // without interfering with userland code.
@@ -112,8 +111,13 @@ const prepareStackTrace = (globalThis, error, trace) => {
   //     at function (file)
   //     at file
   let errorString;
-  if (kIsNodeError in error) {
-    errorString = `${error.name} [${error.code}]: ${error.message}`;
+  // Do not use primordials here: we intercept user code here.
+  const { toString } = error;
+  if (typeof toString === 'function') {
+    errorString = FunctionPrototypeCall(toString, error);
+    if (typeof errorString !== 'string' || StringPrototypeStartsWith(errorString, '[object ')) {
+      errorString = ErrorPrototypeToString(error);
+    }
   } else {
     errorString = ErrorPrototypeToString(error);
   }
@@ -221,8 +225,9 @@ function inspectWithNoCustomRetry(obj, options) {
 // and may have .path and .dest.
 class SystemError extends Error {
   constructor(key, context) {
-    super();
-    const prefix = getMessage(key, [], this);
+    const msg = messages.get(key);
+
+    const prefix = getMessage(key, msg, []);
     let message = `${prefix}: ${context.syscall} returned ` +
                   `${context.code} (${context.message})`;
 
@@ -231,16 +236,11 @@ class SystemError extends Error {
     if (context.dest !== undefined)
       message += ` => ${context.dest}`;
 
+    super(message);
+
     this.code = key;
 
     ObjectDefineProperties(this, {
-      [kIsNodeError]: {
-        __proto__: null,
-        value: true,
-        enumerable: false,
-        writable: false,
-        configurable: true,
-      },
       name: {
         __proto__: null,
         value: 'SystemError',
@@ -248,13 +248,6 @@ class SystemError extends Error {
         writable: true,
         configurable: true,
       },
-      message: {
-        __proto__: null,
-        value: message,
-        enumerable: false,
-        writable: true,
-        configurable: true,
-      },
       info: {
         __proto__: null,
         value: context,
@@ -338,45 +331,53 @@ class SystemError extends Error {
 }
 
 function makeSystemErrorWithCode(key) {
-  return class NodeError extends SystemError {
+  const clazz = class NodeError extends SystemError {
     constructor(ctx) {
       super(key, ctx);
     }
   };
+  // The constructor must be the Base class to align with the WPT tests.
+  SystemError.prototype.constructor = Error;
+  return clazz;
 }
 
-function makeNodeErrorWithCode(Base, key) {
-  return function NodeError(...args) {
-    const error = new Base();
-    const message = getMessage(key, args, error);
-    ObjectDefineProperties(error, {
-      [kIsNodeError]: {
-        __proto__: null,
-        value: true,
-        enumerable: false,
-        writable: false,
-        configurable: true,
-      },
-      message: {
-        __proto__: null,
-        value: message,
-        enumerable: false,
-        writable: true,
-        configurable: true,
-      },
-      toString: {
-        __proto__: null,
-        value() {
-          return `${this.name} [${key}]: ${this.message}`;
-        },
-        enumerable: false,
-        writable: true,
-        configurable: true,
-      },
-    });
-    error.code = key;
-    return error;
-  };
+function makeNodeErrorWithCode(Base, key, val) {
+  let clazz;
+  if (typeof val === 'string') {
+    clazz = class NodeError extends Base {
+      constructor(...args) {
+        const message = getMessage(key, val, args);
+        super(message);
+        this.code = key;
+      }
+
+      toString() {
+        return `${this.name} [${key}]: ${this.message}`;
+      }
+    };
+  } else {
+    clazz = class NodeError extends Base {
+      constructor(...args) {
+        super();
+        const message = getFnMessage(key, val, args, this);
+        ObjectDefineProperty(this, 'message', {
+          __proto__: null,
+          value: message,
+          enumerable: false,
+          writable: true,
+          configurable: true,
+        });
+        this.code = key;
+      }
+
+      toString() {
+        return `${this.name} [${key}]: ${this.message}`;
+      }
+    };
+  }
+  // The constructor must be the Base class to align with the WPT tests.
+  clazz.prototype.constructor = Base;
+  return clazz;
 }
 
 /**
@@ -410,32 +411,34 @@ function E(sym, val, def, ...otherClasses) {
   if (def === SystemError) {
     def = makeSystemErrorWithCode(sym);
   } else {
-    def = makeNodeErrorWithCode(def, sym);
+    def = makeNodeErrorWithCode(def, sym, val);
   }
 
   if (otherClasses.length !== 0) {
     otherClasses.forEach((clazz) => {
-      def[clazz.name] = makeNodeErrorWithCode(clazz, sym);
+      def[clazz.name] = makeNodeErrorWithCode(clazz, sym, val);
     });
   }
   codes[sym] = def;
 }
 
-function getMessage(key, args, self) {
-  const msg = messages.get(key);
-
+function getFnMessage(key, fn, args, self) {
   assert ??= require('internal/assert');
 
   const expectedLength = messageArguments.get(key);
 
-  if (typeof msg === 'function') {
-    assert(
-      expectedLength <= args.length,
-      `Code: ${key}; The provided arguments length (${args.length}) does not ` +
-        `match the required ones (${msg.length}).`,
-    );
-    return ReflectApply(msg, self, args);
-  }
+  assert(
+    expectedLength <= args.length,
+    `Code: ${key}; The provided arguments length (${args.length}) does not ` +
+      `match the required ones (${expectedLength}).`,
+  );
+  return ReflectApply(fn, self, args);
+}
+
+function getMessage(key, msg, args) {
+  assert ??= require('internal/assert');
+
+  const expectedLength = messageArguments.get(key);
 
   assert(
     expectedLength === args.length,
@@ -859,6 +862,7 @@ module.exports = {
   fatalExceptionStackEnhancers,
   formatList,
   genericNodeError,
+  messages,
   getMessage,
   hideInternalStackFrames,
   hideStackFrames,
@@ -866,7 +870,6 @@ module.exports = {
   setStackTraceLimit,
   isStackOverflowError,
   kEnhanceStackBeforeInspector,
-  kIsNodeError,
   kNoOverride,
   maybeOverridePrepareStackTrace,
   overrideStackTrace,
diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
index b022481bc8bc44..6c16a6e1125d40 100644
--- a/lib/internal/http2/util.js
+++ b/lib/internal/http2/util.js
@@ -8,7 +8,6 @@ const {
   Error,
   MathMax,
   Number,
-  ObjectDefineProperty,
   ObjectKeys,
   SafeSet,
   String,
@@ -28,9 +27,9 @@ const {
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_HTTP_TOKEN
   },
+  messages,
   getMessage,
   hideStackFrames,
-  kIsNodeError,
 } = require('internal/errors');
 
 const kSensitiveHeaders = Symbol('nodejs.http2.sensitiveHeaders');
@@ -546,18 +545,12 @@ function mapToHeaders(map,
 
 class NghttpError extends Error {
   constructor(integerCode, customErrorCode) {
+    const msg = messages.get(customErrorCode);
     super(customErrorCode ?
-      getMessage(customErrorCode, [], null) :
+      getMessage(customErrorCode, msg, []) :
       binding.nghttp2ErrorString(integerCode));
     this.code = customErrorCode || 'ERR_HTTP2_ERROR';
     this.errno = integerCode;
-    ObjectDefineProperty(this, kIsNodeError, {
-      __proto__: null,
-      value: true,
-      enumerable: false,
-      writable: false,
-      configurable: true,
-    });
   }
 
   toString() {
diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js
index 860564f4a34ab5..e865d38341c3c3 100644
--- a/lib/internal/source_map/prepare_stack_trace.js
+++ b/lib/internal/source_map/prepare_stack_trace.js
@@ -1,10 +1,10 @@
 'use strict';
 
 const {
+  ArrayPrototypeForEach,
   ArrayPrototypeIndexOf,
-  ArrayPrototypeJoin,
-  ArrayPrototypeMap,
   ErrorPrototypeToString,
+  ObjectPrototype,
   RegExpPrototypeSymbolSplit,
   StringPrototypeRepeat,
   StringPrototypeSlice,
@@ -22,21 +22,22 @@ const {
   kNoOverride,
   overrideStackTrace,
   maybeOverridePrepareStackTrace,
-  kIsNodeError,
 } = require('internal/errors');
 const { fileURLToPath } = require('internal/url');
 const { setGetSourceMapErrorSource } = internalBinding('errors');
 
+const MainContextObjectToString = ObjectPrototype.toString;
+
 // Create a prettified stacktrace, inserting context from source maps
 // if possible.
 const prepareStackTrace = (globalThis, error, trace) => {
   // API for node internals to override error stack formatting
   // without interfering with userland code.
   // TODO(bcoe): add support for source-maps to repl.
-  if (overrideStackTrace.has(error)) {
-    const f = overrideStackTrace.get(error);
+  const fn = overrideStackTrace.get(error);
+  if (fn !== undefined) {
     overrideStackTrace.delete(error);
-    return f(error, trace);
+    return fn(error, trace);
   }
 
   const globalOverride =
@@ -44,8 +45,13 @@ const prepareStackTrace = (globalThis, error, trace) => {
   if (globalOverride !== kNoOverride) return globalOverride;
 
   let errorString;
-  if (kIsNodeError in error) {
-    errorString = `${error.name} [${error.code}]: ${error.message}`;
+  // Do not use primordials here: we intercept user code here.
+  if (typeof error.toString === 'function' &&
+      error.toString !== MainContextObjectToString) {
+    errorString = error.toString();
+    if (errorString === '[Object object]') {
+      errorString = ErrorPrototypeToString(error);
+    }
   } else {
     errorString = ErrorPrototypeToString(error);
   }
@@ -56,15 +62,15 @@ const prepareStackTrace = (globalThis, error, trace) => {
 
   let lastSourceMap;
   let lastFileName;
-  const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => {
+  let preparedTrace = '';
+
+  ArrayPrototypeForEach(trace, (t, i) => {
     const str = i !== 0 ? '\n    at ' : '';
     try {
       // A stack trace will often have several call sites in a row within the
       // same file, cache the source map and file content accordingly:
-      let fileName = t.getFileName();
-      if (fileName === undefined) {
-        fileName = t.getEvalOrigin();
-      }
+      const fileName = t.getFileName() ?? t.getEvalOrigin();
+
       const sm = fileName === lastFileName ?
         lastSourceMap :
         findSourceMap(fileName);
@@ -96,16 +102,17 @@ const prepareStackTrace = (globalThis, error, trace) => {
             StringPrototypeStartsWith(originalSource, 'file://') ?
               fileURLToPath(originalSource) : originalSource;
           // Replace the transpiled call site with the original:
-          return `${str}${prefix}${hasName ? ' (' : ''}` +
+          preparedTrace += `${str}${prefix}${hasName ? ' (' : ''}` +
             `${originalSourceNoScheme}:${originalLine + 1}:` +
             `${originalColumn + 1}${hasName ? ')' : ''}`;
+          return;
         }
       }
     } catch (err) {
       debug(err);
     }
-    return `${str}${t}`;
-  }), '');
+    preparedTrace += `${str}${t}`;
+  });
   return `${errorString}\n    at ${preparedTrace}`;
 };
 
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 72270e464248a5..d34976b930329e 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -20,7 +20,6 @@ const {
     ERR_INVALID_ARG_VALUE,
     ERR_TEST_FAILURE,
   },
-  kIsNodeError,
 } = require('internal/errors');
 const { compose } = require('stream');
 
@@ -68,7 +67,7 @@ function createDeferredCallback() {
 }
 
 function isTestFailureError(err) {
-  return err?.code === 'ERR_TEST_FAILURE' && kIsNodeError in err;
+  return err instanceof ERR_TEST_FAILURE;
 }
 
 function convertStringToRegExp(str, name) {
diff --git a/test/message/internal_assert.out b/test/message/internal_assert.out
index bd25c879478083..197b863bf6ae69 100644
--- a/test/message/internal_assert.out
+++ b/test/message/internal_assert.out
@@ -5,7 +5,6 @@ node:internal/assert:*
 Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
 Please open an issue with this stack trace at https://github.com/nodejs/node/issues
 
-    at new NodeError (node:internal/errors:*:*)
     at assert (node:internal/assert:*:*)
     at * (*test*message*internal_assert.js:7:1)
     at *
diff --git a/test/message/internal_assert_fail.out b/test/message/internal_assert_fail.out
index 408d6d3364470d..e6895691cda9c1 100644
--- a/test/message/internal_assert_fail.out
+++ b/test/message/internal_assert_fail.out
@@ -6,7 +6,6 @@ Error [ERR_INTERNAL_ASSERTION]: Unreachable!
 This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
 Please open an issue with this stack trace at https://github.com/nodejs/node/issues
 
-    at new NodeError (node:internal/errors:*:*)
     at Function.fail (node:internal/assert:*:*)
     at * (*test*message*internal_assert_fail.js:7:8)
     at *
diff --git a/test/message/util_inspect_error.js b/test/message/util_inspect_error.js
index 20affd6c711fd8..164ed10d3996e6 100644
--- a/test/message/util_inspect_error.js
+++ b/test/message/util_inspect_error.js
@@ -10,3 +10,11 @@ console.log(util.inspect({ err, nested: { err } }, { compact: false }));
 
 err.foo = 'bar';
 console.log(util.inspect(err, { compact: true, breakLength: 5 }));
+
+class Foo extends Error {
+  toString() {
+    return 1;
+  }
+}
+
+console.log(new Foo());
diff --git a/test/message/util_inspect_error.out b/test/message/util_inspect_error.out
index 644ccd58831ef7..dc320fe94e6b4e 100644
--- a/test/message/util_inspect_error.out
+++ b/test/message/util_inspect_error.out
@@ -51,3 +51,11 @@ bar
     at *
     at *
   foo: 'bar' }
+Foo [Error]
+    at *util_inspect_error.js*
+    at *
+    at *
+    at *
+    at *
+    at *
+    at *
diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js
index 1abcca75f1e2a0..44b5d25d807010 100644
--- a/test/parallel/test-repl-top-level-await.js
+++ b/test/parallel/test-repl-top-level-await.js
@@ -204,13 +204,12 @@ async function ctrlCTest() {
     'await new Promise(() => {})',
     { ctrl: true, name: 'c' },
   ]);
-  assert.deepStrictEqual(output.slice(0, 3), [
+  assert.deepStrictEqual(output, [
     'await new Promise(() => {})\r',
     'Uncaught:',
-    'Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
-      'Script execution was interrupted by `SIGINT`',
-  ]);
-  assert.deepStrictEqual(output.slice(-2), [
+    '[Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
+      'Script execution was interrupted by `SIGINT`] {',
+    "  code: 'ERR_SCRIPT_EXECUTION_INTERRUPTED'",
     '}',
     PROMPT,
   ]);

From b3aa706ef651fb29ae68d551155571948a6b9ed4 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 21 Feb 2023 10:38:18 +0100
Subject: [PATCH 12/15] test_runner: set the properties inside of the error
 method

This improves the error recreation performance due to limiting the
stack trace that is generated and it makes sure NodeErrors only set
properties inside of the actual error method.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js                 |  5 ++++-
 lib/internal/test_runner/runner.js     | 12 +++++-------
 lib/internal/test_runner/yaml_to_js.js |  4 ++++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index fc6343822a1b6a..4043f7393edb3e 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -1530,7 +1530,7 @@ E('ERR_TAP_VALIDATION_ERROR', function(errorMsg) {
   hideInternalStackFrames(this);
   return errorMsg;
 }, Error);
-E('ERR_TEST_FAILURE', function(error, failureType) {
+E('ERR_TEST_FAILURE', function(error, failureType, extraProperties = undefined) {
   hideInternalStackFrames(this);
   assert(typeof failureType === 'string' || typeof failureType === 'symbol',
          "The 'failureType' argument must be of type string or symbol.");
@@ -1543,6 +1543,9 @@ E('ERR_TEST_FAILURE', function(error, failureType) {
 
   this.failureType = failureType;
   this.cause = error;
+  if (extraProperties !== undefined) {
+    ObjectAssign(this, extraProperties);
+  }
   return msg;
 }, Error);
 E('ERR_TLS_CERT_ALTNAME_FORMAT', 'Invalid subject alternative name string',
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index c2fbecdb83aa23..78c50dd4b22343 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -12,7 +12,6 @@ const {
   ArrayPrototypeSplice,
   FunctionPrototypeCall,
   Number,
-  ObjectAssign,
   ObjectKeys,
   PromisePrototypeThen,
   SafePromiseAll,
@@ -37,6 +36,7 @@ const {
   codes: {
     ERR_TEST_FAILURE,
   },
+  setStackTraceLimit,
 } = require('internal/errors');
 const { validateArray, validateBoolean, validateFunction } = require('internal/validators');
 const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
@@ -314,16 +314,14 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
     if (code !== 0 || signal !== null) {
       if (!err) {
         const failureType = subtest.failedSubtests ? kSubtestsFailed : kTestCodeFailure;
-        // TODO(BridgeAR): The stack should not be created in the first place
-        // and the properties should be set inside of the function.
-        err = ObjectAssign(new ERR_TEST_FAILURE('test failed', failureType), {
+        const { stackTraceLimit } = Error;
+        setStackTraceLimit(0);
+        err = new ERR_TEST_FAILURE('test failed', failureType, {
           __proto__: null,
           exitCode: code,
           signal: signal,
-          // The stack will not be useful since the failures came from tests
-          // in a child process.
-          stack: undefined,
         });
+        setStackTraceLimit(stackTraceLimit);
       }
 
       throw err;
diff --git a/lib/internal/test_runner/yaml_to_js.js b/lib/internal/test_runner/yaml_to_js.js
index 6eb193f4afd36e..21d0d62552e421 100644
--- a/lib/internal/test_runner/yaml_to_js.js
+++ b/lib/internal/test_runner/yaml_to_js.js
@@ -3,6 +3,7 @@ const {
   codes: {
     ERR_TEST_FAILURE,
   },
+  setStackTraceLimit,
 } = require('internal/errors');
 const AssertionError = require('internal/assert/assertion_error');
 const {
@@ -32,6 +33,8 @@ function reConstructError(parsedYaml) {
   const stack = parsedYaml.stack ? kStackDelimiter + ArrayPrototypeJoin(parsedYaml.stack, `\n${kStackDelimiter}`) : '';
   let error, cause;
 
+  const { stackTraceLimit } = Error;
+  setStackTraceLimit(0);
   if (isAssertionError) {
     cause = new AssertionError({
       message: parsedYaml.error,
@@ -54,6 +57,7 @@ function reConstructError(parsedYaml) {
     error = new ERR_TEST_FAILURE(cause, parsedYaml.failureType);
     error.stack = stack;
   }
+  setStackTraceLimit(stackTraceLimit);
 
   parsedYaml.error = error ?? cause;
   delete parsedYaml.stack;

From 72f87e98d3ca33e779d0fa117665a706ddd9d984 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 21 Feb 2023 15:03:01 +0100
Subject: [PATCH 13/15] errors,repl: align prepareStackTrace behavior

This fixes the issue that multiple prepareStackTrace functions
existed that had to duplicate code in multiple spots. Instead, the
diverging part is now replaced during runtime. This reduces the
code overhead and makes sure there is no duplication.

It also fixes the issue that `overrideStackTrace` usage would not
adhere to the `hideStackFrame` calls. Frames are now removed before
passing them to the override method.

A second fix is for the repl: the stack frames passed to a user
defined Error.prepareStackTrace() method are now in the correct
order.
As drive-by it also improves the performance for repl errors
marginally.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js                        | 70 +++++++++----------
 .../source_map/prepare_stack_trace.js         | 44 +-----------
 lib/internal/source_map/source_map_cache.js   | 17 +++--
 lib/repl.js                                   | 26 +++----
 .../parallel/test-repl-pretty-custom-stack.js |  4 +-
 5 files changed, 64 insertions(+), 97 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 4043f7393edb3e..0d2cc7d8da6dae 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -24,6 +24,7 @@ const {
   ArrayPrototypeSplice,
   Error,
   ErrorPrototypeToString,
+  FunctionPrototypeCall,
   JSONStringify,
   MapPrototypeGet,
   MathMax,
@@ -74,18 +75,19 @@ const kTypes = [
 
 const MainContextError = Error;
 const overrideStackTrace = new SafeWeakMap();
-const kNoOverride = Symbol('kNoOverride');
 const nodeInternalPrefix = '__node_internal_';
 
-const prepareStackTrace = (globalThis, error, trace) => {
-  // API for node internals to override error stack formatting
-  // without interfering with userland code.
-  const fn = overrideStackTrace.get(error);
-  if (fn !== undefined) {
-    overrideStackTrace.delete(error);
-    return fn(error, trace);
-  }
+const defaultFormatStackTrace = (errorString, trace) => {
+  return `${errorString}\n    at ${ArrayPrototypeJoin(trace, '\n    at ')}`;
+};
+let formatStackTrace = defaultFormatStackTrace;
 
+function setFormatStackTrace(fn = defaultFormatStackTrace) {
+  formatStackTrace = fn;
+}
+
+const prepareStackTrace = (globalThis, error, trace) => {
+  // Remove stack frames that should be hidden.
   const firstFrame = trace[0]?.getFunctionName();
   if (firstFrame && StringPrototypeStartsWith(firstFrame, nodeInternalPrefix)) {
     let i = trace.length - 1;
@@ -101,9 +103,27 @@ const prepareStackTrace = (globalThis, error, trace) => {
     }
   }
 
-  const globalOverride =
-    maybeOverridePrepareStackTrace(globalThis, error, trace);
-  if (globalOverride !== kNoOverride) return globalOverride;
+  // API for node internals to override error stack formatting
+  // without interfering with userland code.
+  const fn = overrideStackTrace.get(error);
+  if (fn !== undefined) {
+    overrideStackTrace.delete(error);
+    return fn(error, trace);
+  }
+
+  // Polyfill of V8's Error.prepareStackTrace API.
+  // https://crbug.com/v8/7848
+  // `globalThis` is the global that contains the constructor which
+  // created `error`.
+  if (typeof globalThis.Error?.prepareStackTrace === 'function') {
+    return globalThis.Error.prepareStackTrace(error, trace);
+  }
+  // We still have legacy usage that depends on the main context's `Error`
+  // being used, even when the error is from a different context.
+  // TODO(devsnek): evaluate if this can be eventually deprecated/removed.
+  if (typeof MainContextError.prepareStackTrace === 'function') {
+    return MainContextError.prepareStackTrace(error, trace);
+  }
 
   // Normal error formatting:
   //
@@ -124,25 +144,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
   if (trace.length === 0) {
     return errorString;
   }
-  return `${errorString}\n    at ${ArrayPrototypeJoin(trace, '\n    at ')}`;
-};
-
-const maybeOverridePrepareStackTrace = (globalThis, error, trace) => {
-  // Polyfill of V8's Error.prepareStackTrace API.
-  // https://crbug.com/v8/7848
-  // `globalThis` is the global that contains the constructor which
-  // created `error`.
-  if (typeof globalThis.Error?.prepareStackTrace === 'function') {
-    return globalThis.Error.prepareStackTrace(error, trace);
-  }
-  // We still have legacy usage that depends on the main context's `Error`
-  // being used, even when the error is from a different context.
-  // TODO(devsnek): evaluate if this can be eventually deprecated/removed.
-  if (typeof MainContextError.prepareStackTrace === 'function') {
-    return MainContextError.prepareStackTrace(error, trace);
-  }
-
-  return kNoOverride;
+  return formatStackTrace(errorString, trace);
 };
 
 const aggregateTwoErrors = hideStackFrames((innerError, outerError) => {
@@ -777,8 +779,7 @@ function hideInternalStackFrames(error) {
         result += `\n    at ${frame}`;
       }
     }
-    result = error + result;
-    return result;
+    return error + result;
   });
 }
 
@@ -870,8 +871,7 @@ module.exports = {
   setStackTraceLimit,
   isStackOverflowError,
   kEnhanceStackBeforeInspector,
-  kNoOverride,
-  maybeOverridePrepareStackTrace,
+  setFormatStackTrace,
   overrideStackTrace,
   prepareStackTrace,
   setArrowMessage,
diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js
index e865d38341c3c3..2445779dfffcb4 100644
--- a/lib/internal/source_map/prepare_stack_trace.js
+++ b/lib/internal/source_map/prepare_stack_trace.js
@@ -3,8 +3,6 @@
 const {
   ArrayPrototypeForEach,
   ArrayPrototypeIndexOf,
-  ErrorPrototypeToString,
-  ObjectPrototype,
   RegExpPrototypeSymbolSplit,
   StringPrototypeRepeat,
   StringPrototypeSlice,
@@ -18,48 +16,12 @@ let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
 const { getStringWidth } = require('internal/util/inspect');
 const { readFileSync } = require('fs');
 const { findSourceMap } = require('internal/source_map/source_map_cache');
-const {
-  kNoOverride,
-  overrideStackTrace,
-  maybeOverridePrepareStackTrace,
-} = require('internal/errors');
 const { fileURLToPath } = require('internal/url');
 const { setGetSourceMapErrorSource } = internalBinding('errors');
 
-const MainContextObjectToString = ObjectPrototype.toString;
-
 // Create a prettified stacktrace, inserting context from source maps
 // if possible.
-const prepareStackTrace = (globalThis, error, trace) => {
-  // API for node internals to override error stack formatting
-  // without interfering with userland code.
-  // TODO(bcoe): add support for source-maps to repl.
-  const fn = overrideStackTrace.get(error);
-  if (fn !== undefined) {
-    overrideStackTrace.delete(error);
-    return fn(error, trace);
-  }
-
-  const globalOverride =
-    maybeOverridePrepareStackTrace(globalThis, error, trace);
-  if (globalOverride !== kNoOverride) return globalOverride;
-
-  let errorString;
-  // Do not use primordials here: we intercept user code here.
-  if (typeof error.toString === 'function' &&
-      error.toString !== MainContextObjectToString) {
-    errorString = error.toString();
-    if (errorString === '[Object object]') {
-      errorString = ErrorPrototypeToString(error);
-    }
-  } else {
-    errorString = ErrorPrototypeToString(error);
-  }
-
-  if (trace.length === 0) {
-    return errorString;
-  }
-
+function formatStackTrace(errorString, trace) {
   let lastSourceMap;
   let lastFileName;
   let preparedTrace = '';
@@ -114,7 +76,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
     preparedTrace += `${str}${t}`;
   });
   return `${errorString}\n    at ${preparedTrace}`;
-};
+}
 
 // Transpilers may have removed the original symbol name used in the stack
 // trace, if possible restore it from the names field of the source map:
@@ -217,5 +179,5 @@ function getSourceMapErrorSource(fileName, lineNumber, columnNumber) {
 setGetSourceMapErrorSource(getSourceMapErrorSource);
 
 module.exports = {
-  prepareStackTrace,
+  formatStackTrace,
 };
diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js
index ee1f9c404e8b6f..8a512d7084fa12 100644
--- a/lib/internal/source_map/source_map_cache.js
+++ b/lib/internal/source_map/source_map_cache.js
@@ -56,18 +56,23 @@ function setSourceMapsEnabled(val) {
     setSourceMapsEnabled,
     setPrepareStackTraceCallback,
   } = internalBinding('errors');
+
   setSourceMapsEnabled(val);
+
+  const {
+    setFormatStackTrace,
+    prepareStackTrace,
+  } = require('internal/errors');
+
+  setPrepareStackTraceCallback(prepareStackTrace);
   if (val) {
     const {
-      prepareStackTrace,
+      formatStackTrace,
     } = require('internal/source_map/prepare_stack_trace');
-    setPrepareStackTraceCallback(prepareStackTrace);
+    setFormatStackTrace(formatStackTrace);
   } else if (sourceMapsEnabled !== undefined) {
     // Reset prepare stack trace callback only when disabling source maps.
-    const {
-      prepareStackTrace,
-    } = require('internal/errors');
-    setPrepareStackTraceCallback(prepareStackTrace);
+    setFormatStackTrace();
   }
 
   sourceMapsEnabled = val;
diff --git a/lib/repl.js b/lib/repl.js
index 3356b88e48b605..8e039746aafa2d 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -44,7 +44,6 @@
 
 const {
   ArrayPrototypeFilter,
-  ArrayPrototypeFindIndex,
   ArrayPrototypeForEach,
   ArrayPrototypeIncludes,
   ArrayPrototypeJoin,
@@ -52,12 +51,10 @@ const {
   ArrayPrototypePop,
   ArrayPrototypePush,
   ArrayPrototypePushApply,
-  ArrayPrototypeReverse,
   ArrayPrototypeShift,
   ArrayPrototypeSlice,
   ArrayPrototypeSome,
   ArrayPrototypeSort,
-  ArrayPrototypeSplice,
   ArrayPrototypeUnshift,
   Boolean,
   Error,
@@ -646,18 +643,17 @@ function REPLServer(prompt,
 
     if (typeof e === 'object' && e !== null) {
       overrideStackTrace.set(e, (error, stackFrames) => {
-        let frames;
+        let frames = stackFrames;
         if (typeof stackFrames === 'object') {
           // Search from the bottom of the call stack to
           // find the first frame with a null function name
-          const idx = ArrayPrototypeFindIndex(
-            ArrayPrototypeReverse(stackFrames),
-            (frame) => frame.getFunctionName() === null,
+          const idx = stackFrames.findLastIndex(
+            (frame, i) => frame.getFunctionName() === null,
           );
-          // If found, get rid of it and everything below it
-          frames = ArrayPrototypeSplice(stackFrames, idx + 1);
-        } else {
-          frames = stackFrames;
+          if (idx !== -1) {
+            // If found, get rid of it and everything above it
+            frames = ArrayPrototypeSlice(stackFrames, 0, idx);
+          }
         }
         // FIXME(devsnek): this is inconsistent with the checks
         // that the real prepareStackTrace dispatch uses in
@@ -665,8 +661,12 @@ function REPLServer(prompt,
         if (typeof Error.prepareStackTrace === 'function') {
           return Error.prepareStackTrace(error, frames);
         }
-        ArrayPrototypePush(frames, error);
-        return ArrayPrototypeJoin(ArrayPrototypeReverse(frames), '\n    at ');
+
+        if (frames.length === 0) {
+          return `${error}`;
+        }
+
+        return `${error}\n    at ${ArrayPrototypeJoin(frames, '\n    at ')}`;
       });
       decorateErrorStack(e);
 
diff --git a/test/parallel/test-repl-pretty-custom-stack.js b/test/parallel/test-repl-pretty-custom-stack.js
index a10cd032a688c4..59171e45005491 100644
--- a/test/parallel/test-repl-pretty-custom-stack.js
+++ b/test/parallel/test-repl-pretty-custom-stack.js
@@ -42,8 +42,8 @@ const origPrepareStackTrace = Error.prepareStackTrace;
 Error.prepareStackTrace = (err, stack) => {
   if (err instanceof SyntaxError)
     return err.toString();
-  stack.push(err);
-  return stack.reverse().join('--->\n');
+  stack.unshift(err);
+  return stack.join('--->\n');
 };
 
 process.on('uncaughtException', (e) => {

From ee8e40f848d80873315ab8fd4a4e8b7d3535b232 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 2 Mar 2023 01:18:56 +0100
Subject: [PATCH 14/15] errors: remove unused ERR_INVALID_RETURN_PROPERTY error

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/errors.md      | 14 +++++++-------
 lib/internal/errors.js |  4 ----
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/doc/api/errors.md b/doc/api/errors.md
index 9139194719ade5..a1f0a0a0a2da35 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -2040,13 +2040,6 @@ which is not supported.
 The input may not be used in the [`REPL`][]. The conditions under which this
 error is used are described in the [`REPL`][] documentation.
 
-<a id="ERR_INVALID_RETURN_PROPERTY"></a>
-
-### `ERR_INVALID_RETURN_PROPERTY`
-
-Thrown in case a function option does not provide a valid value for one of its
-returned object properties on execution.
-
 <a id="ERR_INVALID_RETURN_PROPERTY_VALUE"></a>
 
 ### `ERR_INVALID_RETURN_PROPERTY_VALUE`
@@ -3283,6 +3276,13 @@ removed: v15.0.0
 
 An invalid or unknown file encoding was passed.
 
+<a id="ERR_INVALID_RETURN_PROPERTY"></a>
+
+### `ERR_INVALID_RETURN_PROPERTY`
+
+Thrown in case a function option does not provide a valid value for one of its
+returned object properties on execution.
+
 <a id="ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST"></a>
 
 ### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 0d2cc7d8da6dae..e7399d2fb6154e 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -1281,10 +1281,6 @@ E('ERR_INVALID_PROTOCOL',
 E('ERR_INVALID_REPL_EVAL_CONFIG',
   'Cannot specify both "breakEvalOnSigint" and "eval" for REPL', TypeError);
 E('ERR_INVALID_REPL_INPUT', '%s', TypeError);
-E('ERR_INVALID_RETURN_PROPERTY', (input, name, prop, value) => {
-  return `Expected a valid ${input} to be returned for the "${prop}" from the` +
-         ` "${name}" function but got ${value}.`;
-}, TypeError);
 E('ERR_INVALID_RETURN_PROPERTY_VALUE', (input, name, prop, value) => {
   let type;
   if (value?.constructor?.name) {

From 40ab9c34ce8e5ddb483b9fd6773baad837368564 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 2 Mar 2023 01:20:01 +0100
Subject: [PATCH 15/15] errors: faster ERR_MODULE_NOT_FOUND error

The function calls are more expensive than using the error printf
format style.

Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
---
 lib/internal/errors.js                   | 4 +---
 lib/internal/modules/esm/fetch_module.js | 2 +-
 lib/internal/modules/esm/resolve.js      | 9 ++++++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index e7399d2fb6154e..901e88993f1eb5 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -1383,9 +1383,7 @@ E('ERR_MISSING_ARGS',
     return `${msg} must be specified`;
   }, TypeError);
 E('ERR_MISSING_OPTION', '%s is required', TypeError);
-E('ERR_MODULE_NOT_FOUND', (path, base, type = 'package') => {
-  return `Cannot find ${type} '${path}' imported from ${base}`;
-}, Error);
+E('ERR_MODULE_NOT_FOUND', "Cannot find %s '%s' imported from %s", Error);
 E('ERR_MULTIPLE_CALLBACK', 'Callback called multiple times', Error);
 E('ERR_NAPI_CONS_FUNCTION', 'Constructor must be a function', TypeError);
 E('ERR_NAPI_INVALID_DATAVIEW_ARGS',
diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js
index d79e5a5eb99e84..89d62016454dbd 100644
--- a/lib/internal/modules/esm/fetch_module.js
+++ b/lib/internal/modules/esm/fetch_module.js
@@ -144,7 +144,7 @@ function fetchWithRedirects(parsed) {
         return entry;
       }
       if (res.statusCode === 404) {
-        const err = new ERR_MODULE_NOT_FOUND(parsed.href, null);
+        const err = new ERR_MODULE_NOT_FOUND('package', parsed.href, null);
         err.message = `Cannot find module '${parsed.href}', HTTP 404`;
         throw err;
       }
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
index 194d2ba37bf3b2..bd79009cd0a449 100644
--- a/lib/internal/modules/esm/resolve.js
+++ b/lib/internal/modules/esm/resolve.js
@@ -200,7 +200,10 @@ function legacyMainResolve(packageJSONUrl, packageConfig, base) {
   }
   // Not found.
   throw new ERR_MODULE_NOT_FOUND(
-    fileURLToPath(new URL('.', packageJSONUrl)), fileURLToPath(base));
+    'package',
+    fileURLToPath(new URL('.', packageJSONUrl)),
+    fileURLToPath(base),
+  );
 }
 
 const encodedSepRegEx = /%2F|%5C/i;
@@ -229,7 +232,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
       process.send({ 'watch:require': [path || resolved.pathname] });
     }
     throw new ERR_MODULE_NOT_FOUND(
-      path || resolved.pathname, base && fileURLToPath(base), 'module');
+      'module', path || resolved.pathname, base && fileURLToPath(base));
   }
 
   if (!preserveSymlinks) {
@@ -791,7 +794,7 @@ function packageResolve(specifier, base, conditions) {
 
   // eslint can't handle the above code.
   // eslint-disable-next-line no-unreachable
-  throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base));
+  throw new ERR_MODULE_NOT_FOUND('package', packageName, fileURLToPath(base));
 }
 
 /**