errors: make sure all Node.js errors show their properties

lib/internal/errors.js

@@ -12,8 +12,6 @@
 
 const { Object, Math } = primordials;
 
-const kCode = Symbol('code');
-const kInfo = Symbol('info');
 const messages = new Map();
 const codes = {};
 
@@ -107,76 +105,86 @@ class SystemError extends Error {
       writable: true,
       configurable: true
     });
-    Object.defineProperty(this, kInfo, {
-      configurable: false,
-      enumerable: false,
-      value: context
-    });
-    Object.defineProperty(this, kCode, {
-      configurable: true,
-      enumerable: false,
-      value: key,
-      writable: true
-    });
     addCodeToName(this, 'SystemError', key);
-  }
 
-  get code() {
-    return this[kCode];
-  }
+    this.code = key;
 
-  set code(value) {
-    Object.defineProperty(this, 'code', {
-      configurable: true,
+    Object.defineProperty(this, 'info', {
+      value: context,
       enumerable: true,
-      value,
-      writable: true
+      configurable: true,
+      writable: false
     });
-  }
-
-  get info() {
-    return this[kInfo];
-  }
 
-  get errno() {
-    return this[kInfo].errno;
-  }
-
-  set errno(val) {
-    this[kInfo].errno = val;
-  }
-
-  get syscall() {
-    return this[kInfo].syscall;
-  }
-
-  set syscall(val) {
-    this[kInfo].syscall = val;
-  }
-
-  get path() {
-    return this[kInfo].path !== undefined ?
-      this[kInfo].path.toString() : undefined;
-  }
+    Object.defineProperty(this, 'errno', {
+      get() {
+        return context.errno;
+      },
+      set: (value) => {
+        context.errno = value;
+      },
+      enumerable: true,
+      configurable: true
+    });
 
-  set path(val) {
-    this[kInfo].path = val ?
-      lazyBuffer().from(val.toString()) : undefined;
-  }
+    Object.defineProperty(this, 'syscall', {
+      get() {
+        return context.syscall;
+      },
+      set: (value) => {
+        context.syscall = value;
+      },
+      enumerable: true,
+      configurable: true
+    });
 
-  get dest() {
-    return this[kInfo].path !== undefined ?
-      this[kInfo].dest.toString() : undefined;
-  }
+    if (context.path !== undefined) {
+      // TODO(BridgeAR): Investigate why and when the `.toString()` was
+      // introduced. The `path` and `dest` properties in the context seem to
+      // always be of type string. We should probably just remove the
+      // `.toString()` and `Buffer.from()` operations and set the value on the
+      // context as the user did.
+      Object.defineProperty(this, 'path', {
+        get() {
+          return context.path != null ?
+            context.path.toString() : context.path;
+        },
+        set: (value) => {
+          context.path = value ?
+            lazyBuffer().from(value.toString()) : undefined;
+        },
+        enumerable: true,
+        configurable: true
+      });
+    }
 
-  set dest(val) {
-    this[kInfo].dest = val ?
-      lazyBuffer().from(val.toString()) : undefined;
+    if (context.dest !== undefined) {
+      Object.defineProperty(this, 'dest', {
+        get() {
+          return context.dest != null ?
+            context.dest.toString() : context.dest;
+        },
+        set: (value) => {
+          context.dest = value ?
+            lazyBuffer().from(value.toString()) : undefined;
+        },
+        enumerable: true,
+        configurable: true
+      });
+    }
   }
 
   toString() {
     return `${this.name} [${this.code}]: ${this.message}`;
   }
+
+  [Symbol.for('nodejs.util.inspect.custom')](recurseTimes, ctx) {
+    return lazyInternalUtilInspect().inspect(this, {
+      ...ctx,
+      getters: true,
+      customInspect: false
+    });
+  }
 }
 
 function makeSystemErrorWithCode(key) {
@@ -207,19 +215,7 @@ function makeNodeErrorWithCode(Base, key) {
         configurable: true
       });
       addCodeToName(this, super.name, key);
-    }
-
-    get code() {
-      return key;
-    }
-
-    set code(value) {
-      Object.defineProperty(this, 'code', {
-        configurable: true,
-        enumerable: true,
-        value,
-        writable: true
-      });
+      this.code = key;
     }
 
     toString() {
@@ -380,7 +376,6 @@ function uvException(ctx) {
     err[prop] = ctx[prop];
   }
 
-  // TODO(BridgeAR): Show the `code` property as part of the stack.
   err.code = code;
   if (path) {
     err.path = path;

test/message/internal_assert.out

@@ -12,4 +12,6 @@ Please open an issue with this stack trace at https://github.com/nodejs/node/iss
     at *
     at *
     at *
-    at *
+    at * {
+  code: 'ERR_INTERNAL_ASSERTION'
+}

test/message/internal_assert_fail.out

@@ -13,4 +13,6 @@ Please open an issue with this stack trace at https://github.com/nodejs/node/iss
     at *
     at *
     at *
-    at *
+    at * {
+  code: 'ERR_INTERNAL_ASSERTION'
+}

test/parallel/test-dgram-socket-buffer-size.js

@@ -4,6 +4,7 @@
 const common = require('../common');
 const assert = require('assert');
 const dgram = require('dgram');
+const { inspect } = require('util');
 const { SystemError } = require('internal/errors');
 const { internalBinding } = require('internal/test/binding');
 const {
@@ -22,7 +23,7 @@ function getExpectedError(type) {
     'ENOTSOCK (socket operation on non-socket)' : 'EBADF (bad file descriptor)';
   const error = {
     code: 'ERR_SOCKET_BUFFER_SIZE',
-    type: SystemError,
+    name: 'SystemError',
     message: `Could not get or set buffer size: ${syscall} returned ${suffix}`,
     info: {
       code,
@@ -40,9 +41,25 @@ function getExpectedError(type) {
 
   const socket = dgram.createSocket('udp4');
 
-  common.expectsError(() => {
+  assert.throws(() => {
     socket.setSendBufferSize(8192);
-  }, errorObj);
+  }, (err) => {
+    assert.strictEqual(
+      inspect(err).replace(/^ +at .*\n/gm, ''),
+      `SystemError [ERR_SOCKET_BUFFER_SIZE]: ${errorObj.message}\n` +
+        "  code: 'ERR_SOCKET_BUFFER_SIZE',\n" +
+        '  info: {\n' +
+        `    errno: ${errorObj.info.errno},\n` +
+        `    code: '${errorObj.info.code}',\n` +
+        `    message: '${errorObj.info.message}',\n` +
+        `    syscall: '${errorObj.info.syscall}'\n` +
+        '  },\n' +
+        `  errno: [Getter/Setter: ${errorObj.info.errno}],\n` +
+        `  syscall: [Getter/Setter: '${errorObj.info.syscall}']\n` +
+        '}'
+    );
+    return true;
+  });
 
   common.expectsError(() => {
     socket.getSendBufferSize();

test/parallel/test-internal-errors.js

@@ -82,9 +82,9 @@ common.expectsError(() => {
 {
   const myError = new errors.codes.TEST_ERROR_1('foo');
   assert.strictEqual(myError.code, 'TEST_ERROR_1');
-  assert.strictEqual(myError.hasOwnProperty('code'), false);
+  assert.strictEqual(myError.hasOwnProperty('code'), true);
   assert.strictEqual(myError.hasOwnProperty('name'), false);
-  assert.deepStrictEqual(Object.keys(myError), []);
+  assert.deepStrictEqual(Object.keys(myError), ['code']);
   const initialName = myError.name;
   myError.code = 'FHQWHGADS';
   assert.strictEqual(myError.code, 'FHQWHGADS');
@@ -99,11 +99,11 @@ common.expectsError(() => {
 // browser. Note that `name` becomes enumerable after being assigned.
 {
   const myError = new errors.codes.TEST_ERROR_1('foo');
-  assert.deepStrictEqual(Object.keys(myError), []);
+  assert.deepStrictEqual(Object.keys(myError), ['code']);
   const initialToString = myError.toString();
 
   myError.name = 'Fhqwhgads';
-  assert.deepStrictEqual(Object.keys(myError), ['name']);
+  assert.deepStrictEqual(Object.keys(myError), ['code', 'name']);
   assert.notStrictEqual(myError.toString(), initialToString);
 }
 
@@ -114,7 +114,7 @@ common.expectsError(() => {
   let initialConsoleLog = '';
   hijackStdout((data) => { initialConsoleLog += data; });
   const myError = new errors.codes.TEST_ERROR_1('foo');
-  assert.deepStrictEqual(Object.keys(myError), []);
+  assert.deepStrictEqual(Object.keys(myError), ['code']);
   const initialToString = myError.toString();
   console.log(myError);
   assert.notStrictEqual(initialConsoleLog, '');
@@ -124,7 +124,7 @@ common.expectsError(() => {
   let subsequentConsoleLog = '';
   hijackStdout((data) => { subsequentConsoleLog += data; });
   myError.message = 'Fhqwhgads';
-  assert.deepStrictEqual(Object.keys(myError), []);
+  assert.deepStrictEqual(Object.keys(myError), ['code']);
   assert.notStrictEqual(myError.toString(), initialToString);
   console.log(myError);
   assert.strictEqual(subsequentConsoleLog, initialConsoleLog);

test/parallel/test-repl-top-level-await.js

@@ -161,8 +161,10 @@ async function ctrlCTest() {
   ]), [
     'await timeout(100000)\r',
     'Thrown:',
-    'Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
-      'Script execution was interrupted by `SIGINT`',
+    '[Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
+      'Script execution was interrupted by `SIGINT`] {',
+    "  code: 'ERR_SCRIPT_EXECUTION_INTERRUPTED'",
+    '}',
     PROMPT
   ]);
 }