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>
This commit is contained in:
Joyee Cheung 2018-03-21 00:46:30 +08:00
parent 3e0d40d4af
commit 7d06761f83
No known key found for this signature in database
GPG Key ID: F586868AAD831D0C
8 changed files with 270 additions and 272 deletions

View File

@ -1521,6 +1521,11 @@ A Transform stream finished while it was still transforming.
A Transform stream finished with data still in the write buffer.
<a id="ERR_TTY_INIT_FAILED"></a>
### ERR_TTY_INIT_FAILED
The initialization of a TTY failed due to a system error.
<a id="ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET"></a>
### ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET

View File

@ -188,7 +188,7 @@ function bufferSize(self, size, buffer) {
const ctx = {};
const ret = self._handle.bufferSize(size, buffer, ctx);
if (ret === undefined) {
throw new ERR_SOCKET_BUFFER_SIZE(new errors.SystemError(ctx));
throw new ERR_SOCKET_BUFFER_SIZE(ctx);
}
return ret;
}

View File

@ -47,120 +47,65 @@ function inspectValue(val) {
).split('\n');
}
function makeNodeError(Base) {
return class NodeError extends Base {
constructor(key, ...args) {
super(message(key, args));
defineProperty(this, kCode, {
configurable: true,
enumerable: false,
value: key,
writable: true
});
}
get name() {
return `${super.name} [${this[kCode]}]`;
}
set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
get code() {
return this[kCode];
}
set code(value) {
defineProperty(this, 'code', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
};
}
function makeNodeErrorWithCode(Base, key) {
return class NodeError extends Base {
constructor(...args) {
super(message(key, args));
}
get name() {
return `${super.name} [${key}]`;
}
set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
get code() {
return key;
}
set code(value) {
defineProperty(this, 'code', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
};
}
// Utility function for registering the error codes. Only used here. Exported
// *only* to allow for testing.
function E(sym, val, def, ...otherClasses) {
messages.set(sym, val);
def = makeNodeErrorWithCode(def, sym);
if (otherClasses.length !== 0) {
otherClasses.forEach((clazz) => {
def[clazz.name] = makeNodeErrorWithCode(clazz, sym);
});
}
codes[sym] = def;
}
function lazyBuffer() {
if (buffer === undefined)
buffer = require('buffer').Buffer;
return buffer;
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. The code key will
// be extracted from the context object or the ERR_SYSTEM_ERROR default
// will be used.
class SystemError extends makeNodeError(Error) {
constructor(context) {
// additional information about the error condition.
// It has the properties present in a UVException but with a custom error
// message followed by the uv error code and uv error message.
// It also has its own error code with the original uv error context put into
// `err.info`.
// 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 || {};
let code = 'ERR_SYSTEM_ERROR';
if (messages.has(context.code))
code = context.code;
super(code,
context.code,
context.syscall,
context.path,
context.dest,
context.message);
super(sysErrorMessage(message(key), context));
Object.defineProperty(this, kInfo, {
configurable: false,
enumerable: false,
value: context
});
Object.defineProperty(this, kCode, {
configurable: true,
enumerable: false,
value: key,
writable: true
});
}
get name() {
return `SystemError [${this[kCode]}]`;
}
set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
get code() {
return this[kCode];
}
set code(value) {
defineProperty(this, 'code', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
get info() {
@ -204,6 +149,74 @@ class SystemError extends makeNodeError(Error) {
}
}
function makeSystemErrorWithCode(key) {
return class NodeError extends SystemError {
constructor(...args) {
super(key, ...args);
}
};
}
function makeNodeErrorWithCode(Base, key) {
return class NodeError extends Base {
constructor(...args) {
super(message(key, args));
}
get name() {
return `${super.name} [${key}]`;
}
set name(value) {
defineProperty(this, 'name', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
get code() {
return key;
}
set code(value) {
defineProperty(this, 'code', {
configurable: true,
enumerable: true,
value,
writable: true
});
}
};
}
// Utility function for registering the error codes. Only used here. Exported
// *only* to allow for testing.
function E(sym, val, def, ...otherClasses) {
// Special case for SystemError that formats the error message differently
// The SystemErrors only have SystemError as their base classes.
messages.set(sym, val);
if (def === SystemError) {
def = makeSystemErrorWithCode(sym);
} else {
def = makeNodeErrorWithCode(def, sym);
}
if (otherClasses.length !== 0) {
otherClasses.forEach((clazz) => {
def[clazz.name] = makeNodeErrorWithCode(clazz, sym);
});
}
codes[sym] = def;
}
function lazyBuffer() {
if (buffer === undefined)
buffer = require('buffer').Buffer;
return buffer;
}
function createErrDiff(actual, expected, operator) {
var other = '';
var res = '';
@ -872,7 +885,9 @@ E('ERR_SOCKET_BAD_PORT',
'Port should be > 0 and < 65536. Received %s.', RangeError);
E('ERR_SOCKET_BAD_TYPE',
'Bad socket type specified. Valid types are: udp4, udp6', TypeError);
E('ERR_SOCKET_BUFFER_SIZE', 'Could not get or set buffer size: %s', Error);
E('ERR_SOCKET_BUFFER_SIZE',
'Could not get or set buffer size',
SystemError);
E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data', Error);
E('ERR_SOCKET_CLOSED', 'Socket is closed', Error);
E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running', Error);
@ -886,6 +901,7 @@ E('ERR_STREAM_UNSHIFT_AFTER_END_EVENT',
'stream.unshift() after end event', Error);
E('ERR_STREAM_WRAP', 'Stream has StringDecoder set or is in objectMode', Error);
E('ERR_STREAM_WRITE_AFTER_END', 'write after end', Error);
E('ERR_SYSTEM_ERROR', 'A system error occurred', SystemError);
E('ERR_TLS_CERT_ALTNAME_INVALID',
'Hostname/IP does not match certificate\'s altnames: %s', Error);
E('ERR_TLS_DH_PARAM_SIZE', 'DH parameter size %s is less than 2048', Error);
@ -905,6 +921,7 @@ E('ERR_TRANSFORM_ALREADY_TRANSFORMING',
// This should probably be a `RangeError`.
E('ERR_TRANSFORM_WITH_LENGTH_0',
'Calling transform done when writableState.length != 0', Error);
E('ERR_TTY_INIT_FAILED', 'TTY initialization failed', SystemError);
E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET',
'`process.setupUncaughtExceptionCapture()` was called while a capture ' +
'callback was already active',
@ -945,24 +962,6 @@ E('ERR_VM_MODULE_NOT_MODULE',
E('ERR_VM_MODULE_STATUS', 'Module status %s', Error);
E('ERR_ZLIB_INITIALIZATION_FAILED', 'Initialization failed', Error);
function sysError(code, syscall, path, dest,
message = 'A system error occurred') {
if (code !== undefined)
message += `: ${code}`;
if (syscall !== undefined) {
if (code === undefined)
message += ':';
message += ` [${syscall}]`;
}
if (path !== undefined) {
message += `: ${path}`;
if (dest !== undefined)
message += ` => ${dest}`;
}
return message;
}
messages.set('ERR_SYSTEM_ERROR', sysError);
function invalidArgType(name, expected, actual) {
internalAssert(typeof name === 'string');
internalAssert(arguments.length === 3);

View File

@ -27,7 +27,7 @@ const { deprecate } = require('internal/util');
const { getCIDRSuffix } = require('internal/os');
const isWindows = process.platform === 'win32';
const errors = require('internal/errors');
const { ERR_SYSTEM_ERROR } = require('internal/errors');
const {
getCPUs,
@ -49,7 +49,7 @@ function getCheckedFunction(fn) {
const ctx = {};
const ret = fn(...args, ctx);
if (ret === undefined) {
const err = new errors.SystemError(ctx);
const err = new ERR_SYSTEM_ERROR(ctx);
Error.captureStackTrace(err, checkError);
throw err;
}

View File

@ -25,7 +25,7 @@ const { inherits, _extend } = require('util');
const net = require('net');
const { TTY, isTTY } = process.binding('tty_wrap');
const errors = require('internal/errors');
const { ERR_INVALID_FD } = errors.codes;
const { ERR_INVALID_FD, ERR_TTY_INIT_FAILED } = errors.codes;
const readline = require('readline');
const { getColorDepth } = require('internal/tty');
@ -42,7 +42,7 @@ function ReadStream(fd, options) {
const ctx = {};
const tty = new TTY(fd, true, ctx);
if (ctx.code !== undefined) {
throw new errors.SystemError(ctx);
throw new ERR_TTY_INIT_FAILED(ctx);
}
options = _extend({
@ -74,7 +74,7 @@ function WriteStream(fd) {
const ctx = {};
const tty = new TTY(fd, false, ctx);
if (ctx.code !== undefined) {
throw new errors.SystemError(ctx);
throw new ERR_TTY_INIT_FAILED(ctx);
}
net.Socket.call(this, {

View File

@ -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);
}, errorObj);
common.expectsError(() => {
socket.setSendBufferSize(8192);
}, errorObj);
common.expectsError(() => {
socket.getRecvBufferSize();
socket.getSendBufferSize();
}, errorObj);
}
{
const socket = dgram.createSocket('udp4');
// Should throw error if the socket is never bound.
const errorObj = getExpectedError('recv');
common.expectsError(() => {
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();
}));
}

View File

@ -4,149 +4,79 @@
const common = require('../common');
const assert = require('assert');
const errors = require('internal/errors');
const { AssertionError } = require('assert');
const { E, SystemError } = errors;
common.expectsError(
() => { 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_SYSTEM_ERROR',
type: errors.SystemError,
message: 'A system error occurred'
}
);
common.expectsError(
() => { throw new errors.SystemError(null); },
{
code: 'ERR_SYSTEM_ERROR',
type: errors.SystemError,
message: 'A system error occurred'
}
);
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); },
() => { throw new ERR_TEST(ctx); },
{
code: 'ERR_SYSTEM_ERROR',
type: errors.SystemError,
message: 'A system error occurred: ERR [foo]'
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')
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: 'A system error occurred: ERR [foo]: a'
code: 'ERR_TEST',
type: SystemError,
message: 'custom message: syscall_test returned ETEST (code message)' +
' /buf => /str2',
info: ctx
}
);
}
{
const ctx = {
code: 'ERR',
syscall: 'foo',
path: Buffer.from('a'),
dest: Buffer.from('b')
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_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')
};
common.expectsError(
() => { throw new errors.SystemError(ctx); },
{
code: 'ERR_SYSTEM_ERROR',
type: errors.SystemError,
message: 'something happened: ERR [foo]: a => b'
}
);
}
{
const ctx = {
code: 'ERR_ASSERTION'
};
common.expectsError(
() => { throw new errors.SystemError(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');

View File

@ -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
});
}