fix: handle websocket closed correctly

lib/web/websocket/connection.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const { uid } = require('./constants')
+const { uid, states } = require('./constants')
 const { failWebsocketConnection, parseExtensions } = require('./util')
 const { channels } = require('../../core/diagnostics')
 const { makeRequest } = require('../fetch/request')
@@ -94,10 +94,16 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
     useParallelQueue: true,
     dispatcher: options.dispatcher,
     processResponse (response) {
+      if (response.type === 'error') {
+        // If the WebSocket connection could not be established, it is also said
+        // that _The WebSocket Connection is Closed_, but not _cleanly_.
+        handler.readyState = states.CLOSED
+      }
+
       // 1. If response is a network error or its status is not 101,
       //    fail the WebSocket connection.
       if (response.type === 'error' || response.status !== 101) {
-        failWebsocketConnection(handler, 'Received network error or non-101 status code.')
+        failWebsocketConnection(handler, 1002, 'Received network error or non-101 status code.')
         return
       }
 
@@ -106,7 +112,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       //    header list results in null, failure, or the empty byte
       //    sequence, then fail the WebSocket connection.
       if (protocols.length !== 0 && !response.headersList.get('Sec-WebSocket-Protocol')) {
-        failWebsocketConnection(handler, 'Server did not respond with sent protocols.')
+        failWebsocketConnection(handler, 1002, 'Server did not respond with sent protocols.')
         return
       }
 
@@ -121,7 +127,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       //    insensitive match for the value "websocket", the client MUST
       //    _Fail the WebSocket Connection_.
       if (response.headersList.get('Upgrade')?.toLowerCase() !== 'websocket') {
-        failWebsocketConnection(handler, 'Server did not set Upgrade header to "websocket".')
+        failWebsocketConnection(handler, 1002, 'Server did not set Upgrade header to "websocket".')
         return
       }
 
@@ -130,7 +136,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       //    ASCII case-insensitive match for the value "Upgrade", the client
       //    MUST _Fail the WebSocket Connection_.
       if (response.headersList.get('Connection')?.toLowerCase() !== 'upgrade') {
-        failWebsocketConnection(handler, 'Server did not set Connection header to "upgrade".')
+        failWebsocketConnection(handler, 1002, 'Server did not set Connection header to "upgrade".')
         return
       }
 
@@ -144,7 +150,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       const secWSAccept = response.headersList.get('Sec-WebSocket-Accept')
       const digest = crypto.createHash('sha1').update(keyValue + uid).digest('base64')
       if (secWSAccept !== digest) {
-        failWebsocketConnection(handler, 'Incorrect hash received in Sec-WebSocket-Accept header.')
+        failWebsocketConnection(handler, 1002, 'Incorrect hash received in Sec-WebSocket-Accept header.')
         return
       }
 
@@ -162,7 +168,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
         extensions = parseExtensions(secExtension)
 
         if (!extensions.has('permessage-deflate')) {
-          failWebsocketConnection(handler, 'Sec-WebSocket-Extensions header does not match.')
+          failWebsocketConnection(handler, 1002, 'Sec-WebSocket-Extensions header does not match.')
           return
         }
       }
@@ -183,7 +189,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
         // the selected subprotocol values in its response for the connection to
         // be established.
         if (!requestProtocols.includes(secProtocol)) {
-          failWebsocketConnection(handler, 'Protocol was not set in the opening handshake.')
+          failWebsocketConnection(handler, 1002, 'Protocol was not set in the opening handshake.')
           return
         }
       }

lib/web/websocket/receiver.js

