errors: improve SystemError messages

This commit improves the SystemError messages by allowing user
to combine a custom message and the libuv error message. Also
since we now prefer use subclasses to construct the errors instead
of using `new errors.SystemError()` directly, this removes
the behavior of assigning a default error code `ERR_SYSTEM_ERROR`
to SystemError and requires the user to directly use the
`ERR_SYSTEM_ERROR` class to construct errors instead.

Also merges `makeNodeError` into the SystemError class definition
since that's the only place the function gets used and it seems
unnecessary to introduce another level of inheritance. SystemError
now directly inherits from Error instead of an intermmediate Error
class that inherits from Error.

Class hierarchy before this patch:

ERR_SOCKET_BUFFER_SIZE -> Error (use message formatted by SystemError)
ERR_SYSTEM_ERROR -> NodeError (temp) -> Error

After:

ERR_SOCKET_BUFFER_SIZE -> SystemError -> Error
ERR_TTY_INIT_FAILED -> SystemError -> Error
ERR_SYSTEM_ERROR -> SystemError -> Error

Error messages before this patch:

```
const dgram = require('dgram');
const socket = dgram.createSocket('udp4');
socket.setRecvBufferSize(8192);

// Error [ERR_SOCKET_BUFFER_SIZE]: Could not get or set buffer
// size: Error [ERR_SYSTEM_ERROR]: bad file descriptor:
// EBADF [uv_recv_buffer_size]
//    at bufferSize (dgram.js:191:11)
//    at Socket.setRecvBufferSize (dgram.js:689:3)

const tty = require('tty');
new tty.WriteStream(1 << 30);
// Error [ERR_SYSTEM_ERROR]: invalid argument: EINVAL [uv_tty_init]
//     at new WriteStream (tty.js:84:11)
```

After:

```
const dgram = require('dgram');
const socket = dgram.createSocket('udp4');
socket.setRecvBufferSize(8192);

// SystemError [ERR_SOCKET_BUFFER_SIZE]: Could not get or set buffer
// size: uv_recv_buffer_size returned EBADF (bad file descriptor)
//     at bufferSize (dgram.js:191:11)
//     at Socket.setRecvBufferSize (dgram.js:689:3)

const tty = require('tty');
new tty.WriteStream(1 << 30);
// SystemError [ERR_TTY_INIT_FAILED]: TTY initialization failed:
// uv_tty_init returned EINVAL (invalid argument)
//     at new WriteStream (tty.js:84:11)
```

PR-URL: https://github.com/nodejs/node/pull/19514
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

3 files changed, 133 insertions, 139 deletions

diff --git a/test/parallel/test-dgram-socket-buffer-size.js b/test/parallel/test-dgram-socket-buffer-size.js
index 1fe72690d2..834ca30c57 100644
--- a/test/parallel/test-dgram-socket-buffer-size.js
+++ b/test/parallel/test-dgram-socket-buffer-size.js
@@ -1,33 +1,61 @@
 'use strict';
+// Flags: --expose-internals
 
 const common = require('../common');
 const assert = require('assert');
 const dgram = require('dgram');
+const { SystemError } = require('internal/errors');
+const uv = process.binding('uv');
+
+function getExpectedError(type) {
+  const code = common.isWindows ? 'ENOTSOCK' : 'EBADF';
+  const message = common.isWindows ?
+    'socket operation on non-socket' : 'bad file descriptor';
+  const errno = common.isWindows ? uv.UV_ENOTSOCK : uv.UV_EBADF;
+  const syscall = `uv_${type}_buffer_size`;
+  const suffix = common.isWindows ?
+    'ENOTSOCK (socket operation on non-socket)' : 'EBADF (bad file descriptor)';
+  const error = {
+    code: 'ERR_SOCKET_BUFFER_SIZE',
+    type: SystemError,
+    message: `Could not get or set buffer size: ${syscall} returned ${suffix}`,
+    info: {
+      code,
+      message,
+      errno,
+      syscall
+    }
+  };
+  return error;
+}
 
 {
   // Should throw error if the socket is never bound.
-  const errorObj = {
-    code: 'ERR_SOCKET_BUFFER_SIZE',
-    type: Error,
-    message: /^Could not get or set buffer size:.*$/
-  };
+  const errorObj = getExpectedError('send');
 
   const socket = dgram.createSocket('udp4');
 
   common.expectsError(() => {
-    socket.setRecvBufferSize(8192);
+    socket.setSendBufferSize(8192);
   }, errorObj);
 
   common.expectsError(() => {
-    socket.setSendBufferSize(8192);
+    socket.getSendBufferSize();
   }, errorObj);
+}
+
+{
+  const socket = dgram.createSocket('udp4');
+
+  // Should throw error if the socket is never bound.
+  const errorObj = getExpectedError('recv');
 
   common.expectsError(() => {
-    socket.getRecvBufferSize();
+    socket.setRecvBufferSize(8192);
   }, errorObj);
 
   common.expectsError(() => {
-    socket.getSendBufferSize();
+    socket.getRecvBufferSize();
   }, errorObj);
 }
 
