Skip to content

Commit

Permalink
dgram: convert to using internal/errors
Browse files Browse the repository at this point in the history
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.

PR-URL: #12926
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben.bridgewater@fintura.de>
  • Loading branch information
mhdawson authored and jasnell committed May 24, 2017
1 parent f21ae31 commit d6e20cb
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 47 deletions.
29 changes: 29 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a id="ERR_SOCKET_ALREADY_BOUND"></a>
### 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.

<a id="ERR_SOCKET_BAD_PORT"></a>
### 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.

<a id="ERR_SOCKET_BAD_TYPE"></a>
### 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.

<a id="ERR_SOCKET_CANNOT_SEND"></a>
### ERR_SOCKET_CANNOT_SEND

An error using the `'ERR_SOCKET_CANNOT_SEND'` code is thrown when data
cannot be sent on a socket.

<a id="ERR_SOCKET_DGRAM_NOT_RUNNING"></a>
### 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.

<a id="ERR_STDERR_CLOSE"></a>
### ERR_STDERR_CLOSE

Expand Down
64 changes: 43 additions & 21 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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');
}


Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
};
Expand All @@ -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;
Expand Down Expand Up @@ -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'));
}


Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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');
}
};


Expand Down
6 changes: 6 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-dgram-bind.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));
Expand Down
8 changes: 6 additions & 2 deletions test/parallel/test-dgram-createSocket-type.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const dgram = require('dgram');
const invalidTypes = [
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-dgram-implicit-bind-failure.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand Down
36 changes: 28 additions & 8 deletions test/parallel/test-dgram-membership.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,53 @@ 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$/
}));
}));
}

// dropMembership() on closed socket should throw
{
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();
}

Expand Down
6 changes: 5 additions & 1 deletion test/parallel/test-dgram-multicast-setTTL.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
13 changes: 8 additions & 5 deletions test/parallel/test-dgram-send-address-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Loading

0 comments on commit d6e20cb

Please sign in to comment.