Repository: websockets/ws
Pull request #939 (2016-12-20 14:31:05)

Use the same handler for binary and text messages
This merges `receiver.onbinary` and `receiver.ontext` into `receiver.onmessage`.
I think thank using 2 different handlers is pointless.

This is tagged as "major" because it changes the `Receiver` interface but I don't think many people are  using the `Receiver` and `Sender` classes directly. Their interface is not documented and imo they should be private and not used directly. Unfortunately they are exposed as static properties of the `WebSocket` class.

I was also thinking that maybe it makes sense to emit to the `WebSocket` instance directly instead of using these wrapper [functions](https://github.com/websockets/ws/blob/fc956f8071e3930de432676ea2ea9cc075ee9fde/lib/WebSocket.js#L139).

For example we could make the `Receiver` constructor accept a `WebSocket` instance, the one the receiver is bound to.

```js
class Receiver {
  constructor (websocket, extensions, maxPayload) {
    this.websocket = websocket;
    ...
  }
}
```

and then instead of calling the wrapper functions like [this](https://github.com/websockets/ws/blob/fc956f8071e3930de432676ea2ea9cc075ee9fde/lib/Receiver.js#L353), emit the event directly on the `websocket` object.

```js
this.websocket.emit('message', buf, { binary: true, masked: this.masked });
```


Patch:
diff --git a/lib/Receiver.js b/lib/Receiver.js
index 62d68c36..07cbb9b6 100644
--- a/lib/Receiver.js
+++ b/lib/Receiver.js
@@ -53,10 +53,9 @@ class Receiver {
 
     this.dead = false;
 
-    this.onbinary = noop;
+    this.onmessage = noop;
     this.onclose = noop;
     this.onerror = noop;
-    this.ontext = noop;
     this.onping = noop;
     this.onpong = noop;
 
@@ -350,14 +349,14 @@ class Receiver {
       this.fragmented = 0;
 
       if (this.opcode === 2) {
-        this.onbinary(buf, { masked: this.masked });
+        this.onmessage(buf, { masked: this.masked, binary: true });
       } else {
         if (!Validation.isValidUTF8(buf)) {
           this.error(new Error('invalid utf8 sequence'), 1007);
           return;
         }
 
-        this.ontext(buf.toString(), { masked: this.masked });
+        this.onmessage(buf.toString(), { masked: this.masked });
       }
     }
 
@@ -477,10 +476,9 @@ class Receiver {
     this.buffers = null;
     this.mask = null;
 
-    this.onbinary = null;
+    this.onmessage = null;
     this.onclose = null;
     this.onerror = null;
-    this.ontext = null;
     this.onping = null;
     this.onpong = null;
   }
diff --git a/lib/WebSocket.js b/lib/WebSocket.js
index c1061bc8..98b5eeec 100644
--- a/lib/WebSocket.js
+++ b/lib/WebSocket.js
@@ -136,11 +136,7 @@ class WebSocket extends EventEmitter {
     });
 
     // receiver event handlers
-    this._receiver.ontext = (data, flags) => this.emit('message', data, flags);
-    this._receiver.onbinary = (data, flags) => {
-      flags.binary = true;
-      this.emit('message', data, flags);
-    };
+    this._receiver.onmessage = (data, flags) => this.emit('message', data, flags);
     this._receiver.onping = (data, flags) => {
       this.pong(data, { mask: !this._isServer }, true);
       this.emit('ping', data, flags);
@@ -151,9 +147,9 @@ class WebSocket extends EventEmitter {
       this._closeCode = code;
       this.close(code, reason);
     };
-    this._receiver.onerror = (error, errorCode) => {
+    this._receiver.onerror = (error, code) => {
       // close the connection when the receiver reports a HyBi error code
-      this.close(errorCode, '');
+      this.close(code, '');
       this.emit('error', error);
     };
 


Test patch:
diff --git a/test/Receiver.test.js b/test/Receiver.test.js
index d54cbb65..c139b5f3 100644
--- a/test/Receiver.test.js
+++ b/test/Receiver.test.js
@@ -17,7 +17,7 @@ describe('Receiver', function () {
   it('can parse unmasked text message', function (done) {
     const p = new Receiver();
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, 'Hello');
       done();
     };
@@ -40,7 +40,7 @@ describe('Receiver', function () {
   it('can parse masked text message', function (done) {
     const p = new Receiver();
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, '5:::{"name":"echo"}');
       done();
     };
@@ -56,7 +56,7 @@ describe('Receiver', function () {
     const frame = Buffer.from('81FE' + util.pack(4, msg.length) + mask +
       util.mask(msg, mask).toString('hex'), 'hex');
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, msg);
       done();
     };
@@ -73,7 +73,7 @@ describe('Receiver', function () {
     const frame = '81FF' + util.pack(16, msg.length) + mask +
       util.mask(msg, mask).toString('hex');
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, msg);
       done();
     };
@@ -94,7 +94,7 @@ describe('Receiver', function () {
     const frame2 = '80FE' + util.pack(4, fragment2.length) + mask +
       util.mask(fragment2, mask).toString('hex');
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, msg);
       done();
     };
@@ -148,7 +148,7 @@ describe('Receiver', function () {
 
     let gotPing = false;
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, msg);
       assert.ok(gotPing);
       done();
@@ -187,7 +187,7 @@ describe('Receiver', function () {
 
     let gotPing = false;
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, msg);
       assert.ok(gotPing);
       done();
@@ -210,8 +210,8 @@ describe('Receiver', function () {
     const frame = '82' + util.getHybiLengthAsHexString(msg.length, true) + mask +
       util.mask(msg, mask).toString('hex');
 
-    p.onbinary = function (data) {
-      assert.deepStrictEqual(data.toString('hex'), msg.toString('hex'));
+    p.onmessage = function (data) {
+      assert.ok(data.equals(msg));
       done();
     };
 
@@ -226,8 +226,8 @@ describe('Receiver', function () {
     const frame = '82' + util.getHybiLengthAsHexString(msg.length, true) + mask +
       util.mask(msg, mask).toString('hex');
 
-    p.onbinary = function (data) {
-      assert.deepStrictEqual(data, msg);
+    p.onmessage = function (data) {
+      assert.ok(data.equals(msg));
       done();
     };
 
@@ -242,8 +242,8 @@ describe('Receiver', function () {
     const frame = '82' + util.getHybiLengthAsHexString(msg.length, true) + mask +
       util.mask(msg, mask).toString('hex');
 
-    p.onbinary = function (data) {
-      assert.deepStrictEqual(data, msg);
+    p.onmessage = function (data) {
+      assert.ok(data.equals(msg));
       done();
     };
 
@@ -257,8 +257,8 @@ describe('Receiver', function () {
     const frame = '82' + util.getHybiLengthAsHexString(msg.length, false) +
       msg.toString('hex');
 
-    p.onbinary = function (data) {
-      assert.deepStrictEqual(data, msg);
+    p.onmessage = function (data) {
+      assert.ok(data.equals(msg));
       done();
     };
 
@@ -272,7 +272,7 @@ describe('Receiver', function () {
     const p = new Receiver({ 'permessage-deflate': perMessageDeflate });
     const buf = Buffer.from('Hello');
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, 'Hello');
       done();
     };
@@ -293,7 +293,7 @@ describe('Receiver', function () {
     const buf1 = Buffer.from('foo');
     const buf2 = Buffer.from('bar');
 
-    p.ontext = function (data) {
+    p.onmessage = function (data) {
       assert.strictEqual(data, 'foobar');
       done();
     };
diff --git a/test/testserver.js b/test/testserver.js
index 48e1e287..8214650e 100644
--- a/test/testserver.js
+++ b/test/testserver.js
@@ -62,14 +62,9 @@ function validServer (server, req, socket) {
 
   receiver.onping = (message, flags) => server.emit('ping', message, flags);
   receiver.onpong = (message, flags) => server.emit('pong', message, flags);
-  receiver.ontext = (message, flags) => {
+  receiver.onmessage = (message, flags) => {
     server.emit('message', message, flags);
-    sender.send(message, { fin: true });
-  };
-  receiver.onbinary = (message, flags) => {
-    flags.binary = true;
-    server.emit('message', message, flags);
-    sender.send(message, { binary: true, fin: true });
+    sender.send(message, { binary: flags.binary, fin: true });
   };
   receiver.onclose = (code, message, flags) => {
     sender.close(code, message, false, () => {