errors: fix stacktrace of SystemError

PR-URL: https://github.com/nodejs/node/pull/49956 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: James M Snell <jasnell@gmail.com>
2023-09-29 16:50:50 +02:00 · 2023-09-29 16:50:50 +02:00 · c74c514273
parent e266bb1722
commit c74c514273
2 changed files with 344 additions and 5 deletions
--- a/benchmark/error/system-error-instantiation.js
+++ b/benchmark/error/system-error-instantiation.js
@ -0,0 +1,64 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+const bench = common.createBenchmark(main, {
+  n: [1e6],
+  code: [
+    'built-in',
+    'ERR_FS_CP_DIR_TO_NON_DIR',
+  ],
+  stackTraceLimit: [0, 10],
+}, {
+  flags: ['--expose-internals'],
+});
+
+function getErrorFactory(code) {
+  const {
+    ERR_FS_CP_DIR_TO_NON_DIR,
+  } = require('internal/errors').codes;
+
+  switch (code) {
+    case 'built-in':
+      return (n) => new Error();
+    case 'ERR_FS_CP_DIR_TO_NON_DIR':
+      return (n) => new ERR_FS_CP_DIR_TO_NON_DIR({
+        message: 'cannot overwrite directory',
+        path: 'dest',
+        syscall: 'cp',
+        errno: 21,
+        code: 'EISDIR',
+      });
+    default:
+      throw new Error(`${code} not supported`);
+  }
+}
+
+function main({ n, code, stackTraceLimit }) {
+  const getError = getErrorFactory(code);
+
+  Error.stackTraceLimit = stackTraceLimit;
+
+  // Warm up.
+  const length = 1024;
+  const array = [];
+  for (let i = 0; i < length; ++i) {
+    array.push(getError(i));
+  }
+
+  bench.start();
+
+  for (let i = 0; i < n; ++i) {
+    const index = i % length;
+    array[index] = getError(index);
+  }
+
+  bench.end(n);
+
+  // Verify the entries to prevent dead code elimination from making
+  // the benchmark invalid.
+  for (let i = 0; i < length; ++i) {
+    assert.strictEqual(typeof array[i], 'object');
+  }
+}
--- a/test/parallel/test-errors-systemerror.js
+++ b/test/parallel/test-errors-systemerror.js
@ -3,7 +3,8 @@

 require('../common');
 const assert = require('assert');
-const { E, SystemError, codes } = require('internal/errors');
+const { E, SystemError, codes, kIsNodeError } = require('internal/errors');
+const { inspect } = require('internal/util/inspect');

 assert.throws(
  () => { new SystemError(); },
@ -31,7 +32,7 @@ const { ERR_TEST } = codes;
      code: 'ERR_TEST',
      name: 'SystemError',
      message: 'custom message: syscall_test returned ETEST (code message)' +
-               ' /str => /str2',
+        ' /str => /str2',
      info: ctx
    }
  );
@ -51,7 +52,7 @@ const { ERR_TEST } = codes;
      code: 'ERR_TEST',
      name: 'SystemError',
      message: 'custom message: syscall_test returned ETEST (code message)' +
-               ' /buf => /str2',
+        ' /buf => /str2',
      info: ctx
    }
  );
@ -71,7 +72,7 @@ const { ERR_TEST } = codes;
      code: 'ERR_TEST',
      name: 'SystemError',
      message: 'custom message: syscall_test returned ETEST (code message)' +
-               ' /buf => /buf2',
+        ' /buf => /buf2',
      info: ctx
    }
  );
@ -110,6 +111,11 @@ const { ERR_TEST } = codes;
  assert.strictEqual(err.syscall, 'test');
  assert.strictEqual(err.path, 'path');
  assert.strictEqual(err.dest, 'path');
+
+  assert.strictEqual(err.info.errno, 321);
+  assert.strictEqual(err.info.dest.toString(), 'path');
+  assert.strictEqual(err.info.path.toString(), 'path');
+  assert.strictEqual(err.info.syscall, 'test');
 }

 {
@ -128,8 +134,277 @@ const { ERR_TEST } = codes;
      code: 'ERR_TEST',
      name: 'Foobar',
      message: 'custom message: syscall_test returned ERR_TEST ' +
-               '(Error occurred)',
+        '(Error occurred)',
      info: ctx
    }
  );
 }
