From 47759429578952672ef9d1079412ed56aad77c4d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 4 Mar 2017 15:03:15 +0800 Subject: [PATCH] lib, test: fix server.listen error message 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: https://github.com/nodejs/node/pull/11693 Reviewed-By: James M Snell Reviewed-By: Franziska Hinkelmann --- lib/net.js | 2 +- test/parallel/test-net-listen-port-option.js | 45 ----------- .../test-net-server-listen-options.js | 81 +++++++++++++++++++ 3 files changed, 82 insertions(+), 46 deletions(-) delete mode 100644 test/parallel/test-net-listen-port-option.js create mode 100644 test/parallel/test-net-server-listen-options.js diff --git a/lib/net.js b/lib/net.js index 64e50e907719d5..2231e5bb32db0f 100644 --- a/lib/net.js +++ b/lib/net.js @@ -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) { diff --git a/test/parallel/test-net-listen-port-option.js b/test/parallel/test-net-listen-port-option.js deleted file mode 100644 index 693bf97907aeee..00000000000000 --- a/test/parallel/test-net-listen-port-option.js +++ /dev/null @@ -1,45 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const net = require('net'); - -function close() { this.close(); } - -// From lib/net.js -function toNumber(x) { return (x = Number(x)) >= 0 ? x : false; } - -function isPipeName(s) { - return typeof s === 'string' && toNumber(s) === false; -} - -const listenVariants = [ - (port, cb) => net.Server().listen({port}, cb), - (port, cb) => net.Server().listen(port, cb) -]; - -listenVariants.forEach((listenVariant, i) => { - listenVariant(undefined, common.mustCall(close)); - listenVariant('0', common.mustCall(close)); - - [ - 'nan', - -1, - 123.456, - 0x10000, - 1 / 0, - -1 / 0, - '+Infinity', - '-Infinity' - ].forEach((port) => { - if (i === 1 && isPipeName(port)) { - // skip this, because listen(port) can also be listen(path) - return; - } - assert.throws(() => listenVariant(port, common.mustNotCall()), - /"port" argument must be >= 0 and < 65536/i); - }); - - [null, true, false].forEach((port) => - assert.throws(() => listenVariant(port, common.mustNotCall()), - /invalid listen argument/i)); -}); diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js new file mode 100644 index 00000000000000..ed1d0dc894d260 --- /dev/null +++ b/test/parallel/test-net-server-listen-options.js @@ -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' }); +}