From 93acf251fdfb4b1eb61443d5e8b6261d7be37d06 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 9 May 2017 14:18:35 -0400 Subject: [PATCH] dgram: convert to using internal/errors Covert lib/dgram.js over to using lib/internal/errors.js for generating Errors. See [using-internal-errors.md](https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md) for more details. I have not addressed the cases that use errnoException() and exceptionWithHostPort() helper methods as changing these would require fixing the tests across all of the different files that use them. In addition, these helpers already add a `code` to the Error and we'll have to discuss how that interacts with the `code` used by lib/internal/errors.js. I believe we should convert all users of errnoException and exceptionWithHostPort in a PR dedicated to that conversion. --- doc/api/errors.md | 29 +++++++++ lib/dgram.js | 64 +++++++++++++------ lib/internal/errors.js | 6 ++ test/parallel/test-dgram-bind.js | 6 +- test/parallel/test-dgram-createSocket-type.js | 8 ++- .../test-dgram-implicit-bind-failure.js | 2 +- test/parallel/test-dgram-membership.js | 36 ++++++++--- test/parallel/test-dgram-multicast-setTTL.js | 6 +- .../parallel/test-dgram-send-address-types.js | 13 ++-- test/parallel/test-dgram-sendto.js | 38 +++++++++-- test/parallel/test-dgram-setTTL.js | 6 +- 11 files changed, 167 insertions(+), 47 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index ebafd5a05fba54..40b8e7ab74cf71 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -703,6 +703,35 @@ in some cases may accept `func(undefined)` but not `func()`). In most native Node.js APIs, `func(undefined)` and `func()` are treated identically, and the [`ERR_INVALID_ARG_TYPE`][] error code may be used instead. + +### ERR_SOCKET_ALREADY_BOUND +An error using the `'ERR_SOCKET_ALREADY_BOUND'` code is thrown when an attempt +is made to bind a socket that has already been bound. + + +### ERR_SOCKET_BAD_PORT + +An error using the `'ERR_SOCKET_BAD_PORT'` code is thrown when an API +function expecting a port > 0 and < 65536 receives an invalid value. + + +### ERR_SOCKET_BAD_TYPE + +An error using the `'ERR_SOCKET_BAD_TYPE'` code is thrown when an API +function expecting a socket type (`udp4` or `udp6`) receives an invalid value. + + +### ERR_SOCKET_CANNOT_SEND + +An error using the `'ERR_SOCKET_CANNOT_SEND'` code is thrown when data +cannot be sent on a socket. + + +### ERR_SOCKET_DGRAM_NOT_RUNNING + +An error using the `'ERR_SOCKET_DGRAM_NOT_RUNNING'` code is thrown +when a call is made and the UDP subsystem is not running. + ### ERR_STDERR_CLOSE diff --git a/lib/dgram.js b/lib/dgram.js index 4d9f7eb29fd97c..0d93ca28748e87 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -22,6 +22,7 @@ 'use strict'; const assert = require('assert'); +const errors = require('internal/errors'); const Buffer = require('buffer').Buffer; const util = require('util'); const EventEmitter = require('events'); @@ -78,7 +79,7 @@ function newHandle(type) { return handle; } - throw new Error('Bad socket type specified. Valid types are: udp4, udp6'); + throw new errors.Error('ERR_SOCKET_BAD_TYPE'); } @@ -162,7 +163,7 @@ Socket.prototype.bind = function(port_, address_ /*, callback*/) { this._healthCheck(); if (this._bindState !== BIND_STATE_UNBOUND) - throw new Error('Socket is already bound'); + throw new errors.Error('ERR_SOCKET_ALREADY_BOUND'); this._bindState = BIND_STATE_BINDING; @@ -260,11 +261,21 @@ Socket.prototype.sendto = function(buffer, port, address, callback) { - if (typeof offset !== 'number' || typeof length !== 'number') - throw new Error('Send takes "offset" and "length" as args 2 and 3'); + if (typeof offset !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'offset', 'number'); + } + + if (typeof length !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'length', 'number'); + } - if (typeof address !== 'string') - throw new Error(this.type + ' sockets must send to port, address'); + if (typeof port !== 'number') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'port', 'number'); + } + + if (typeof address !== 'string') { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'address', 'string'); + } this.send(buffer, offset, length, port, address, callback); }; @@ -274,8 +285,9 @@ function sliceBuffer(buffer, offset, length) { if (typeof buffer === 'string') { buffer = Buffer.from(buffer); } else if (!isUint8Array(buffer)) { - throw new TypeError('First argument must be a Buffer, ' + - 'Uint8Array or string'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'buffer', + ['Buffer', 'Uint8Array', 'string']); } offset = offset >>> 0; @@ -323,7 +335,7 @@ function onListenSuccess() { function onListenError(err) { this.removeListener('listening', onListenSuccess); this._queue = undefined; - this.emit('error', new Error('Unable to send data')); + this.emit('error', new errors.Error('ERR_SOCKET_CANNOT_SEND')); } @@ -366,18 +378,21 @@ Socket.prototype.send = function(buffer, if (typeof buffer === 'string') { list = [ Buffer.from(buffer) ]; } else if (!isUint8Array(buffer)) { - throw new TypeError('First argument must be a Buffer, ' + - 'Uint8Array or string'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'buffer', + ['Buffer', 'Uint8Array', 'string']); } else { list = [ buffer ]; } } else if (!(list = fixBufferList(buffer))) { - throw new TypeError('Buffer list arguments must be buffers or strings'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'buffer list arguments', + ['Buffer', 'string']); } port = port >>> 0; if (port === 0 || port > 65535) - throw new RangeError('Port should be > 0 and < 65536'); + throw new errors.RangeError('ERR_SOCKET_BAD_PORT'); // Normalize callback so it's either a function or undefined but not anything // else. @@ -388,8 +403,9 @@ Socket.prototype.send = function(buffer, callback = address; address = undefined; } else if (address && typeof address !== 'string') { - throw new TypeError('Invalid arguments: address must be a nonempty ' + - 'string or falsy'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'address', + ['string', 'falsy']); } this._healthCheck(); @@ -510,7 +526,9 @@ Socket.prototype.setBroadcast = function(arg) { Socket.prototype.setTTL = function(arg) { if (typeof arg !== 'number') { - throw new TypeError('Argument must be a number'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'arg', + 'number'); } var err = this._handle.setTTL(arg); @@ -524,7 +542,9 @@ Socket.prototype.setTTL = function(arg) { Socket.prototype.setMulticastTTL = function(arg) { if (typeof arg !== 'number') { - throw new TypeError('Argument must be a number'); + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', + 'arg', + 'number'); } var err = this._handle.setMulticastTTL(arg); @@ -551,7 +571,7 @@ Socket.prototype.addMembership = function(multicastAddress, this._healthCheck(); if (!multicastAddress) { - throw new Error('multicast address must be specified'); + throw new errors.TypeError('ERR_MISSING_ARGS', 'multicastAddress'); } var err = this._handle.addMembership(multicastAddress, interfaceAddress); @@ -566,7 +586,7 @@ Socket.prototype.dropMembership = function(multicastAddress, this._healthCheck(); if (!multicastAddress) { - throw new Error('multicast address must be specified'); + throw new errors.TypeError('ERR_MISSING_ARGS', 'multicastAddress'); } var err = this._handle.dropMembership(multicastAddress, interfaceAddress); @@ -577,8 +597,10 @@ Socket.prototype.dropMembership = function(multicastAddress, Socket.prototype._healthCheck = function() { - if (!this._handle) - throw new Error('Not running'); // error message from dgram_legacy.js + if (!this._handle) { + // Error message from dgram_legacy.js. + throw new errors.Error('ERR_SOCKET_DGRAM_NOT_RUNNING'); + } }; diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 50d2c20325f0d6..1ca0f59d793b41 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -142,6 +142,12 @@ 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_SOCKET_ALREADY_BOUND', 'Socket is already bound'); +E('ERR_SOCKET_BAD_TYPE', + 'Bad socket type specified. Valid types are: udp4, udp6'); +E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data'); +E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536'); +E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running'); // Add new errors from here... function invalidArgType(name, expected, actual) { diff --git a/test/parallel/test-dgram-bind.js b/test/parallel/test-dgram-bind.js index 25fd7408ffd17a..4e293a8021c160 100644 --- a/test/parallel/test-dgram-bind.js +++ b/test/parallel/test-dgram-bind.js @@ -29,7 +29,11 @@ const socket = dgram.createSocket('udp4'); socket.on('listening', common.mustCall(() => { assert.throws(() => { socket.bind(); - }, /^Error: Socket is already bound$/); + }, common.expectsError({ + code: 'ERR_SOCKET_ALREADY_BOUND', + type: Error, + message: /^Socket is already bound$/ + })); socket.close(); })); diff --git a/test/parallel/test-dgram-createSocket-type.js b/test/parallel/test-dgram-createSocket-type.js index 1d269ddeda26eb..e6b17251f82060 100644 --- a/test/parallel/test-dgram-createSocket-type.js +++ b/test/parallel/test-dgram-createSocket-type.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const dgram = require('dgram'); const invalidTypes = [ @@ -24,7 +24,11 @@ const validTypes = [ invalidTypes.forEach((invalidType) => { assert.throws(() => { dgram.createSocket(invalidType); - }, /Bad socket type specified/); + }, common.expectsError({ + code: 'ERR_SOCKET_BAD_TYPE', + type: Error, + message: /^Bad socket type specified\. Valid types are: udp4, udp6$/ + })); }); // Error must not be thrown with valid types diff --git a/test/parallel/test-dgram-implicit-bind-failure.js b/test/parallel/test-dgram-implicit-bind-failure.js index 626af418e533a9..2944c9aae72abe 100644 --- a/test/parallel/test-dgram-implicit-bind-failure.js +++ b/test/parallel/test-dgram-implicit-bind-failure.js @@ -31,7 +31,7 @@ socket.on('error', (err) => { return; } - if (/^Error: Unable to send data$/.test(err)) { + if (err.code === 'ERR_SOCKET_CANNOT_SEND') { // On error, the queue should be destroyed and this function should be // the only listener. sendFailures++; diff --git a/test/parallel/test-dgram-membership.js b/test/parallel/test-dgram-membership.js index 1543b9043f7738..586db5f3cfeb70 100644 --- a/test/parallel/test-dgram-membership.js +++ b/test/parallel/test-dgram-membership.js @@ -11,8 +11,13 @@ const setup = dgram.createSocket.bind(dgram, {type: 'udp4', reuseAddr: true}); { const socket = setup(); socket.close(common.mustCall(() => { - assert.throws(() => { socket.addMembership(multicastAddress); }, - /^Error: Not running$/); + assert.throws(() => { + socket.addMembership(multicastAddress); + }, common.expectsError({ + code: 'ERR_SOCKET_DGRAM_NOT_RUNNING', + type: Error, + message: /^Not running$/ + })); })); } @@ -20,24 +25,39 @@ const setup = dgram.createSocket.bind(dgram, {type: 'udp4', reuseAddr: true}); { const socket = setup(); socket.close(common.mustCall(() => { - assert.throws(() => { socket.dropMembership(multicastAddress); }, - /^Error: Not running$/); + assert.throws(() => { + socket.dropMembership(multicastAddress); + }, common.expectsError({ + code: 'ERR_SOCKET_DGRAM_NOT_RUNNING', + type: Error, + message: /^Not running$/ + })); })); } // addMembership() with no argument should throw { const socket = setup(); - assert.throws(() => { socket.addMembership(); }, - /^Error: multicast address must be specified$/); + assert.throws(() => { + socket.addMembership(); + }, common.expectsError({ + code: 'ERR_MISSING_ARGS', + type: TypeError, + message: /^The "multicastAddress" argument must be specified$/ + })); socket.close(); } // dropMembership() with no argument should throw { const socket = setup(); - assert.throws(() => { socket.dropMembership(); }, - /^Error: multicast address must be specified$/); + assert.throws(() => { + socket.dropMembership(); + }, common.expectsError({ + code: 'ERR_MISSING_ARGS', + type: TypeError, + message: /^The "multicastAddress" argument must be specified$/ + })); socket.close(); } diff --git a/test/parallel/test-dgram-multicast-setTTL.js b/test/parallel/test-dgram-multicast-setTTL.js index 37a2d1586318ef..b7d1e01b321ac7 100644 --- a/test/parallel/test-dgram-multicast-setTTL.js +++ b/test/parallel/test-dgram-multicast-setTTL.js @@ -37,7 +37,11 @@ socket.on('listening', common.mustCall(() => { assert.throws(() => { socket.setMulticastTTL('foo'); - }, /^TypeError: Argument must be a number$/); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "arg" argument must be of type number$/ + })); //close the socket socket.close(); diff --git a/test/parallel/test-dgram-send-address-types.js b/test/parallel/test-dgram-send-address-types.js index 6c5bf20e00db78..6b26c23a266558 100644 --- a/test/parallel/test-dgram-send-address-types.js +++ b/test/parallel/test-dgram-send-address-types.js @@ -10,8 +10,11 @@ const onMessage = common.mustCall((err, bytes) => { assert.strictEqual(bytes, buf.length); }, 6); -const expectedError = - /^TypeError: Invalid arguments: address must be a nonempty string or falsy$/; +const expectedError = { code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: + /^The "address" argument must be one of type string or falsy$/ +}; const client = dgram.createSocket('udp4').bind(0, () => { const port = client.address().port; @@ -37,17 +40,17 @@ const client = dgram.createSocket('udp4').bind(0, () => { // invalid address: object assert.throws(() => { client.send(buf, port, []); - }, expectedError); + }, common.expectsError(expectedError)); // invalid address: nonzero number assert.throws(() => { client.send(buf, port, 1); - }, expectedError); + }, common.expectsError(expectedError)); // invalid address: true assert.throws(() => { client.send(buf, port, true); - }, expectedError); + }, common.expectsError(expectedError)); }); client.unref(); diff --git a/test/parallel/test-dgram-sendto.js b/test/parallel/test-dgram-sendto.js index 350f488f12af0d..c922dc1039e732 100644 --- a/test/parallel/test-dgram-sendto.js +++ b/test/parallel/test-dgram-sendto.js @@ -1,24 +1,48 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const dgram = require('dgram'); const socket = dgram.createSocket('udp4'); -const errorMessage = - /^Error: Send takes "offset" and "length" as args 2 and 3$/; +const errorMessageOffset = + /^The "offset" argument must be of type number$/; assert.throws(() => { socket.sendto(); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: errorMessageOffset +})); assert.throws(() => { socket.sendto('buffer', 1, 'offset', 'port', 'address', 'cb'); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "length" argument must be of type number$/ +})); assert.throws(() => { socket.sendto('buffer', 'offset', 1, 'port', 'address', 'cb'); -}, errorMessage); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: errorMessageOffset +})); assert.throws(() => { socket.sendto('buffer', 1, 1, 10, false, 'cb'); -}, /^Error: udp4 sockets must send to port, address$/); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "address" argument must be of type string$/ +})); + +assert.throws(() => { + socket.sendto('buffer', 1, 1, false, 'address', 'cb'); +}, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "port" argument must be of type number$/ +})); diff --git a/test/parallel/test-dgram-setTTL.js b/test/parallel/test-dgram-setTTL.js index 7da3975ad4c3fd..c061fbc1870d9e 100644 --- a/test/parallel/test-dgram-setTTL.js +++ b/test/parallel/test-dgram-setTTL.js @@ -11,7 +11,11 @@ socket.on('listening', common.mustCall(() => { assert.throws(() => { socket.setTTL('foo'); - }, /^TypeError: Argument must be a number$/); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "arg" argument must be of type number$/ + })); // TTL must be a number from > 0 to < 256 assert.throws(() => {