Skip to content

Commit

Permalink
[minor] Throw an error on invalid usage
Browse files Browse the repository at this point in the history
Throw an error if the same `server` option is used for multiple
WebSocket servers at the same time.
  • Loading branch information
lpinca committed Jul 14, 2019
1 parent dd42c8b commit 3641266
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 7 deletions.
11 changes: 11 additions & 0 deletions lib/websocket-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const { format, parse } = require('./extension');
const { GUID } = require('./constants');

const keyRegex = /^[+/0-9A-Za-z]{22}==$/;
const kUsedByWebSocketServer = Symbol('kUsedByWebSocketServer');

/**
* Class representing a WebSocket server.
Expand Down Expand Up @@ -82,6 +83,14 @@ class WebSocketServer extends EventEmitter {
}

if (this._server) {
if (this._server[kUsedByWebSocketServer]) {
throw new Error(
'The HTTP/S server is already being used by another WebSocket server'
);
} else {
this._server[kUsedByWebSocketServer] = true;
}

this._removeListeners = addListeners(this._server, {
listening: this.emit.bind(this, 'listening'),
error: this.emit.bind(this, 'error'),
Expand Down Expand Up @@ -135,6 +144,8 @@ class WebSocketServer extends EventEmitter {
const server = this._server;

if (server) {
server[kUsedByWebSocketServer] = false;

this._removeListeners();
this._removeListeners = this._server = null;

Expand Down
31 changes: 24 additions & 7 deletions test/websocket-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,22 @@ describe('WebSocketServer', () => {
assert.throws(() => new WebSocket.Server());
});

it('throws an error if no port or server is specified', () => {
assert.throws(() => new WebSocket.Server({}));
});

describe('options', () => {
it('throws an error if no `port` or `server` option is specified', () => {
assert.throws(() => new WebSocket.Server({}));
});

it('throws an error if the server is already being used', () => {
const server = http.createServer();

new WebSocket.Server({ server });

assert.throws(
() => new WebSocket.Server({ server }),
/^Error: The HTTP\/S server is already being used by another WebSocket server$/
);
});

it('exposes options passed to constructor', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
assert.strictEqual(wss.options.port, 0);
Expand Down Expand Up @@ -225,15 +236,21 @@ describe('WebSocketServer', () => {

it('cleans event handlers on precreated server', (done) => {
const server = http.createServer();
const wss = new WebSocket.Server({ server });
const wss1 = new WebSocket.Server({ server });

server.listen(0, () => {
wss.close(() => {
wss1.close(() => {
assert.strictEqual(server.listenerCount('listening'), 0);
assert.strictEqual(server.listenerCount('upgrade'), 0);
assert.strictEqual(server.listenerCount('error'), 0);

server.close(done);
// Check that no error is thrown if `server` is resued now as there
// are no other `WebSocketServer`s using it.
const wss2 = new WebSocket.Server({ server });

wss2.close(() => {
server.close(done);
});
});
});
});
Expand Down

0 comments on commit 3641266

Please sign in to comment.