From 66e58d279ffabe5108424c08ab71403aceddcad9 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Thu, 8 Jul 2021 13:55:29 +0200 Subject: [PATCH] [fix] Make the `{noS,s}erver`, and `port` options mutually exclusive Remove ambiguity and prevent `WebSocketServer.prototype.address()` from throwing an error if the `noServer` option is used along with the `port` and/or `server` options. --- doc/ws.md | 4 ++-- lib/websocket-server.js | 9 +++++++-- test/websocket-server.test.js | 38 ++++++++++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/doc/ws.md b/doc/ws.md index af0dfb325..a3b1bff81 100644 --- a/doc/ws.md +++ b/doc/ws.md @@ -80,8 +80,8 @@ This class represents a WebSocket server. It extends the `EventEmitter`. - `maxPayload` {Number} The maximum allowed message size in bytes. - `callback` {Function} -Create a new server instance. One of `port`, `server` or `noServer` must be -provided or an error is thrown. An HTTP server is automatically created, +Create a new server instance. One and only one of `port`, `server` or `noServer` +must be provided or an error is thrown. An HTTP server is automatically created, started, and used if `port` is set. To use an external HTTP/S server instead, specify only `server` or `noServer`. In this case the HTTP/S server must be started manually. The "noServer" mode allows the WebSocket server to be diff --git a/lib/websocket-server.js b/lib/websocket-server.js index 1610e767a..20276edb3 100644 --- a/lib/websocket-server.js +++ b/lib/websocket-server.js @@ -62,9 +62,14 @@ class WebSocketServer extends EventEmitter { ...options }; - if (options.port == null && !options.server && !options.noServer) { + if ( + (options.port == null && !options.server && !options.noServer) || + (options.port != null && (options.server || options.noServer)) || + (options.server && options.noServer) + ) { throw new TypeError( - 'One of the "port", "server", or "noServer" options must be specified' + 'One and only one of the "port", "server", or "noServer" options ' + + 'must be specified' ); } diff --git a/test/websocket-server.test.js b/test/websocket-server.test.js index ce6617ec6..9b43284a1 100644 --- a/test/websocket-server.test.js +++ b/test/websocket-server.test.js @@ -18,12 +18,44 @@ const { NOOP } = require('../lib/constants'); describe('WebSocketServer', () => { describe('#ctor', () => { it('throws an error if no option object is passed', () => { - assert.throws(() => new WebSocket.Server()); + assert.throws( + () => new WebSocket.Server(), + new RegExp( + '^TypeError: One and only one of the "port", "server", or ' + + '"noServer" options must be specified$' + ) + ); }); describe('options', () => { - it('throws an error if no `port` or `server` option is specified', () => { - assert.throws(() => new WebSocket.Server({})); + it('throws an error if required options are not specified', () => { + assert.throws( + () => new WebSocket.Server({}), + new RegExp( + '^TypeError: One and only one of the "port", "server", or ' + + '"noServer" options must be specified$' + ) + ); + }); + + it('throws an error if mutually exclusive options are specified', () => { + const server = http.createServer(); + const variants = [ + { port: 0, noServer: true, server }, + { port: 0, noServer: true }, + { port: 0, server }, + { noServer: true, server } + ]; + + for (const options of variants) { + assert.throws( + () => new WebSocket.Server(options), + new RegExp( + '^TypeError: One and only one of the "port", "server", or ' + + '"noServer" options must be specified$' + ) + ); + } }); it('exposes options passed to constructor', (done) => {