[major] Make WebSocket#p{i,o}ng() accept an optional callback

Fixes #599

Author: lpinca

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);
 ```

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}

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,26 +188,28 @@ 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);
   }
 
   /**
    * Sends a pong message to the other peer.
    *
    * @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);
   }
 
   /**

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();

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));
         });
       });