@@ -88,12 +88,12 @@ class ByteParser extends Writable {
         const rsv3 = buffer[0] & 0x10
 
         if (!isValidOpcode(opcode)) {
-          failWebsocketConnection(this.#handler, 'Invalid opcode received')
+          failWebsocketConnection(this.#handler, 1002, 'Invalid opcode received')
           return callback()
         }
 
         if (masked) {
-          failWebsocketConnection(this.#handler, 'Frame cannot be masked')
+          failWebsocketConnection(this.#handler, 1002, 'Frame cannot be masked')
           return callback()
         }
 
@@ -107,43 +107,43 @@ class ByteParser extends Writable {
         // WebSocket connection where a PMCE is in use, this bit indicates
         // whether a message is compressed or not.
         if (rsv1 !== 0 && !this.#extensions.has('permessage-deflate')) {
-          failWebsocketConnection(this.#handler, 'Expected RSV1 to be clear.')
+          failWebsocketConnection(this.#handler, 1002, 'Expected RSV1 to be clear.')
           return
         }
 
         if (rsv2 !== 0 || rsv3 !== 0) {
-          failWebsocketConnection(this.#handler, 'RSV1, RSV2, RSV3 must be clear')
+          failWebsocketConnection(this.#handler, 1002, 'RSV1, RSV2, RSV3 must be clear')
           return
         }
 
         if (fragmented && !isTextBinaryFrame(opcode)) {
           // Only text and binary frames can be fragmented
-          failWebsocketConnection(this.#handler, 'Invalid frame type was fragmented.')
+          failWebsocketConnection(this.#handler, 1002, 'Invalid frame type was fragmented.')
           return
         }
 
         // If we are already parsing a text/binary frame and do not receive either
         // a continuation frame or close frame, fail the connection.
         if (isTextBinaryFrame(opcode) && this.#fragments.length > 0) {
-          failWebsocketConnection(this.#handler, 'Expected continuation frame')
+          failWebsocketConnection(this.#handler, 1002, 'Expected continuation frame')
           return
         }
 
         if (this.#info.fragmented && fragmented) {
           // A fragmented frame can't be fragmented itself
-          failWebsocketConnection(this.#handler, 'Fragmented frame exceeded 125 bytes.')
+          failWebsocketConnection(this.#handler, 1002, 'Fragmented frame exceeded 125 bytes.')
           return
         }
 
         // "All control frames MUST have a payload length of 125 bytes or less
         // and MUST NOT be fragmented."
         if ((payloadLength > 125 || fragmented) && isControlFrame(opcode)) {
-          failWebsocketConnection(this.#handler, 'Control frame either too large or fragmented')
+          failWebsocketConnection(this.#handler, 1002, 'Control frame either too large or fragmented')
           return
         }
 
         if (isContinuationFrame(opcode) && this.#fragments.length === 0 && !this.#info.compressed) {
-          failWebsocketConnection(this.#handler, 'Unexpected continuation frame')
+          failWebsocketConnection(this.#handler, 1002, 'Unexpected continuation frame')
           return
         }
 
@@ -189,7 +189,7 @@ class ByteParser extends Writable {
         // https://source.chromium.org/chromium/chromium/src/+/main:v8/src/common/globals.h;drc=1946212ac0100668f14eb9e2843bdd846e510a1e;bpv=1;bpt=1;l=1275
         // https://source.chromium.org/chromium/chromium/src/+/main:v8/src/objects/js-array-buffer.h;l=34;drc=1946212ac0100668f14eb9e2843bdd846e510a1e
         if (upper > 2 ** 31 - 1) {
-          failWebsocketConnection(this.#handler, 'Received payload length > 2^31 bytes.')
+          failWebsocketConnection(this.#handler, 1009, 'Received payload length > 2^31 bytes.')
           return
         }
 
@@ -341,7 +341,7 @@ class ByteParser extends Writable {
 
     if (opcode === opcodes.CLOSE) {
       if (payloadLength === 1) {
-        failWebsocketConnection(this.#handler, 'Received close frame with a 1-byte body.')
+        failWebsocketConnection(this.#handler, 1002, 'Received close frame with a 1-byte body.')
         return false
       }
 
@@ -350,8 +350,7 @@ class ByteParser extends Writable {
       if (this.#info.closeInfo.error) {
         const { code, reason } = this.#info.closeInfo
 
-        closeWebSocketConnection(this.#handler, code, reason, reason.length)
-        failWebsocketConnection(this.#handler, reason)
+        failWebsocketConnection(this.#handler, code, reason)
         return false
       }
 

lib/web/websocket/util.js

@@ -150,10 +150,11 @@ function isValidStatusCode (code) {
 
 /**
  * @param {import('./websocket').Handler} handler
+ * @param {number} code
  * @param {string|undefined} reason
  */
-function failWebsocketConnection (handler, reason) {
-  handler.onFail(reason)
+function failWebsocketConnection (handler, code, reason) {
+  handler.onFail(code, reason)
 }
 
 /**

lib/web/websocket/websocket.js

@@ -28,7 +28,7 @@ const { channels } = require('../../core/diagnostics')
 /**
  * @typedef {object} Handler
  * @property {(response: any, extensions?: string[]) => void} onConnectionEstablished
- * @property {(reason: any) => void} onFail
+ * @property {(code: number, reason: any) => void} onFail
  * @property {(opcode: number, data: Buffer) => void} onMessage
  * @property {(code: number, reason: any, reasonByteLength: number) => void} onClose
  * @property {(error: Error) => void} onParserError
@@ -62,7 +62,7 @@ class WebSocket extends EventTarget {
   /** @type {Handler} */
   #handler = {
     onConnectionEstablished: (response, extensions) => this.#onConnectionEstablished(response, extensions),
-    onFail: (reason) => this.#onFail(reason),
+    onFail: (code, reason) => this.#onFail(code, reason),
     onMessage: (opcode, data) => this.#onMessage(opcode, data),
     onClose: (code, reason, reasonByteLength) => this.#onClose(code, reason, reasonByteLength),
     onParserError: (err) => this.#onParserError(err),
@@ -515,15 +515,21 @@ class WebSocket extends EventTarget {
     fireEvent('open', this)
   }
 
-  #onFail (reason) {
+  #onFail (code, reason) {
+    // If _The WebSocket Connection is Established_ prior to the point where
+    // the endpoint is required to _Fail the WebSocket Connection_, the
+    // endpoint SHOULD send a Close frame with an appropriate status code
+    // (Section 7.4) before proceeding to _Close the WebSocket Connection_.
+    if (isEstablished(this.#handler.readyState)) {
+      this.#onClose(code, reason, reason?.length)
+    }
+
     this.#controller.abort()
 
     if (this.#handler.socket && !this.#handler.socket.destroyed) {
       this.#handler.socket.destroy()
     }
 
-    this.#handler.readyState = states.CLOSED
-
     if (reason) {
       // TODO: process.nextTick
       fireEvent('error', this, (type, init) => new ErrorEvent(type, init), {
@@ -548,7 +554,7 @@ class WebSocket extends EventTarget {
       try {
         dataForEvent = utf8Decode(data)
       } catch {
-        failWebsocketConnection(this.#handler, 'Received invalid UTF-8 in text frame.')
+        failWebsocketConnection(this.#handler, 1007, 'Received invalid UTF-8 in text frame.')
         return
       }
     } else if (type === opcodes.BINARY) {
@@ -582,7 +588,7 @@ class WebSocket extends EventTarget {
       // If the WebSocket connection is not yet established
       // Fail the WebSocket connection and set this's ready state
       // to CLOSING (2).
-      failWebsocketConnection(this.#handler, 'Connection was closed before it was established.')
+      failWebsocketConnection(this.#handler, 1002, 'Connection was closed before it was established.')
       this.#handler.readyState = states.CLOSING
     } else if (this.#handler.closeState === sentCloseFrameState.NOT_SENT) {
       // If the WebSocket closing handshake has not yet been started

test/websocket/issue-3546.js

@@ -0,0 +1,24 @@
+'use strict'
+
+const { test } = require('node:test')
+const { WebSocket } = require('../..')
+const { tspl } = require('@matteo.collina/tspl')
+
+test('first error than close event is fired on failed connection', async (t) => {
+  const { completed, strictEqual } = tspl(t, { plan: 2 })
+  const ws = new WebSocket('ws://localhost:1')
+
+  let orderOfEvents = 0
+
+  ws.addEventListener('error', () => {
+    strictEqual(orderOfEvents++, 0)
+    strictEqual(ws.readyState, WebSocket.CLOSED)
+  })
+
+  ws.addEventListener('close', () => {
+    strictEqual(orderOfEvents++, 1)
+    strictEqual(ws.readyState, WebSocket.CLOSED)
+  })
+
+  await completed
+})