From 30c9f715247b2f5ad0c691e535708f894d82ed2b Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sun, 24 Dec 2017 10:11:49 +0100 Subject: [PATCH] [major] Make `WebSocket#p{i,o}ng()` accept an optional callback Fixes #599 --- README.md | 4 ++- doc/ws.md | 14 ++++----- lib/sender.js | 24 ++++++++------- lib/websocket.js | 55 +++++++++++++++++++++++------------ test/websocket.test.js | 66 +++++++++++++++++++++++++----------------- 5 files changed, 101 insertions(+), 62 deletions(-) diff --git a/README.md b/README.md index e2cc3bd38..12f06c16c 100644 --- a/README.md +++ b/README.md @@ -298,6 +298,8 @@ const WebSocket = require('ws'); const wss = new WebSocket.Server({ port: 8080 }); +function noop() {} + function heartbeat() { this.isAlive = true; } @@ -312,7 +314,7 @@ const interval = setInterval(function ping() { if (ws.isAlive === false) return ws.terminate(); ws.isAlive = false; - ws.ping('', false, true); + ws.ping(noop); }); }, 30000); ``` diff --git a/doc/ws.md b/doc/ws.md index b2962b2ff..c9cb973eb 100644 --- a/doc/ws.md +++ b/doc/ws.md @@ -355,23 +355,23 @@ receives an `OpenEvent` named "open". Pause the socket. -### websocket.ping([data[, mask[, failSilently]]]) +### websocket.ping([data[, mask]][, callback]) - `data` {Any} The data to send in the ping frame. - `mask` {Boolean} Specifies whether `data` should be masked or not. Defaults to `true` when `websocket` is not a server client. -- `failSilently` {Boolean} Specifies whether or not to throw an error if the - connection is not open. +- `callback` {Function} An optional callback which is invoked when the ping + frame is written out. Send a ping. -### websocket.pong([data[, mask[, failSilently]]]) +### websocket.pong([data[, mask]][, callback]) - `data` {Any} The data to send in the pong frame. - `mask` {Boolean} Specifies whether `data` should be masked or not. Defaults to `true` when `websocket` is not a server client. -- `failSilently` {Boolean} Specifies whether or not to throw an error if the - connection is not open. +- `callback` {Function} An optional callback which is invoked when the pong + frame is written out. Send a pong. @@ -404,7 +404,7 @@ Removes an event listener emulating the `EventTarget` interface. Resume the socket. -### websocket.send(data, [options][, callback]) +### websocket.send(data[, options][, callback]) - `data` {Any} The data to send. - `options` {Object} diff --git a/lib/sender.js b/lib/sender.js index 4526d2204..61d73df30 100644 --- a/lib/sender.js +++ b/lib/sender.js @@ -158,9 +158,10 @@ class Sender { * * @param {*} data The message to send * @param {Boolean} mask Specifies whether or not to mask `data` + * @param {Function} cb Callback * @public */ - ping (data, mask) { + ping (data, mask, cb) { var readOnly = true; if (!Buffer.isBuffer(data)) { @@ -175,9 +176,9 @@ class Sender { } if (this._deflating) { - this.enqueue([this.doPing, data, mask, readOnly]); + this.enqueue([this.doPing, data, mask, readOnly, cb]); } else { - this.doPing(data, mask, readOnly); + this.doPing(data, mask, readOnly, cb); } } @@ -187,16 +188,17 @@ class Sender { * @param {*} data The message to send * @param {Boolean} mask Specifies whether or not to mask `data` * @param {Boolean} readOnly Specifies whether `data` can be modified + * @param {Function} cb Callback * @private */ - doPing (data, mask, readOnly) { + doPing (data, mask, readOnly, cb) { this.sendFrame(Sender.frame(data, { fin: true, rsv1: false, opcode: 0x09, mask, readOnly - })); + }), cb); } /** @@ -204,9 +206,10 @@ class Sender { * * @param {*} data The message to send * @param {Boolean} mask Specifies whether or not to mask `data` + * @param {Function} cb Callback * @public */ - pong (data, mask) { + pong (data, mask, cb) { var readOnly = true; if (!Buffer.isBuffer(data)) { @@ -221,9 +224,9 @@ class Sender { } if (this._deflating) { - this.enqueue([this.doPong, data, mask, readOnly]); + this.enqueue([this.doPong, data, mask, readOnly, cb]); } else { - this.doPong(data, mask, readOnly); + this.doPong(data, mask, readOnly, cb); } } @@ -233,16 +236,17 @@ class Sender { * @param {*} data The message to send * @param {Boolean} mask Specifies whether or not to mask `data` * @param {Boolean} readOnly Specifies whether `data` can be modified + * @param {Function} cb Callback * @private */ - doPong (data, mask, readOnly) { + doPong (data, mask, readOnly, cb) { this.sendFrame(Sender.frame(data, { fin: true, rsv1: false, opcode: 0x0a, mask, readOnly - })); + }), cb); } /** diff --git a/lib/websocket.js b/lib/websocket.js index 31f4ba7a6..1a38497c9 100644 --- a/lib/websocket.js +++ b/lib/websocket.js @@ -141,7 +141,7 @@ class WebSocket extends EventEmitter { this._receiver.onmessage = (data) => this.emit('message', data); this._receiver.onping = (data) => { - this.pong(data, !this._isServer, true); + this.pong(data, !this._isServer, constants.NOOP); this.emit('ping', data); }; this._receiver.onpong = (data) => this.emit('pong', data); @@ -317,47 +317,67 @@ class WebSocket extends EventEmitter { } /** - * Send a ping message. + * Send a ping. * - * @param {*} data The message to send + * @param {*} data The data to send * @param {Boolean} mask Indicates whether or not to mask `data` - * @param {Boolean} failSilently Indicates whether or not to throw if `readyState` isn't `OPEN` + * @param {Function} cb Callback which is executed when the ping is sent * @public */ - ping (data, mask, failSilently) { + ping (data, mask, cb) { + if (typeof data === 'function') { + cb = data; + data = mask = undefined; + } else if (typeof mask === 'function') { + cb = mask; + mask = undefined; + } + if (this.readyState !== WebSocket.OPEN) { - if (failSilently) return; - throw new Error( + const err = new Error( `WebSocket is not open: readyState ${this.readyState} ` + `(${readyStates[this.readyState]})` ); + + if (cb) return cb(err); + throw err; } if (typeof data === 'number') data = data.toString(); if (mask === undefined) mask = !this._isServer; - this._sender.ping(data || constants.EMPTY_BUFFER, mask); + this._sender.ping(data || constants.EMPTY_BUFFER, mask, cb); } /** - * Send a pong message. + * Send a pong. * - * @param {*} data The message to send + * @param {*} data The data to send * @param {Boolean} mask Indicates whether or not to mask `data` - * @param {Boolean} failSilently Indicates whether or not to throw if `readyState` isn't `OPEN` + * @param {Function} cb Callback which is executed when the pong is sent * @public */ - pong (data, mask, failSilently) { + pong (data, mask, cb) { + if (typeof data === 'function') { + cb = data; + data = mask = undefined; + } else if (typeof mask === 'function') { + cb = mask; + mask = undefined; + } + if (this.readyState !== WebSocket.OPEN) { - if (failSilently) return; - throw new Error( + const err = new Error( `WebSocket is not open: readyState ${this.readyState} ` + `(${readyStates[this.readyState]})` ); + + if (cb) return cb(err); + throw err; } if (typeof data === 'number') data = data.toString(); if (mask === undefined) mask = !this._isServer; - this._sender.pong(data || constants.EMPTY_BUFFER, mask); + this._sender.pong(data || constants.EMPTY_BUFFER, mask, cb); } /** @@ -384,9 +404,8 @@ class WebSocket extends EventEmitter { `(${readyStates[this.readyState]})` ); - if (cb) cb(err); - else throw err; - return; + if (cb) return cb(err); + throw err; } if (typeof data === 'number') data = data.toString(); diff --git a/test/websocket.test.js b/test/websocket.test.js index 634340349..30ad6e9e6 100644 --- a/test/websocket.test.js +++ b/test/websocket.test.js @@ -663,7 +663,7 @@ describe('WebSocket', function () { }); describe('#ping', function () { - it('before connect should fail', function () { + it('throws an error if `readyState` is not `OPEN`', function (done) { const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() }); @@ -672,37 +672,44 @@ describe('WebSocket', function () { () => ws.ping(), /^Error: WebSocket is not open: readyState 0 \(CONNECTING\)$/ ); - }); - it('before connect can silently fail', function () { - const ws = new WebSocket('ws://localhost', { - agent: new CustomAgent() + ws.ping((err) => { + assert.ok(err instanceof Error); + assert.strictEqual( + err.message, + 'WebSocket is not open: readyState 0 (CONNECTING)' + ); + done(); }); - - assert.doesNotThrow(() => ws.ping('', true, true)); }); - it('without message is successfully transmitted to the server', function (done) { + it('can send a ping with no data', function (done) { const wss = new WebSocket.Server({ port: 0 }, () => { const port = wss._server.address().port; const ws = new WebSocket(`ws://localhost:${port}`); - ws.on('open', () => ws.ping()); + ws.on('open', () => { + ws.ping(() => ws.ping()); + }); }); wss.on('connection', (ws) => { - ws.on('ping', () => wss.close(done)); + let pings = 0; + ws.on('ping', (data) => { + assert.ok(Buffer.isBuffer(data)); + assert.strictEqual(data.length, 0); + if (++pings === 2) wss.close(done); + }); }); }); - it('with message is successfully transmitted to the server', function (done) { + it('can send a ping with data', function (done) { const wss = new WebSocket.Server({ port: 0 }, () => { const port = wss._server.address().port; const ws = new WebSocket(`ws://localhost:${port}`); ws.on('open', () => { - ws.ping('hi', true); - ws.ping('hi'); + ws.ping('hi', () => ws.ping('hi', true)); }); }); @@ -733,7 +740,7 @@ describe('WebSocket', function () { }); describe('#pong', function () { - it('before connect should fail', () => { + it('throws an error if `readyState` is not `OPEN`', (done) => { const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() }); @@ -742,37 +749,44 @@ describe('WebSocket', function () { () => ws.pong(), /^Error: WebSocket is not open: readyState 0 \(CONNECTING\)$/ ); - }); - it('before connect can silently fail', function () { - const ws = new WebSocket('ws://localhost', { - agent: new CustomAgent() + ws.pong((err) => { + assert.ok(err instanceof Error); + assert.strictEqual( + err.message, + 'WebSocket is not open: readyState 0 (CONNECTING)' + ); + done(); }); - - assert.doesNotThrow(() => ws.pong('', true, true)); }); - it('without message is successfully transmitted to the server', function (done) { + it('can send a pong with no data', function (done) { const wss = new WebSocket.Server({ port: 0 }, () => { const port = wss._server.address().port; const ws = new WebSocket(`ws://localhost:${port}`); - ws.on('open', () => ws.pong()); + ws.on('open', () => { + ws.pong(() => ws.pong()); + }); }); wss.on('connection', (ws) => { - ws.on('pong', () => wss.close(done)); + let pongs = 0; + ws.on('pong', (data) => { + assert.ok(Buffer.isBuffer(data)); + assert.strictEqual(data.length, 0); + if (++pongs === 2) wss.close(done); + }); }); }); - it('with message is successfully transmitted to the server', function (done) { + it('can send a pong with data', function (done) { const wss = new WebSocket.Server({ port: 0 }, () => { const port = wss._server.address().port; const ws = new WebSocket(`ws://localhost:${port}`); ws.on('open', () => { - ws.pong('hi', true); - ws.pong('hi'); + ws.pong('hi', () => ws.pong('hi', true)); }); });