From 76327613aff38f53c017eb26b2209fb1ca2b81b9 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 10 Feb 2017 12:08:49 -0800 Subject: [PATCH] errors, child_process: migrate to using internal/errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/11300 Ref: https://github.com/nodejs/node/issues/11273 Reviewed-By: Michaël Zasso Reviewed-By: Michael Dawson --- doc/api/errors.md | 61 +++++++++++++++++++ lib/internal/child_process.js | 32 +++++----- lib/internal/errors.js | 17 +++++- lib/internal/util.js | 3 +- .../test-child-process-constructor.js | 4 +- .../test-child-process-send-type-error.js | 2 +- ...est-child-process-spawnsync-kill-signal.js | 2 +- ...ild-process-spawnsync-validation-errors.js | 3 +- test/parallel/test-child-process-stdio.js | 2 +- .../test-child-process-validate-stdio.js | 18 +++--- 10 files changed, 111 insertions(+), 33 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index a46d847ca2b23f..75c0b8699f598b 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -599,6 +599,27 @@ Node.js API that consumes `file:` URLs (such as certain functions in the [`fs`][] module) encounters a file URL with an incompatible path. The exact semantics for determining whether a path can be used is platform-dependent. + +### ERR_INVALID_HANDLE_TYPE + +The '`ERR_INVALID_HANDLE_TYPE`' error code is used when an attempt is made to +send an unsupported "handle" over an IPC communication channel to a child +process. See [`child.send()`] and [`process.send()`] for more information. + + +### ERR_INVALID_OPT_VALUE + +The `'ERR_INVALID_OPT_VALUE'` error code is used generically to identify when +an invalid or unexpected value has been passed in an options object. + + +### ERR_INVALID_SYNC_FORK_INPUT + +The `'ERR_INVALID_SYNC_FORK_INPUT'` error code is used when a `Buffer`, +`Uint8Array` or `string` is provided as stdio input to a synchronous +fork. See the documentation for the [`child_process`](child_process.html) +module for more information. + ### ERR_INVALID_THIS @@ -642,6 +663,36 @@ It is currently only used in the [WHATWG URL API][] support in the [`fs`][] module (which only accepts URLs with `'file'` scheme), but may be used in other Node.js APIs as well in the future. + +### ERR_IPC_CHANNEL_CLOSED + +The `'ERR_IPC_CHANNEL_CLOSED'` error code is used when an attempt is made to use +an IPC communication channel that has already been closed. + + +### ERR_IPC_DISCONNECTED + +The `'ERR_IPC_DISCONNECTED'` error code is used when an attempt is made to +disconnect an already disconnected IPC communication channel between two +Node.js processes. See the documentation for the +[`child_process`](child_process.html) module for more information. + + +### ERR_IPC_ONE_PIPE + +The `'ERR_IPC_ONE_PIPE'` error code is used when an attempt is made to create +a child Node.js process using more than one IPC communication channel. +See the documentation for the [`child_process`](child_process.html) +module for more information. + + +### ERR_IPC_SYNC_FORK + +The `'ERR_IPC_SYNC_FORK'` error code is used when an attempt is made to open +an IPC communication channel with a synchronous forked Node.js process. +See the documentation for the [`child_process`](child_process.html) +module for more information. + ### ERR_MISSING_ARGS @@ -674,6 +725,13 @@ kind of internal Node.js error that should not typically be triggered by user code. Instances of this error point to an internal bug within the Node.js binary itself. + +### ERR_UNKNOWN_SIGNAL + +The `'ERR_UNKNOWN_SIGNAL`' error code is used when an invalid or unknown +process signal is passed to an API expecting a valid signal (such as +[`child.kill()`][]). + ### ERR_UNKNOWN_STDIN_TYPE @@ -693,6 +751,9 @@ in user code, although it is not impossible. Occurrences of this error are most likely an indication of a bug within Node.js itself. +[`child.kill()`]: child_process.html#child_process_child_kill_signal +[`child.send()`]: child_process.html#child_process_child_send_message_sendhandle_options_callback +[`process.send()`]: process.html#process_process_send_message_sendhandle_options_callback [`fs.readdir`]: fs.html#fs_fs_readdir_path_options_callback [`fs.readFileSync`]: fs.html#fs_fs_readfilesync_file_options [`fs.unlink`]: fs.html#fs_fs_unlink_path_callback diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index e411a4661dc599..abd4b16d120dc3 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -1,5 +1,6 @@ 'use strict'; +const errors = require('internal/errors'); const StringDecoder = require('string_decoder').StringDecoder; const EventEmitter = require('events'); const net = require('net'); @@ -367,6 +368,7 @@ function onErrorNT(self, err) { ChildProcess.prototype.kill = function(sig) { + const signal = sig === 0 ? sig : convertToValidSignal(sig === undefined ? 'SIGTERM' : sig); @@ -538,7 +540,7 @@ function setupChannel(target, channel) { options = undefined; } else if (options !== undefined && (options === null || typeof options !== 'object')) { - throw new TypeError('"options" argument must be an object'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options', 'Object'); } options = Object.assign({swallowErrors: false}, options); @@ -546,7 +548,7 @@ function setupChannel(target, channel) { if (this.connected) { return this._send(message, handle, options, callback); } - const ex = new Error('channel closed'); + const ex = new errors.Error('ERR_IPC_CHANNEL_CLOSED'); if (typeof callback === 'function') { process.nextTick(callback, ex); } else { @@ -559,7 +561,7 @@ function setupChannel(target, channel) { assert(this.connected || this.channel); if (message === undefined) - throw new TypeError('"message" argument cannot be undefined'); + throw new errors.TypeError('ERR_MISSING_ARGS', 'message'); // Support legacy function signature if (typeof options === 'boolean') { @@ -586,7 +588,7 @@ function setupChannel(target, channel) { } else if (handle instanceof UDP) { message.type = 'dgram.Native'; } else { - throw new TypeError('This handle type can\'t be sent'); + throw new errors.TypeError('ERR_INVALID_HANDLE_TYPE'); } // Queue-up message and handle if we haven't received ACK yet. @@ -686,7 +688,7 @@ function setupChannel(target, channel) { target.disconnect = function() { if (!this.connected) { - this.emit('error', new Error('IPC channel is already disconnected')); + this.emit('error', new errors.Error('ERR_IPC_DISCONNECTED')); return; } @@ -766,11 +768,12 @@ function _validateStdio(stdio, sync) { case 'ignore': stdio = ['ignore', 'ignore', 'ignore']; break; case 'pipe': stdio = ['pipe', 'pipe', 'pipe']; break; case 'inherit': stdio = [0, 1, 2]; break; - default: throw new TypeError('Incorrect value of stdio option: ' + stdio); + default: + throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'stdio', stdio); } } else if (!Array.isArray(stdio)) { - throw new TypeError('Incorrect value of stdio option: ' + - util.inspect(stdio)); + throw new errors.TypeError('ERR_INVALID_OPT_VALUE', + 'stdio', util.inspect(stdio)); } // At least 3 stdio will be created @@ -812,9 +815,9 @@ function _validateStdio(stdio, sync) { // Cleanup previously created pipes cleanup(); if (!sync) - throw new Error('Child process can have only one IPC pipe'); + throw new errors.Error('ERR_IPC_ONE_PIPE'); else - throw new Error('You cannot use IPC with synchronous forks'); + throw new errors.Error('ERR_IPC_SYNC_FORK'); } ipc = new Pipe(true); @@ -849,15 +852,14 @@ function _validateStdio(stdio, sync) { } else if (isUint8Array(stdio) || typeof stdio === 'string') { if (!sync) { cleanup(); - throw new TypeError('Asynchronous forks do not support ' + - 'Buffer, Uint8Array or string input: ' + - util.inspect(stdio)); + throw new errors.TypeError('ERR_INVALID_SYNC_FORK_INPUT', + util.inspect(stdio)); } } else { // Cleanup cleanup(); - throw new TypeError('Incorrect value for stdio stream: ' + - util.inspect(stdio)); + throw new errors.TypeError('ERR_INVALID_OPT_VALUE', 'stdio', + util.inspect(stdio)); } return acc; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index fbaa06170d34c8..eb4eecd29b6890 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -91,17 +91,32 @@ E('ERR_INVALID_ARG_TYPE', invalidArgType); E('ERR_INVALID_CALLBACK', 'callback must be a function'); E('ERR_INVALID_FILE_URL_HOST', 'File URL host %s'); E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s'); +E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent'); +E('ERR_INVALID_OPT_VALUE', + (name, value) => { + return `The value "${String(value)}" is invalid for option "${name}"`; + }); +E('ERR_INVALID_SYNC_FORK_INPUT', + (value) => { + return 'Asynchronous forks do not support Buffer, Uint8Array or string' + + `input: ${value}`; + }); E('ERR_INVALID_THIS', 'Value of "this" must be of type %s'); E('ERR_INVALID_TUPLE', '%s must be an iterable %s tuple'); E('ERR_INVALID_URL', 'Invalid URL: %s'); E('ERR_INVALID_URL_SCHEME', (expected) => `The URL must be ${oneOf(expected, 'scheme')}`); +E('ERR_IPC_CHANNEL_CLOSED', 'channel closed'); +E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected'); +E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe'); +E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks'); E('ERR_MISSING_ARGS', missingArgs); E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed'); E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed'); +E('ERR_UNKNOWN_BUILTIN_MODULE', (id) => `No such built-in module: ${id}`); +E('ERR_UNKNOWN_SIGNAL', (signal) => `Unknown signal: ${signal}`); E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type'); E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type'); -E('ERR_UNKNOWN_BUILTIN_MODULE', (id) => `No such built-in module: ${id}`); // Add new errors from here... function invalidArgType(name, expected, actual) { diff --git a/lib/internal/util.js b/lib/internal/util.js index 8b2148fd6a00c6..4c11e59271a601 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -1,5 +1,6 @@ 'use strict'; +const errors = require('internal/errors'); const binding = process.binding('util'); const signals = process.binding('constants').os.signals; @@ -177,7 +178,7 @@ function convertToValidSignal(signal) { if (signalName) return signalName; } - throw new Error('Unknown signal: ' + signal); + throw new errors.Error('ERR_UNKNOWN_SIGNAL', signal); } function getConstructorOf(obj) { diff --git a/test/parallel/test-child-process-constructor.js b/test/parallel/test-child-process-constructor.js index 7e86ad198fceeb..c495f1895d0002 100644 --- a/test/parallel/test-child-process-constructor.js +++ b/test/parallel/test-child-process-constructor.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const { ChildProcess } = require('child_process'); assert.strictEqual(typeof ChildProcess, 'function'); @@ -64,6 +64,6 @@ assert(Number.isInteger(child.pid)); // try killing with invalid signal assert.throws(() => { child.kill('foo'); -}, /^Error: Unknown signal: foo$/); +}, common.expectsError({ code: 'ERR_UNKNOWN_SIGNAL' })); assert.strictEqual(child.kill(), true); diff --git a/test/parallel/test-child-process-send-type-error.js b/test/parallel/test-child-process-send-type-error.js index dc305c368963bd..40c415666f20cc 100644 --- a/test/parallel/test-child-process-send-type-error.js +++ b/test/parallel/test-child-process-send-type-error.js @@ -6,7 +6,7 @@ const cp = require('child_process'); function fail(proc, args) { assert.throws(() => { proc.send.apply(proc, args); - }, /"options" argument must be an object/); + }, common.expectsError({code: 'ERR_INVALID_ARG_TYPE', type: TypeError})); } let target = process; diff --git a/test/parallel/test-child-process-spawnsync-kill-signal.js b/test/parallel/test-child-process-spawnsync-kill-signal.js index 906718e5b7fa6e..1b8b267ff6b170 100644 --- a/test/parallel/test-child-process-spawnsync-kill-signal.js +++ b/test/parallel/test-child-process-spawnsync-kill-signal.js @@ -21,7 +21,7 @@ if (process.argv[2] === 'child') { // Verify that an error is thrown for unknown signals. assert.throws(() => { spawn('SIG_NOT_A_REAL_SIGNAL'); - }, /Error: Unknown signal: SIG_NOT_A_REAL_SIGNAL/); + }, common.expectsError({ code: 'ERR_UNKNOWN_SIGNAL' })); // Verify that the default kill signal is SIGTERM. { diff --git a/test/parallel/test-child-process-spawnsync-validation-errors.js b/test/parallel/test-child-process-spawnsync-validation-errors.js index 83b114f7ba76b2..7ac18ba7555b68 100644 --- a/test/parallel/test-child-process-spawnsync-validation-errors.js +++ b/test/parallel/test-child-process-spawnsync-validation-errors.js @@ -185,7 +185,8 @@ if (!common.isWindows) { { // Validate the killSignal option const typeErr = /^TypeError: "killSignal" must be a string or number$/; - const unknownSignalErr = /^Error: Unknown signal:/; + const unknownSignalErr = + common.expectsError({ code: 'ERR_UNKNOWN_SIGNAL' }); pass('killSignal', undefined); pass('killSignal', null); diff --git a/test/parallel/test-child-process-stdio.js b/test/parallel/test-child-process-stdio.js index 00c002e89d6cda..eda2d8841a8f5c 100644 --- a/test/parallel/test-child-process-stdio.js +++ b/test/parallel/test-child-process-stdio.js @@ -42,4 +42,4 @@ assert.deepStrictEqual(options, {stdio: 'ignore'}); assert.throws(() => { common.spawnPwd({stdio: ['pipe', 'pipe', 'pipe', 'ipc', 'ipc']}); -}, /^Error: Child process can have only one IPC pipe$/); +}, common.expectsError({code: 'ERR_IPC_ONE_PIPE', type: Error})); diff --git a/test/parallel/test-child-process-validate-stdio.js b/test/parallel/test-child-process-validate-stdio.js index 384efdf15a1425..c6a9bd8e19129c 100644 --- a/test/parallel/test-child-process-validate-stdio.js +++ b/test/parallel/test-child-process-validate-stdio.js @@ -1,19 +1,18 @@ 'use strict'; // Flags: --expose_internals -require('../common'); +const common = require('../common'); const assert = require('assert'); const _validateStdio = require('internal/child_process')._validateStdio; +const expectedError = + common.expectsError({code: 'ERR_INVALID_OPT_VALUE', type: TypeError}); + // should throw if string and not ignore, pipe, or inherit -assert.throws(function() { - _validateStdio('foo'); -}, /Incorrect value of stdio option/); +assert.throws(() => _validateStdio('foo'), expectedError); // should throw if not a string or array -assert.throws(function() { - _validateStdio(600); -}, /Incorrect value of stdio option/); +assert.throws(() => _validateStdio(600), expectedError); // should populate stdio with undefined if len < 3 { @@ -27,9 +26,8 @@ assert.throws(function() { // should throw if stdio has ipc and sync is true const stdio2 = ['ipc', 'ipc', 'ipc']; -assert.throws(function() { - _validateStdio(stdio2, true); -}, /You cannot use IPC with synchronous forks/); +assert.throws(() => _validateStdio(stdio2, true), + common.expectsError({code: 'ERR_IPC_SYNC_FORK', type: Error})); { const stdio3 = [process.stdin, process.stdout, process.stderr];