[fix] Don't call ws.terminate() unconditionally in duplex._destroy()

Call `ws.terminate()` only if `duplex.destroy()` is called directly by
the user and not indirectly by the listener of the `'error'` event of
the `WebSocket` object. Calling `ws.terminate()` right after the
`'error'` event is emitted on the `WebSocket` object, might prevent the
close frame from being sent to the other peer.

Author: lpinca

lib/stream.js

@@ -48,6 +48,7 @@ function duplexOnError(err) {
  */
 function createWebSocketStream(ws, options) {
   let resumeOnReceiverDrain = true;
+  let terminateOnDestroy = true;
 
   function receiverOnDrain() {
     if (resumeOnReceiverDrain) ws._socket.resume();
@@ -81,6 +82,16 @@ function createWebSocketStream(ws, options) {
   ws.once('error', function error(err) {
     if (duplex.destroyed) return;
 
+    // Prevent `ws.terminate()` from being called by `duplex._destroy()`.
+    //
+    // - If the state of the `WebSocket` connection is `CONNECTING`,
+    //   `ws.terminate()` is a noop as no socket was assigned.
+    // - Otherwise, the error was re-emitted from the listener of the `'error'`
+    //   event of the `Receiver` object. The listener already closes the
+    //   connection by calling `ws.close()`. This allows a close frame to be
+    //   sent to the other peer. If `ws.terminate()` is called right after this,
+    //   the close frame might not be sent.
+    terminateOnDestroy = false;
     duplex.destroy(err);
   });
 
@@ -108,7 +119,8 @@ function createWebSocketStream(ws, options) {
       if (!called) callback(err);
       process.nextTick(emitClose, duplex);
     });
-    ws.terminate();
+
+    if (terminateOnDestroy) ws.terminate();
   };
 
   duplex._final = function (callback) {

test/create-websocket-stream.test.js

@@ -203,6 +203,7 @@ describe('createWebSocketStream', () => {
     });
 
     it('reemits errors', (done) => {
+      let duplexCloseEventEmitted = false;
       const wss = new WebSocket.Server({ port: 0 }, () => {
         const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
         const duplex = createWebSocketStream(ws);
@@ -215,13 +216,19 @@ describe('createWebSocketStream', () => {
           );
 
           duplex.on('close', () => {
-            wss.close(done);
+            duplexCloseEventEmitted = true;
           });
         });
       });
 
       wss.on('connection', (ws) => {
         ws._socket.write(Buffer.from([0x85, 0x00]));
+        ws.on('close', (code, reason) => {
+          assert.ok(duplexCloseEventEmitted);
+          assert.strictEqual(code, 1002);
+          assert.strictEqual(reason, '');
+          wss.close(done);
+        });
       });
     });