Skip to content

Commit

Permalink
errors: update error name
Browse files Browse the repository at this point in the history
This updates all Node.js errors by removing the `code` being part
of the `name` property. Instead, the name is just changed once on
instantiation, the stack is accessed to create the stack as expected
and then the `name` property is set back to it's original form.

PR-URL: #26738
Fixes: #26669
Fixes: #20253
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
BridgeAR committed Mar 23, 2019
1 parent c757cb1 commit 1ed3c54
Show file tree
Hide file tree
Showing 71 changed files with 283 additions and 284 deletions.
15 changes: 14 additions & 1 deletion lib/internal/assert/assertion_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,25 @@ class AssertionError extends Error {
}

this.generatedMessage = !message;
this.name = 'AssertionError [ERR_ASSERTION]';
Object.defineProperty(this, 'name', {
value: 'AssertionError [ERR_ASSERTION]',
enumerable: false,
writable: true,
configurable: true
});
this.code = 'ERR_ASSERTION';
this.actual = actual;
this.expected = expected;
this.operator = operator;
Error.captureStackTrace(this, stackStartFn);
// Create error message including the error code in the name.
this.stack;
// Reset the name.
this.name = 'AssertionError';
}

toString() {
return `${this.name} [${this.code}]: ${this.message}`;
}

[inspect.custom](recurseTimes, ctx) {
Expand Down
65 changes: 34 additions & 31 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const codes = {};
const { kMaxLength } = internalBinding('buffer');
const { defineProperty } = Object;

let useOriginalName = false;

// Lazily loaded
let util;
let assert;
Expand Down Expand Up @@ -74,19 +76,7 @@ class SystemError extends Error {
value: key,
writable: true
});
}

get name() {
return `SystemError [${this[kCode]}]`;
}

set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
addCodeToName(this, 'SystemError', key);
}

get code() {
Expand Down Expand Up @@ -141,6 +131,10 @@ class SystemError extends Error {
this[kInfo].dest = val ?
lazyBuffer().from(val.toString()) : undefined;
}

toString() {
return `${this.name} [${this.code}]: ${this.message}`;
}
}

function makeSystemErrorWithCode(key) {
Expand All @@ -151,8 +145,6 @@ function makeSystemErrorWithCode(key) {
};
}

let useOriginalName = false;

function makeNodeErrorWithCode(Base, key) {
return class NodeError extends Base {
constructor(...args) {
Expand All @@ -164,22 +156,7 @@ function makeNodeErrorWithCode(Base, key) {
writable: true,
configurable: true
});
}

get name() {
if (useOriginalName) {
return super.name;
}
return `${super.name} [${key}]`;
}

set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
addCodeToName(this, super.name, key);
}

get code() {
Expand All @@ -194,9 +171,35 @@ function makeNodeErrorWithCode(Base, key) {
writable: true
});
}

toString() {
return `${this.name} [${key}]: ${this.message}`;
}
};
}

function addCodeToName(err, name, code) {
if (useOriginalName) {
return;
}
// Add the error code to the name to include it in the stack trace.
err.name = `${name} [${code}]`;
// Access the stack to generate the error message including the error code
// from the name.
err.stack;
// Reset the name to the actual name.
if (name === 'SystemError') {
defineProperty(err, 'name', {
value: name,
enumerable: false,
writable: true,
configurable: true
});
} else {
delete err.name;
}
}

