errors: minor (SystemError) refactoring

This removes the former default values and the spread arguments
usage. That was unnecessary and now it does only what is necessary.
The `message` function got renamed to `getMessage` to outline that
it is actually a function and a helper function was inlined into
the SystemError constructor as it was only used there.

PR-URL: https://github.com/nodejs/node/pull/20337
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/errors.js

@@ -78,16 +78,6 @@ function inspectValue(val) {
   ).split('\n');
 }
 
-function sysErrorMessage(prefix, ctx) {
-  let message = `${prefix}: ${ctx.syscall} returned ` +
-                `${ctx.code} (${ctx.message})`;
-  if (ctx.path !== undefined)
-    message += ` ${ctx.path}`;
-  if (ctx.dest !== undefined)
-    message += ` => ${ctx.dest}`;
-  return message;
-}
-
 // A specialized Error that includes an additional info property with
 // additional information about the error condition.
 // It has the properties present in a UVException but with a custom error
@@ -97,9 +87,17 @@ function sysErrorMessage(prefix, ctx) {
 // The context passed into this error must have .code, .syscall and .message,
 // and may have .path and .dest.
 class SystemError extends Error {
-  constructor(key, context = {}) {
+  constructor(key, context) {
-    context = context || {};
+    const prefix = getMessage(key, []);
-    super(sysErrorMessage(message(key), context));
+    let message = `${prefix}: ${context.syscall} returned ` +
+                  `${context.code} (${context.message})`;
+
+    if (context.path !== undefined)
+      message += ` ${context.path}`;
+    if (context.dest !== undefined)
+      message += ` => ${context.dest}`;
+
+    super(message);
     Object.defineProperty(this, kInfo, {
       configurable: false,
       enumerable: false,
@@ -182,8 +180,8 @@ class SystemError extends Error {
 
 function makeSystemErrorWithCode(key) {
   return class NodeError extends SystemError {
-    constructor(...args) {
+    constructor(ctx) {
-      super(key, ...args);
+      super(key, ctx);
     }
   };
 }
@@ -191,7 +189,7 @@ function makeSystemErrorWithCode(key) {
 function makeNodeErrorWithCode(Base, key) {
   return class NodeError extends Base {
     constructor(...args) {
-      super(message(key, args));
+      super(getMessage(key, args));
     }
 
     get name() {
@@ -484,7 +482,7 @@ function internalAssert(condition, message) {
   }
 }
 
-function message(key, args = []) {
+function getMessage(key, args) {
   const msg = messages.get(key);
   if (util === undefined) util = require('util');
 
@@ -691,7 +689,7 @@ module.exports = {
   exceptionWithHostPort,
   uvException,
   isStackOverflowError,
-  message,
+  getMessage,
   AssertionError,
   SystemError,
   codes,

test/parallel/test-buffer-fill.js

@@ -2,7 +2,7 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const errors = require('internal/errors');
+const { codes: { ERR_INDEX_OUT_OF_RANGE } } = require('internal/errors');
 const SIZE = 28;
 
 const buf1 = Buffer.allocUnsafe(SIZE);
@@ -214,13 +214,11 @@ function genBuffer(size, args) {
   return b.fill(0).fill.apply(b, args);
 }
 
-
 function bufReset() {
   buf1.fill(0);
   buf2.fill(0);
 }
 
-
 // This is mostly accurate. Except write() won't write partial bytes to the
 // string while fill() blindly copies bytes into memory. To account for that an
 // error will be thrown if not all the data can be written, and the SIZE has
@@ -237,8 +235,9 @@ function writeToFill(string, offset, end, encoding) {
     end = buf2.length;
   }
 
+  // Should never be reached.
   if (offset < 0 || end > buf2.length)
-    throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE');
+    throw new ERR_INDEX_OUT_OF_RANGE();
 
   if (end <= offset)
     return buf2;
@@ -266,7 +265,6 @@ function writeToFill(string, offset, end, encoding) {
   return buf2;
 }
 
-
 function testBufs(string, offset, length, encoding) {
   bufReset();
   buf1.fill.apply(buf1, arguments);

test/parallel/test-errors-systemerror.js

@@ -3,12 +3,10 @@
 
 require('../common');
 const assert = require('assert');
-const errors = require('internal/errors');
+const { E, SystemError, codes } = require('internal/errors');
-
-const { E, SystemError } = errors;
 
 assert.throws(
-  () => { throw new errors.SystemError(); },
+  () => { throw new SystemError(); },
   {
     name: 'TypeError',
     message: 'Cannot read property \'match\' of undefined'
@@ -16,7 +14,7 @@ assert.throws(
 );
 
 E('ERR_TEST', 'custom message', SystemError);
-const { ERR_TEST } = errors.codes;
+const { ERR_TEST } = codes;
 
 {
   const ctx = {

test/parallel/test-internal-errors.js

@@ -93,20 +93,22 @@ common.expectsError(() => {
 });
 
 // Test ERR_INVALID_FD_TYPE
-assert.strictEqual(errors.message('ERR_INVALID_FD_TYPE', ['a']),
+assert.strictEqual(errors.getMessage('ERR_INVALID_FD_TYPE', ['a']),
                    'Unsupported fd type: a');
 
 // Test ERR_INVALID_URL_SCHEME
-assert.strictEqual(errors.message('ERR_INVALID_URL_SCHEME', ['file']),
+assert.strictEqual(errors.getMessage('ERR_INVALID_URL_SCHEME', ['file']),
                    'The URL must be of scheme file');
-assert.strictEqual(errors.message('ERR_INVALID_URL_SCHEME', [['file']]),
+assert.strictEqual(errors.getMessage('ERR_INVALID_URL_SCHEME', [['file']]),
                    'The URL must be of scheme file');
-assert.strictEqual(errors.message('ERR_INVALID_URL_SCHEME', [['http', 'ftp']]),
+assert.strictEqual(errors.getMessage('ERR_INVALID_URL_SCHEME',
+                                     [['http', 'ftp']]),
                    'The URL must be one of scheme http or ftp');
-assert.strictEqual(errors.message('ERR_INVALID_URL_SCHEME', [['a', 'b', 'c']]),
+assert.strictEqual(errors.getMessage('ERR_INVALID_URL_SCHEME',
+                                     [['a', 'b', 'c']]),
                    'The URL must be one of scheme a, b, or c');
 common.expectsError(
-  () => errors.message('ERR_INVALID_URL_SCHEME', [[]]),
+  () => errors.getMessage('ERR_INVALID_URL_SCHEME', [[]]),
   {
     code: 'ERR_ASSERTION',
     type: assert.AssertionError,
@@ -114,74 +116,67 @@ common.expectsError(
   });
 
 // Test ERR_MISSING_ARGS
-assert.strictEqual(errors.message('ERR_MISSING_ARGS', ['name']),
+assert.strictEqual(errors.getMessage('ERR_MISSING_ARGS', ['name']),
                    'The "name" argument must be specified');
-assert.strictEqual(errors.message('ERR_MISSING_ARGS', ['name', 'value']),
+assert.strictEqual(errors.getMessage('ERR_MISSING_ARGS', ['name', 'value']),
                    'The "name" and "value" arguments must be specified');
-assert.strictEqual(errors.message('ERR_MISSING_ARGS', ['a', 'b', 'c']),
+assert.strictEqual(errors.getMessage('ERR_MISSING_ARGS', ['a', 'b', 'c']),
                    'The "a", "b", and "c" arguments must be specified');
-common.expectsError(
-  () => errors.message('ERR_MISSING_ARGS'),
-  {
-    code: 'ERR_ASSERTION',
-    type: assert.AssertionError,
-    message: /^At least one arg needs to be specified$/
-  });
 
 // Test ERR_SOCKET_BAD_PORT
 assert.strictEqual(
-  errors.message('ERR_SOCKET_BAD_PORT', [0]),
+  errors.getMessage('ERR_SOCKET_BAD_PORT', [0]),
   'Port should be > 0 and < 65536. Received 0.');
 
 // Test ERR_TLS_CERT_ALTNAME_INVALID
 assert.strictEqual(
-  errors.message('ERR_TLS_CERT_ALTNAME_INVALID', ['altname']),
+  errors.getMessage('ERR_TLS_CERT_ALTNAME_INVALID', ['altname']),
   'Hostname/IP does not match certificate\'s altnames: altname');
 
 assert.strictEqual(
-  errors.message('ERR_INVALID_PROTOCOL', ['bad protocol', 'http']),
+  errors.getMessage('ERR_INVALID_PROTOCOL', ['bad protocol', 'http']),
   'Protocol "bad protocol" not supported. Expected "http"'
 );
 
 assert.strictEqual(
-  errors.message('ERR_HTTP_HEADERS_SENT', ['render']),
+  errors.getMessage('ERR_HTTP_HEADERS_SENT', ['render']),
   'Cannot render headers after they are sent to the client'
 );
 
 assert.strictEqual(
-  errors.message('ERR_INVALID_HTTP_TOKEN', ['Method', 'foo']),
+  errors.getMessage('ERR_INVALID_HTTP_TOKEN', ['Method', 'foo']),
   'Method must be a valid HTTP token ["foo"]'
 );
 
 assert.strictEqual(
-  errors.message('ERR_OUT_OF_RANGE', ['A', 'some values', 'B']),
+  errors.getMessage('ERR_OUT_OF_RANGE', ['A', 'some values', 'B']),
   'The value of "A" is out of range. It must be some values. Received B'
 );
 
 assert.strictEqual(
-  errors.message('ERR_UNESCAPED_CHARACTERS', ['Request path']),
+  errors.getMessage('ERR_UNESCAPED_CHARACTERS', ['Request path']),
   'Request path contains unescaped characters'
 );
 
 // Test ERR_DNS_SET_SERVERS_FAILED
 assert.strictEqual(
-  errors.message('ERR_DNS_SET_SERVERS_FAILED', ['err', 'servers']),
+  errors.getMessage('ERR_DNS_SET_SERVERS_FAILED', ['err', 'servers']),
   'c-ares failed to set servers: "err" [servers]');
 
 // Test ERR_ENCODING_NOT_SUPPORTED
 assert.strictEqual(
-  errors.message('ERR_ENCODING_NOT_SUPPORTED', ['enc']),
+  errors.getMessage('ERR_ENCODING_NOT_SUPPORTED', ['enc']),
   'The "enc" encoding is not supported');
 
 // Test error messages for async_hooks
 assert.strictEqual(
-  errors.message('ERR_ASYNC_CALLBACK', ['init']),
+  errors.getMessage('ERR_ASYNC_CALLBACK', ['init']),
   'init must be a function');
 assert.strictEqual(
-  errors.message('ERR_ASYNC_TYPE', [{}]),
+  errors.getMessage('ERR_ASYNC_TYPE', [{}]),
   'Invalid name for async "type": [object Object]');
 assert.strictEqual(
-  errors.message('ERR_INVALID_ASYNC_ID', ['asyncId', undefined]),
+  errors.getMessage('ERR_INVALID_ASYNC_ID', ['asyncId', undefined]),
   'Invalid asyncId value: undefined');
 
 {