Skip to content

Commit

Permalink
lib: tweak use of internal/errors
Browse files Browse the repository at this point in the history
In addition refactor common.throws to common.expectsError

PR-URL: nodejs#13829
Refs: nodejs#11273
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
BridgeAR authored and refack committed Jul 22, 2017
1 parent 8979b4f commit 095357e
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 53 deletions.
4 changes: 2 additions & 2 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ ChildProcess.prototype.spawn = function(options) {
options.envPairs = [];
else if (!Array.isArray(options.envPairs)) {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.envPairs',
'array', options.envPairs);
'Array', options.envPairs);
}

options.envPairs.push('NODE_CHANNEL_FD=' + ipcFd);
Expand All @@ -301,7 +301,7 @@ ChildProcess.prototype.spawn = function(options) {
else if (options.args === undefined)
this.spawnargs = [];
else
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.args', 'array',
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options.args', 'Array',
options.args);

var err = this._handle.spawn(options);
Expand Down
25 changes: 15 additions & 10 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,17 @@ function getConstructorOf(obj) {
const kCustomPromisifiedSymbol = Symbol('util.promisify.custom');
const kCustomPromisifyArgsSymbol = Symbol('customPromisifyArgs');

function promisify(orig) {
if (typeof orig !== 'function')
function promisify(original) {
if (typeof original !== 'function')
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'original', 'function');

if (orig[kCustomPromisifiedSymbol]) {
const fn = orig[kCustomPromisifiedSymbol];
if (original[kCustomPromisifiedSymbol]) {
const fn = original[kCustomPromisifiedSymbol];
if (typeof fn !== 'function') {
throw new TypeError('The [util.promisify.custom] property must be ' +
'a function');
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'util.promisify.custom',
'function',
fn);
}
Object.defineProperty(fn, kCustomPromisifiedSymbol, {
value: fn, enumerable: false, writable: false, configurable: true
Expand All @@ -219,12 +221,12 @@ function promisify(orig) {

// Names to create an object from in case the callback receives multiple
// arguments, e.g. ['stdout', 'stderr'] for child_process.exec.
const argumentNames = orig[kCustomPromisifyArgsSymbol];
const argumentNames = original[kCustomPromisifyArgsSymbol];

function fn(...args) {
const promise = createPromise();
try {
orig.call(this, ...args, (err, ...values) => {
original.call(this, ...args, (err, ...values) => {
if (err) {
promiseReject(promise, err);
} else if (argumentNames !== undefined && values.length > 1) {
Expand All @@ -242,12 +244,15 @@ function promisify(orig) {
return promise;
}

Object.setPrototypeOf(fn, Object.getPrototypeOf(orig));
Object.setPrototypeOf(fn, Object.getPrototypeOf(original));

Object.defineProperty(fn, kCustomPromisifiedSymbol, {
value: fn, enumerable: false, writable: false, configurable: true
});
return Object.defineProperties(fn, Object.getOwnPropertyDescriptors(orig));
return Object.defineProperties(
fn,
Object.getOwnPropertyDescriptors(original)
);
}

promisify.custom = kCustomPromisifiedSymbol;
Expand Down
35 changes: 18 additions & 17 deletions test/parallel/test-child-process-constructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ function typeName(value) {
const child = new ChildProcess();

[undefined, null, 'foo', 0, 1, NaN, true, false].forEach((options) => {
assert.throws(() => {
common.expectsError(() => {
child.spawn(options);
}, common.expectsError({
}, {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options" argument must be of type object. ' +
`Received type ${typeName(options)}`
}));
});
});
}

Expand All @@ -30,14 +30,14 @@ function typeName(value) {
const child = new ChildProcess();

[undefined, null, 0, 1, NaN, true, false, {}].forEach((file) => {
assert.throws(() => {
common.expectsError(() => {
child.spawn({ file });
}, common.expectsError({
}, {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.file" property must be of type string. ' +
`Received type ${typeName(file)}`
}));
});
});
}

Expand All @@ -46,14 +46,14 @@ function typeName(value) {
const child = new ChildProcess();

[null, 0, 1, NaN, true, false, {}, 'foo'].forEach((envPairs) => {
assert.throws(() => {
common.expectsError(() => {
child.spawn({ envPairs, stdio: ['ignore', 'ignore', 'ignore', 'ipc'] });
}, common.expectsError({
}, {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.envPairs" property must be of type array. ' +
message: 'The "options.envPairs" property must be of type Array. ' +
`Received type ${typeName(envPairs)}`
}));
});
});
}

Expand All @@ -62,14 +62,14 @@ function typeName(value) {
const child = new ChildProcess();

[null, 0, 1, NaN, true, false, {}, 'foo'].forEach((args) => {
assert.throws(() => {
common.expectsError(() => {
child.spawn({ file: 'foo', args });
}, common.expectsError({
}, {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: 'The "options.args" property must be of type array. ' +
message: 'The "options.args" property must be of type Array. ' +
`Received type ${typeName(args)}`
}));
});
});
}

Expand All @@ -86,8 +86,9 @@ assert.strictEqual(child.hasOwnProperty('pid'), true);
assert(Number.isInteger(child.pid));

// try killing with invalid signal
assert.throws(() => {
child.kill('foo');
}, common.expectsError({ code: 'ERR_UNKNOWN_SIGNAL', type: TypeError }));
common.expectsError(
() => { child.kill('foo'); },
{ code: 'ERR_UNKNOWN_SIGNAL', type: TypeError }
);

assert.strictEqual(child.kill(), true);
19 changes: 8 additions & 11 deletions test/parallel/test-fs-open-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,29 +55,26 @@ assert.strictEqual(stringToFlags('xa'), O_APPEND | O_CREAT | O_WRONLY | O_EXCL);
assert.strictEqual(stringToFlags('ax+'), O_APPEND | O_CREAT | O_RDWR | O_EXCL);
assert.strictEqual(stringToFlags('xa+'), O_APPEND | O_CREAT | O_RDWR | O_EXCL);

const expectedError =
common.expectsError({ code: 'ERR_INVALID_OPT_VALUE', type: TypeError }, 23);

('+ +a +r +w rw wa war raw r++ a++ w++ x +x x+ rx rx+ wxx wax xwx xxx')
.split(' ')
.forEach(function(flags) {
assert.throws(
common.expectsError(
() => stringToFlags(flags),
expectedError
{ code: 'ERR_INVALID_OPT_VALUE', type: TypeError }
);
});

assert.throws(
common.expectsError(
() => stringToFlags({}),
expectedError
{ code: 'ERR_INVALID_OPT_VALUE', type: TypeError }
);

assert.throws(
common.expectsError(
() => stringToFlags(true),
expectedError
{ code: 'ERR_INVALID_OPT_VALUE', type: TypeError }
);

assert.throws(
common.expectsError(
() => stringToFlags(null),
expectedError
{ code: 'ERR_INVALID_OPT_VALUE', type: TypeError }
);
10 changes: 2 additions & 8 deletions test/parallel/test-fs-read-file-assert-encoding.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');

const encoding = 'foo-8';
const filename = 'bar.txt';
const expectedError = common.expectsError({
code: 'ERR_INVALID_OPT_VALUE_ENCODING',
type: TypeError,
});

assert.throws(
common.expectsError(
fs.readFile.bind(fs, filename, { encoding }, common.mustNotCall()),
expectedError
{ code: 'ERR_INVALID_OPT_VALUE_ENCODING', type: TypeError }
);
9 changes: 4 additions & 5 deletions test/parallel/test-util-promisify.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,10 @@ const stat = promisify(fs.stat);
{
function fn() {}
fn[promisify.custom] = 42;
assert.throws(
() => promisify(fn),
(err) => err instanceof TypeError &&
err.message === 'The [util.promisify.custom] property must ' +
'be a function');
common.expectsError(
() => promisify(fn),
{ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }
);
}

{
Expand Down

0 comments on commit 095357e

Please sign in to comment.