From 095357e26efc366b1cca389306e0780cc1fa81d9 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 20 Jun 2017 23:20:10 +0200 Subject: [PATCH] lib: tweak use of internal/errors In addition refactor common.throws to common.expectsError PR-URL: https://github.com/nodejs/node/pull/13829 Refs: https://github.com/nodejs/node/issues/11273 Reviewed-By: Refael Ackermann Reviewed-By: Michael Dawson Reviewed-By: Matteo Collina --- lib/internal/child_process.js | 4 +-- lib/internal/util.js | 25 +++++++------ .../test-child-process-constructor.js | 35 ++++++++++--------- test/parallel/test-fs-open-flags.js | 19 +++++----- .../test-fs-read-file-assert-encoding.js | 10 ++---- test/parallel/test-util-promisify.js | 9 +++-- 6 files changed, 49 insertions(+), 53 deletions(-) diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index b88f5423acc435..310987318509c0 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -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); @@ -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); diff --git a/lib/internal/util.js b/lib/internal/util.js index 735f78d4aca35d..e7b9b958c7a198 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -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 @@ -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) { @@ -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; diff --git a/test/parallel/test-child-process-constructor.js b/test/parallel/test-child-process-constructor.js index fe94ff066ed815..92a148c0bab06c 100644 --- a/test/parallel/test-child-process-constructor.js +++ b/test/parallel/test-child-process-constructor.js @@ -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)}` - })); + }); }); } @@ -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)}` - })); + }); }); } @@ -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)}` - })); + }); }); } @@ -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)}` - })); + }); }); } @@ -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); diff --git a/test/parallel/test-fs-open-flags.js b/test/parallel/test-fs-open-flags.js index 8f68b407df91a1..873bd7112828eb 100644 --- a/test/parallel/test-fs-open-flags.js +++ b/test/parallel/test-fs-open-flags.js @@ -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 } ); diff --git a/test/parallel/test-fs-read-file-assert-encoding.js b/test/parallel/test-fs-read-file-assert-encoding.js index 4770db6eb7e091..2640355b56e2ce 100644 --- a/test/parallel/test-fs-read-file-assert-encoding.js +++ b/test/parallel/test-fs-read-file-assert-encoding.js @@ -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 } ); diff --git a/test/parallel/test-util-promisify.js b/test/parallel/test-util-promisify.js index 08a21b3d2c92df..656d6d60aa8f13 100644 --- a/test/parallel/test-util-promisify.js +++ b/test/parallel/test-util-promisify.js @@ -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 } + ); } {