Skip to content

Commit

Permalink
errors: minor (SystemError) refactoring
Browse files Browse the repository at this point in the history 
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: #20337
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@BridgeAR @MylesBorins
BridgeAR authored and MylesBorins committed May 4, 2018
1 parent 58be6ef commit b89d817
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 57 deletions.
34 changes: 16 additions & 18 deletions lib/internal/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,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
Expand All @@ -98,9 +88,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 = {}) {
context = context || {};
super(sysErrorMessage(message(key), context));
constructor(key, context) {
const prefix = getMessage(key, []);
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,
Expand Down Expand Up @@ -183,16 +181,16 @@ class SystemError extends Error {

function makeSystemErrorWithCode(key) {
return class NodeError extends SystemError {
constructor(...args) {
super(key, ...args);
constructor(ctx) {
super(key, ctx);
}
};
}

function makeNodeErrorWithCode(Base, key) {
return class NodeError extends Base {
constructor(...args) {
super(message(key, args));
super(getMessage(key, args));
}

get name() {
Expand Down Expand Up @@ -485,7 +483,7 @@ function internalAssert(condition, message) {
}
}

function message(key, args = []) {
function getMessage(key, args) {
const msg = messages.get(key);
if (util === undefined) util = require('util');

Expand Down Expand Up @@ -694,7 +692,7 @@ module.exports = {
exceptionWithHostPort,
uvException,
isStackOverflowError,
message,
getMessage,
AssertionError,
SystemError,
codes,
Expand Down
8 changes: 3 additions & 5 deletions test/parallel/test-buffer-fill.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -266,7 +265,6 @@ function writeToFill(string, offset, end, encoding) {
return buf2;
}


function testBufs(string, offset, length, encoding) {
bufReset();
buf1.fill.apply(buf1, arguments);
Expand Down
8 changes: 3 additions & 5 deletions test/parallel/test-errors-systemerror.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,18 @@

require('../common');
const assert = require('assert');
const errors = require('internal/errors');

const { E, SystemError } = errors;
const { E, SystemError, codes } = require('internal/errors');

assert.throws(
() => { throw new errors.SystemError(); },
() => { throw new SystemError(); },
{
name: 'TypeError',
message: 'Cannot read property \'match\' of undefined'
}
);

E('ERR_TEST', 'custom message', SystemError);
const { ERR_TEST } = errors.codes;
const { ERR_TEST } = codes;

{
const ctx = {
Expand Down
53 changes: 24 additions & 29 deletions test/parallel/test-internal-errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,100 +93,95 @@ 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,
message: /^At least one expected value needs to be specified$/
});

// 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_DOMAIN_NAME'),
errors.getMessage('ERR_INVALID_DOMAIN_NAME', []),
'Unable to determine the domain name'
);

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');

{
Expand Down

0 comments on commit b89d817

Please sign in to comment.