// Utility function for registering the error codes. Only used here. Exported
// *only* to allow for testing.
function E(sym, val, def, ...otherClasses) {
Expand Down
6 changes: 6 additions & 0 deletions lib/internal/http2/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,12 @@ class NghttpError extends Error {
this.code = 'ERR_HTTP2_ERROR';
this.name = 'Error [ERR_HTTP2_ERROR]';
this.errno = ret;
this.stack;
delete this.name;
}

toString() {
return `${this.name} [${this.code}]: ${this.message}`;
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-assert-async.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const promises = [];
const rejectingFn = async () => assert.fail();
const errObj = {
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: 'Failed'
};
// `assert.rejects` accepts a function or a promise as first argument.
Expand All @@ -38,7 +38,7 @@ const promises = [];

promise = assert.rejects(() => {}, common.mustNotCall());
promises.push(assert.rejects(promise, {
name: 'TypeError [ERR_INVALID_RETURN_VALUE]',
name: 'TypeError',
code: 'ERR_INVALID_RETURN_VALUE',
message: 'Expected instance of Promise to be returned ' +
'from the "promiseFn" function but got type undefined.'
Expand Down Expand Up @@ -75,7 +75,7 @@ promises.push(assert.rejects(
message: 'Expected instance of Promise to be returned ' +
'from the "promiseFn" function but got instance of Map.',
code: 'ERR_INVALID_RETURN_VALUE',
name: 'TypeError [ERR_INVALID_RETURN_VALUE]'
name: 'TypeError'
}));
promises.push(assert.doesNotReject(async () => {}));
promises.push(assert.doesNotReject(Promise.resolve()));
Expand Down
24 changes: 12 additions & 12 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ assert.throws(
assert.throws(
() => assert.notDeepStrictEqual(new Date(2000, 3, 14), new Date(2000, 3, 14)),
{
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: 'Expected "actual" not to be strictly deep-equal to: ' +
util.inspect(new Date(2000, 3, 14))
}
Expand All @@ -790,35 +790,35 @@ assert.throws(
() => assert.deepStrictEqual(/ab/, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n+ /ab/\n- /a/`
});
assert.throws(
() => assert.deepStrictEqual(/a/g, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n+ /a/g\n- /a/`
});
assert.throws(
() => assert.deepStrictEqual(/a/i, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n+ /a/i\n- /a/`
});
assert.throws(
() => assert.deepStrictEqual(/a/m, /a/),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n+ /a/m\n- /a/`
});
assert.throws(
() => assert.deepStrictEqual(/a/igm, /a/im),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n+ /a/gim\n- /a/im\n ^`
});

Expand All @@ -844,22 +844,22 @@ assert.deepStrictEqual({ a: 4, b: '2' }, { a: 4, b: '2' });
assert.throws(() => assert.deepStrictEqual([4], ['4']),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n [\n+ 4\n- '4'\n ]`
});
assert.throws(
() => assert.deepStrictEqual({ a: 4 }, { a: 4, b: true }),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n ` +
'{\n a: 4,\n- b: true\n }'
});
assert.throws(
() => assert.deepStrictEqual(['a'], { 0: 'a' }),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n` +
"+ [\n+ 'a'\n+ ]\n- {\n- '0': 'a'\n- }"
});
Expand Down Expand Up @@ -953,7 +953,7 @@ assert.deepStrictEqual(obj1, obj2);
() => assert.deepStrictEqual(a, b),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: /\.\.\./g
}
);
Expand All @@ -977,7 +977,7 @@ assert.throws(
() => assert.deepStrictEqual([1, 2, 3], [1, 2]),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: `${defaultMsgStartFull}\n\n` +
' [\n' +
' 1,\n' +
Expand Down Expand Up @@ -1063,7 +1063,7 @@ assert.throws(
() => assert.deepStrictEqual(a, b),
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: /a: \[Getter: 5]\n- a: \[Getter: 6]\n /
}
);
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-assert-fail-deprecation.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ assert.throws(() => {
assert.fail('first', 'second');
}, {
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: '\'first\' != \'second\'',
operator: '!=',
actual: 'first',
Expand All @@ -28,7 +28,7 @@ assert.throws(() => {
assert.fail('ignored', 'ignored', 'another custom message');
}, {
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: 'another custom message',
operator: 'fail',
actual: 'ignored',
Expand All @@ -49,7 +49,7 @@ assert.throws(() => {
assert.fail('first', 'second', undefined, 'operator');
}, {
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: '\'first\' operator \'second\'',
operator: 'operator',
actual: 'first',
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-assert-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ assert.throws(
() => { assert.fail(); },
{
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: 'Failed',
operator: 'fail',
actual: undefined,
Expand All @@ -22,7 +22,7 @@ assert.throws(() => {
assert.fail('custom message');
}, {
code: 'ERR_ASSERTION',
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: 'custom message',
operator: 'fail',
actual: undefined,
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-assert-first-line.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ const { path } = require('../common/fixtures');
assert.throws(
() => require(path('assert-first-line')),
{
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: "The expression evaluated to a falsy value:\n\n ässört.ok('')\n"
}
);

assert.throws(
() => require(path('assert-long-line')),
{
name: 'AssertionError [ERR_ASSERTION]',
name: 'AssertionError',
message: "The expression evaluated to a falsy value:\n\n assert.ok('')\n"
}
);
Loading

0 comments on commit 1ed3c54

Please sign in to comment.