From 941aedc6fc7b42accd93d43b07fd43da41181852 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 29 Sep 2023 16:50:50 +0200 Subject: [PATCH] errors: fix stacktrace of SystemError PR-URL: https://github.com/nodejs/node/pull/49956 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Stephen Belanger Reviewed-By: James M Snell --- benchmark/error/system-error-instantiation.js | 64 ++++ test/parallel/test-errors-systemerror.js | 285 +++++++++++++++++- 2 files changed, 344 insertions(+), 5 deletions(-) create mode 100644 benchmark/error/system-error-instantiation.js diff --git a/benchmark/error/system-error-instantiation.js b/benchmark/error/system-error-instantiation.js new file mode 100644 index 00000000000000..0ecdc36a357086 --- /dev/null +++ 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'); + } +} diff --git a/test/parallel/test-errors-systemerror.js b/test/parallel/test-errors-systemerror.js index 38afbd4aa7d164..629a3d1c0869f8 100644 --- 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)' + ); +}