Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: minor (SystemError) refactoring #20337

Closed
wants to merge 1 commit into from
Closed

errors: minor (SystemError) refactoring #20337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 16 additions & 18 deletions lib/internal/errors.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
51 changes: 23 additions & 28 deletions test/parallel/test-internal-errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,95 +93,90 @@ 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_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