Skip to content

Commit

Permalink
lib, test: fix server.listen error message
Browse files Browse the repository at this point in the history
Previously the error messages are mostly `[object Object]`
after the options get normalized. Use util.inspect to make
it more useful.

Refactor the listen option test, add precise
error message validation and a few more test cases.

PR-URL: nodejs#11693
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
  • Loading branch information
joyeecheung authored and jungx098 committed Mar 21, 2017
1 parent a922be0 commit 1cd2fec
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 46 deletions.
2 changes: 1 addition & 1 deletion lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -1416,7 +1416,7 @@ Server.prototype.listen = function() {
return this;
}

throw new Error('Invalid listen argument: ' + options);
throw new Error('Invalid listen argument: ' + util.inspect(options));
};

function lookupAndListen(self, port, address, backlog, exclusive) {
Expand Down
45 changes: 0 additions & 45 deletions test/parallel/test-net-listen-port-option.js

This file was deleted.

81 changes: 81 additions & 0 deletions test/parallel/test-net-server-listen-options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const net = require('net');
const util = require('util');

function close() { this.close(); }

function listenError(literals, ...values) {
let result = literals[0];
for (const [i, value] of values.entries()) {
const str = util.inspect(value);
// Need to escape special characters.
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
result += literals[i + 1];
}
return new RegExp(`Error: Invalid listen argument: ${result}`);
}

{
// Test listen()
net.createServer().listen().on('listening', common.mustCall(close));
// Test listen(cb)
net.createServer().listen(common.mustCall(close));
}

// Test listen(port, cb) and listen({port: port}, cb) combinations
const listenOnPort = [
(port, cb) => net.createServer().listen({port}, cb),
(port, cb) => net.createServer().listen(port, cb)
];

{
const portError = /^RangeError: "port" argument must be >= 0 and < 65536$/;
for (const listen of listenOnPort) {
// Arbitrary unused ports
listen('0', common.mustCall(close));
listen(0, common.mustCall(close));
listen(undefined, common.mustCall(close));
// Test invalid ports
assert.throws(() => listen(-1, common.mustNotCall()), portError);
assert.throws(() => listen(NaN, common.mustNotCall()), portError);
assert.throws(() => listen(123.456, common.mustNotCall()), portError);
assert.throws(() => listen(65536, common.mustNotCall()), portError);
assert.throws(() => listen(1 / 0, common.mustNotCall()), portError);
assert.throws(() => listen(-1 / 0, common.mustNotCall()), portError);
}
// In listen(options, cb), port takes precedence over path
assert.throws(() => {
net.createServer().listen({ port: -1, path: common.PIPE },
common.mustNotCall());
}, portError);
}

{
function shouldFailToListen(options, optionsInMessage) {
// Plain arguments get normalized into an object before
// formatted in message, options objects don't.
if (arguments.length === 1) {
optionsInMessage = options;
}
const block = () => {
net.createServer().listen(options, common.mustNotCall());
};
assert.throws(block, listenError`${optionsInMessage}`,
`expect listen(${util.inspect(options)}) to throw`);
}

shouldFailToListen(null, { port: null });
shouldFailToListen({ port: null });
shouldFailToListen(false, { port: false });
shouldFailToListen({ port: false });
shouldFailToListen(true, { port: true });
shouldFailToListen({ port: true });
// Invalid fd as listen(handle)
shouldFailToListen({ fd: -1 });
// Invalid path in listen(options)
shouldFailToListen({ path: -1 });
// Host without port
shouldFailToListen({ host: 'localhost' });
}

0 comments on commit 1cd2fec

Please sign in to comment.