@@ -73,22 +101,48 @@ const dgram = require('dgram');
   }));
 }
 
-function checkBufferSizeError(type, size) {
+{
+  const info = {
+    code: 'EINVAL',
+    message: 'invalid argument',
+    errno: uv.UV_EINVAL,
+    syscall: 'uv_recv_buffer_size'
+  };
   const errorObj = {
     code: 'ERR_SOCKET_BUFFER_SIZE',
-    type: Error,
-    message: /^Could not get or set buffer size:.*$/
+    type: SystemError,
+    message: 'Could not get or set buffer size: uv_recv_buffer_size ' +
+             'returned EINVAL (invalid argument)',
+    info
   };
-  const functionName = `set${type.charAt(0).toUpperCase()}${type.slice(1)}` +
-    'BufferSize';
   const socket = dgram.createSocket('udp4');
   socket.bind(common.mustCall(() => {
     common.expectsError(() => {
-      socket[functionName](size);
+      socket.setRecvBufferSize(2147483648);
     }, errorObj);
     socket.close();
   }));
 }
 
-checkBufferSizeError('recv', 2147483648);
-checkBufferSizeError('send', 2147483648);
+{
+  const info = {
+    code: 'EINVAL',
+    message: 'invalid argument',
+    errno: uv.UV_EINVAL,
+    syscall: 'uv_send_buffer_size'
+  };
+  const errorObj = {
+    code: 'ERR_SOCKET_BUFFER_SIZE',
+    type: SystemError,
+    message: 'Could not get or set buffer size: uv_send_buffer_size ' +
+             'returned EINVAL (invalid argument)',
+    info
+  };
+  const socket = dgram.createSocket('udp4');
+  socket.bind(common.mustCall(() => {
+    common.expectsError(() => {
+      socket.setSendBufferSize(2147483648);
+    }, errorObj);
+    socket.close();
+  }));
+}
diff --git a/test/parallel/test-errors-systemerror.js b/test/parallel/test-errors-systemerror.js
index 45ac734175..285c89b5d2 100644
--- a/test/parallel/test-errors-systemerror.js
+++ b/test/parallel/test-errors-systemerror.js
@@ -4,149 +4,79 @@
 const common = require('../common');
 const assert = require('assert');
 const errors = require('internal/errors');
+const { AssertionError } = require('assert');
 
-common.expectsError(
-  () => { throw new errors.SystemError(); },
-  {
-    code: 'ERR_SYSTEM_ERROR',
-    type: errors.SystemError,
-    message: 'A system error occurred'
-  }
-);
-
-common.expectsError(
-  () => { throw new errors.SystemError({}); },
-  {
-    code: 'ERR_SYSTEM_ERROR',
-    type: errors.SystemError,
-    message: 'A system error occurred'
-  }
-);
+const { E, SystemError } = errors;
 
 common.expectsError(
-  () => { throw new errors.SystemError(null); },
+  () => { throw new errors.SystemError(); },
   {
-    code: 'ERR_SYSTEM_ERROR',
-    type: errors.SystemError,
-    message: 'A system error occurred'
+    code: 'ERR_ASSERTION',
+    type: AssertionError,
+    message: 'An invalid error message key was used: undefined.'
   }
 );
 
-common.expectsError(
-  () => { throw new errors.SystemError({ code: 'ERR' }); },
-  {
-    code: 'ERR_SYSTEM_ERROR',
-    type: errors.SystemError,
-    message: 'A system error occurred: ERR'
-  }
-);
+E('ERR_TEST', 'custom message', SystemError);
+const { ERR_TEST } = errors.codes;
 
 {
   const ctx = {
-    code: 'ERR',
-    syscall: 'foo'
+    code: 'ETEST',
+    message: 'code message',
+    syscall: 'syscall_test',
+    path: '/str',
+    dest: '/str2'
   };
-  common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
-    {
-      code: 'ERR_SYSTEM_ERROR',
-      type: errors.SystemError,
-      message: 'A system error occurred: ERR [foo]'
-    }
-  );
-}
 
