Skip to content

Commit

Permalink
[fix] Make the {noS,s}erver, and port options mutually exclusive
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lpinca committed Jul 8, 2021
1 parent ecb9d9e commit 66e58d2
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 7 deletions.
4 changes: 2 additions & 2 deletions doc/ws.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions lib/websocket-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
}

Expand Down
38 changes: 35 additions & 3 deletions test/websocket-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 66e58d2

Please sign in to comment.