+
+{
+  const ctx = {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  };
+  const err = new ERR_TEST(ctx);
+
+  // is set to true
+  assert.strictEqual(err[kIsNodeError], true);
+
+  // is not writable
+  assert.throws(
+    () => { err[kIsNodeError] = false; },
+    {
+      name: 'TypeError',
+      message: /Symbol\(kIsNodeError\)/,
+    }
+  );
+
+  // is not enumerable
+  assert.strictEqual(Object.prototype.propertyIsEnumerable.call(err, kIsNodeError), false);
+
+  // is configurable
+  delete err[kIsNodeError];
+  assert.strictEqual(kIsNodeError in err, false);
+}
+
+{
+  const ctx = {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  };
+  const err = new ERR_TEST(ctx);
+
+  // is set to true
+  assert.strictEqual(err.name, 'SystemError');
+
+  // is writable
+  err.name = 'CustomError';
+  assert.strictEqual(err.name, 'CustomError');
+
+  // is not enumerable
+  assert.strictEqual(Object.prototype.propertyIsEnumerable.call(err, 'name'), false);
+
+  // is configurable
+  delete err.name;
+  assert.strictEqual(err.name, 'Error');
+}
+
+{
+  const ctx = {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  };
+  const err = new ERR_TEST(ctx);
+
+  // Is set with the correct message
+  assert.strictEqual(err.message, 'custom message: syscall_test returned ERR (something happened)');
+
+  // is writable
+  err.message = 'custom message';
+  assert.strictEqual(err.message, 'custom message');
+
+  // is not enumerable
+  assert.strictEqual(Object.prototype.propertyIsEnumerable.call(err, 'message'), false);
+
+  // is configurable
+  delete err.message;
+  assert.strictEqual(err.message, '');
+}
+
+{
+  const ctx = {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  };
+  const err = new ERR_TEST(ctx);
+
+  // Is set to the correct syscall
+  assert.strictEqual(err.syscall, 'syscall_test');
+
+  // is writable
+  err.syscall = 'custom syscall';
+  assert.strictEqual(err.syscall, 'custom syscall');
+
+  // is enumerable
+  assert(Object.prototype.propertyIsEnumerable.call(err, 'syscall'));
+
+  // is configurable
+  delete err.syscall;
+  assert.strictEqual('syscall' in err, false);
+}
+
+{
+  const ctx = {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  };
+  const err = new ERR_TEST(ctx);
+
+  // Is set to the correct errno
+  assert.strictEqual(err.errno, 123);
+
+  // is writable
+  err.errno = 'custom errno';
+  assert.strictEqual(err.errno, 'custom errno');
+
+  // is enumerable
+  assert(Object.prototype.propertyIsEnumerable.call(err, 'errno'));
+
+  // is configurable
+  delete err.errno;
+  assert.strictEqual('errno' in err, false);
+}
+
+{
+  const ctx = {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  };
+  const err = new ERR_TEST(ctx);
+
+  // Is set to the correct info
+  assert.strictEqual(Object.keys(err.info).length, 4);
+  assert.strictEqual(err.info.errno, 123);
+  assert.strictEqual(err.info.code, 'ERR');
+  assert.strictEqual(err.info.message, 'something happened');
+  assert.strictEqual(err.info.syscall, 'syscall_test');
+
+  // is not writable
+  assert.throws(
+    () => {
+      err.info = {
+        ...ctx,
+        errno: 124
+      };
+    },
+    {
+      name: 'TypeError',
+      message: /'info'/,
+    }
+  );
+
+  assert.strictEqual(Object.keys(err.info).length, 4);
+  assert.strictEqual(err.info.errno, 123);
+  assert.strictEqual(err.info.code, 'ERR');
+  assert.strictEqual(err.info.message, 'something happened');
+  assert.strictEqual(err.info.syscall, 'syscall_test');
+
+  // is enumerable
+  assert(Object.prototype.propertyIsEnumerable.call(err, 'info'));
+
+  // is configurable
+  delete err.info;
+
+  assert.strictEqual('info' in err, false);
+}
+
+{
+  // Make sure the stack trace does not contain SystemError
+  try {
+    throw new ERR_TEST({
+      code: 'ERR',
+      errno: 123,
+      message: 'something happened',
+      syscall: 'syscall_test',
+    });
+  } catch (e) {
+    assert.doesNotMatch(e.stack, /at new SystemError/);
+    assert.match(e.stack.split('\n')[1], /test-errors-systemerror\.js/);
+  }
+}
+
+{
+  // Make sure the stack trace has the correct number of frames
+  const limit = Error.stackTraceLimit;
+  Error.stackTraceLimit = 3;
+  function a() {
+    b();
+  }
+
+  function b() {
+    c();
+  }
+
+  function c() {
+    throw new ERR_TEST({
+      code: 'ERR',
+      errno: 123,
+      message: 'something happened',
+      syscall: 'syscall_test',
+    });
+  }
+  try {
+    a();
+  } catch (e) {
+    assert.doesNotMatch(e.stack, /at new SystemError/);
+    assert.match(e.stack.split('\n')[1], /test-errors-systemerror\.js/);
+    assert.match(e.stack.split('\n')[1], /at c \(/);
+    assert.match(e.stack.split('\n')[2], /test-errors-systemerror\.js/);
+    assert.match(e.stack.split('\n')[2], /at b \(/);
+    assert.match(e.stack.split('\n')[3], /test-errors-systemerror\.js/);
+    assert.match(e.stack.split('\n')[3], /at a \(/);
+    assert.strictEqual(e.stack.split('\n').length, 4);
+  } finally {
+    Error.stackTraceLimit = limit;
+  }
+}
+
+{
+  // Inspect should return the correct string
+  const err = new ERR_TEST({
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+    custom: 'custom'
+  });
+  let inspectedErr = inspect(err);
+
+  assert.ok(inspectedErr.includes(`info: {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+    custom: 'custom'
+  },`));
+
+  err.syscall = 'custom_syscall';
+
+  inspectedErr = inspect(err);
+
+  assert.ok(inspectedErr.includes(`info: {
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'custom_syscall',
+    custom: 'custom'
+  },`));
+}
+
+{
+  // toString should return the correct string
+
+  const err = new ERR_TEST({
+    code: 'ERR',
+    errno: 123,
+    message: 'something happened',
+    syscall: 'syscall_test',
+  });
+
+  assert.strictEqual(
+    err.toString(),
+    'SystemError [ERR_TEST]: custom message: syscall_test returned ERR (something happened)'
+  );
+}