-{
-  const ctx = {
-    code: 'ERR',
-    syscall: 'foo',
-    path: Buffer.from('a')
-  };
   common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
+    () => { throw new ERR_TEST(ctx); },
     {
-      code: 'ERR_SYSTEM_ERROR',
-      type: errors.SystemError,
-      message: 'A system error occurred: ERR [foo]: a'
+      code: 'ERR_TEST',
+      type: SystemError,
+      message: 'custom message: syscall_test returned ETEST (code message)' +
+               ' /str => /str2',
+      info: ctx
     }
   );
 }
 
 {
   const ctx = {
-    code: 'ERR',
-    syscall: 'foo',
-    path: Buffer.from('a'),
-    dest: Buffer.from('b')
-  };
-  common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
-    {
-      code: 'ERR_SYSTEM_ERROR',
-      type: errors.SystemError,
-      message: 'A system error occurred: ERR [foo]: a => b'
-    }
-  );
-}
-
-{
-  const ctx = {
-    syscall: 'foo',
-    path: Buffer.from('a'),
-    dest: Buffer.from('b')
-  };
-  common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
-    {
-      code: 'ERR_SYSTEM_ERROR',
-      type: errors.SystemError,
-      message: 'A system error occurred: [foo]: a => b'
-    }
-  );
-}
-
-{
-  const ctx = {
-    path: Buffer.from('a'),
-    dest: Buffer.from('b')
-  };
-  common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
-    {
-      code: 'ERR_SYSTEM_ERROR',
-      type: errors.SystemError,
-      message: 'A system error occurred: a => b'
-    }
-  );
-}
-
-{
-  const ctx = {
-    code: 'ERR',
-    message: 'something happened',
-    syscall: 'foo',
-    path: Buffer.from('a'),
-    dest: Buffer.from('b')
+    code: 'ETEST',
+    message: 'code message',
+    syscall: 'syscall_test',
+    path: Buffer.from('/buf'),
+    dest: '/str2'
   };
   common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
+    () => { throw new ERR_TEST(ctx); },
     {
-      code: 'ERR_SYSTEM_ERROR',
-      type: errors.SystemError,
-      message: 'something happened: ERR [foo]: a => b'
+      code: 'ERR_TEST',
+      type: SystemError,
+      message: 'custom message: syscall_test returned ETEST (code message)' +
+               ' /buf => /str2',
+      info: ctx
     }
   );
 }
 
 {
   const ctx = {
-    code: 'ERR_ASSERTION'
+    code: 'ETEST',
+    message: 'code message',
+    syscall: 'syscall_test',
+    path: Buffer.from('/buf'),
+    dest: Buffer.from('/buf2')
   };
   common.expectsError(
-    () => { throw new errors.SystemError(ctx); },
+    () => { throw new ERR_TEST(ctx); },
     {
-      code: 'ERR_ASSERTION',
-      type: errors.SystemError
+      code: 'ERR_TEST',
+      type: SystemError,
+      message: 'custom message: syscall_test returned ETEST (code message)' +
+               ' /buf => /buf2',
+      info: ctx
     }
   );
 }
@@ -156,20 +86,20 @@ common.expectsError(
     code: 'ERR',
     errno: 123,
     message: 'something happened',
-    syscall: 'foo',
+    syscall: 'syscall_test',
     path: Buffer.from('a'),
     dest: Buffer.from('b')
   };
-  const err = new errors.SystemError(ctx);
+  const err = new ERR_TEST(ctx);
   assert.strictEqual(err.info, ctx);
-  assert.strictEqual(err.code, 'ERR_SYSTEM_ERROR');
+  assert.strictEqual(err.code, 'ERR_TEST');
   err.code = 'test';
   assert.strictEqual(err.code, 'test');
 
   // Test legacy properties. These shouldn't be used anymore
   // but let us make sure they still work
   assert.strictEqual(err.errno, 123);
-  assert.strictEqual(err.syscall, 'foo');
+  assert.strictEqual(err.syscall, 'syscall_test');
   assert.strictEqual(err.path, 'a');
   assert.strictEqual(err.dest, 'b');
 
diff --git a/test/parallel/test-ttywrap-invalid-fd.js b/test/parallel/test-ttywrap-invalid-fd.js
index adf88cbde6..c360489cb3 100644
--- a/test/parallel/test-ttywrap-invalid-fd.js
+++ b/test/parallel/test-ttywrap-invalid-fd.js
@@ -4,6 +4,7 @@
 const common = require('../common');
 const tty = require('tty');
 const { SystemError } = require('internal/errors');
+const uv = process.binding('uv');
 
 common.expectsError(
   () => new tty.WriteStream(-1),
@@ -15,9 +16,16 @@ common.expectsError(
 );
 
 {
-  const message = common.isWindows ?
-    'bad file descriptor: EBADF [uv_tty_init]' :
-    'invalid argument: EINVAL [uv_tty_init]';
+  const info = {
+    code: common.isWindows ? 'EBADF' : 'EINVAL',
+    message: common.isWindows ? 'bad file descriptor' : 'invalid argument',
+    errno: common.isWindows ? uv.UV_EBADF : uv.UV_EINVAL,
+    syscall: 'uv_tty_init'
+  };
+
+  const suffix = common.isWindows ?
+    'EBADF (bad file descriptor)' : 'EINVAL (invalid argument)';
+  const message = `TTY initialization failed: uv_tty_init returned ${suffix}`;
 
   common.expectsError(
     () => {
@@ -25,9 +33,10 @@ common.expectsError(
         new tty.WriteStream(fd);
       });
     }, {
-      code: 'ERR_SYSTEM_ERROR',
+      code: 'ERR_TTY_INIT_FAILED',
       type: SystemError,
-      message
+      message,
+      info
     }
   );
 
@@ -37,9 +46,10 @@ common.expectsError(
         new tty.ReadStream(fd);
       });
     }, {
-      code: 'ERR_SYSTEM_ERROR',
+      code: 'ERR_TTY_INIT_FAILED',
       type: SystemError,
-      message
+      message,
+      info
     });
 }