errors: update error name

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 <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
nodejs · Mar 23, 2019 · 1ed3c54 · 1ed3c54
1 parent c757cb1
commit 1ed3c54
Show file tree

Hide file tree

Showing 71 changed files with 283 additions and 284 deletions.
diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js
@@ -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) {

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -18,6 +18,8 @@ const codes = {};
 const { kMaxLength } = internalBinding('buffer');
 const { defineProperty } = Object;
 
+let useOriginalName = false;
+
 // Lazily loaded
 let util;
 let assert;
@@ -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() {
@@ -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) {
@@ -151,8 +145,6 @@ function makeSystemErrorWithCode(key) {
   };
 }
 
-let useOriginalName = false;
-
 function makeNodeErrorWithCode(Base, key) {
   return class NodeError extends Base {
     constructor(...args) {
@@ -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() {
@@ -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) {

diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
@@ -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}`;
   }
 }
 

diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
@@ -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.
@@ -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.'
@@ -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()));

diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js
@@ -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))
   }
@@ -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     ^`
   });
 
@@ -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- }"
   });
@@ -953,7 +953,7 @@ assert.deepStrictEqual(obj1, obj2);
     () => assert.deepStrictEqual(a, b),
     {
       code: 'ERR_ASSERTION',
-      name: 'AssertionError [ERR_ASSERTION]',
+      name: 'AssertionError',
       message: /\.\.\./g
     }
   );
@@ -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' +
@@ -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  /
     }
   );

diff --git a/test/parallel/test-assert-fail-deprecation.js b/test/parallel/test-assert-fail-deprecation.js
@@ -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',
@@ -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',
@@ -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',

diff --git a/test/parallel/test-assert-fail.js b/test/parallel/test-assert-fail.js
@@ -8,7 +8,7 @@ assert.throws(
   () => { assert.fail(); },
   {
     code: 'ERR_ASSERTION',
-    name: 'AssertionError [ERR_ASSERTION]',
+    name: 'AssertionError',
     message: 'Failed',
     operator: 'fail',
     actual: undefined,
@@ -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,

diff --git a/test/parallel/test-assert-first-line.js b/test/parallel/test-assert-first-line.js
@@ -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"
   }
 );