ws: move implementation agnostic onFail logic to shared function (#3663)

Author: KhafraDev

lib/web/websocket/connection.js

@@ -208,6 +208,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
         })
       }
 
+      handler.wasEverConnected = true
       handler.onConnectionEstablished(response, extensions)
     }
   })

lib/web/websocket/stream/websocketstream.js

@@ -38,17 +38,14 @@ class WebSocketStream {
   /** @type {WritableStream} */
   #writableStream
 
-  // Each WebSocketStream object has an associated was ever connected , which is a boolean, initially false.
-  #wasEverConnected = false
-
   // Each WebSocketStream object has an associated boolean handshake aborted , which is initially false.
   #handshakeAborted = false
 
   /** @type {import('../websocket').Handler} */
   #handler = {
     // https://whatpr.org/websockets/48/7b748d3...d5570f3.html#feedback-to-websocket-stream-from-the-protocol
     onConnectionEstablished: (response, extensions) => this.#onConnectionEstablished(response, extensions),
-    onFail: (reason) => this.#onFail(reason),
+    onFail: (_code, _reason) => {},
     onMessage: (opcode, data) => this.#onMessage(opcode, data),
     onParserError: (err) => failWebsocketConnection(this.#handler, null, err.message),
     onParserDrain: () => this.#handler.socket.resume(),
@@ -71,15 +68,13 @@ class WebSocketStream {
     readyState: states.CONNECTING,
     socket: null,
     closeState: new Set(),
-    receivedClose: false
+    controller: null,
+    wasEverConnected: false
   }
 
   /** @type {import('../receiver').ByteParser} */
   #parser
 
-  /** @type {import('../../fetch/index').Fetch} */
-  #controller
-
   constructor (url, options = undefined) {
     if (!emittedExperimentalWarning) {
       process.emitWarning('WebSocketStream is experimental! Expect it to change at any time.', {
@@ -161,7 +156,7 @@ class WebSocketStream {
 
     // 10. Run this step in parallel :
     // 10.1. Establish a WebSocket connection given urlRecord , protocols , and client . [FETCH]
-    this.#controller = establishWebSocketConnection(
+    this.#handler.controller = establishWebSocketConnection(
       urlRecord,
       protocols,
       client,
@@ -269,7 +264,7 @@ class WebSocketStream {
     this.#handler.readyState = states.OPEN
 
     // 2. Set stream ’s was ever connected to true.
-    this.#wasEverConnected = true
+    // This is done in the opening handshake.
 
     // 3. Let extensions be the extensions in use .
     const extensions = parsedExtensions ?? ''
@@ -367,7 +362,7 @@ class WebSocketStream {
     }
 
     // 3. If stream ’s was ever connected is false, then reject stream ’s opened promise with a new WebSocketError.
-    if (!this.#wasEverConnected) {
+    if (!this.#handler.wasEverConnected) {
       this.#openedPromise.reject(new WebSocketError('Socket never opened'))
     }
 
@@ -422,22 +417,6 @@ class WebSocketStream {
     }
   }
 
-  #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)) {
-      closeWebSocketConnection(this.#handler, code, reason, false)
-    }
-
-    this.#controller.abort()
-
-    if (this.#handler.socket && !this.#handler.socket.destroyed) {
-      this.#handler.socket.destroy()
-    }
-  }
-
   #closeUsingReason (reason) {
     // 1. Let code be null.
     let code = null

lib/web/websocket/util.js

@@ -163,6 +163,22 @@ function isValidStatusCode (code) {
  * @returns {void}
  */
 function failWebsocketConnection (handler, 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(handler.readyState)) {
+    // avoid circular require - performance is not important here
+    const { closeWebSocketConnection } = require('./connection')
+    closeWebSocketConnection(handler, code, reason, false)
+  }
+
+  handler.controller.abort()
+
+  if (handler.socket?.destroyed === false) {
+    handler.socket.destroy()
+  }
+
   handler.onFail(code, reason)
 }
 

lib/web/websocket/websocket.js

@@ -38,6 +38,8 @@ const { channels } = require('../../core/diagnostics')
  * @property {number} readyState
  * @property {import('stream').Duplex} socket
  * @property {Set<number>} closeState
+ * @property {import('../fetch/index').Fetch} controller
+ * @property {boolean} [wasEverConnected=false]
  */
 
 // https://websockets.spec.whatwg.org/#interface-definition
@@ -81,11 +83,12 @@ class WebSocket extends EventTarget {
 
     readyState: states.CONNECTING,
     socket: null,
-    closeState: new Set()
+    closeState: new Set(),
+    controller: null,
+    wasEverConnected: false
   }
 
   #url
-  #controller
   #binaryType
   /** @type {import('./receiver').ByteParser} */
   #parser
@@ -138,7 +141,7 @@ class WebSocket extends EventTarget {
     // 7. Run this step in parallel:
     // 7.1. Establish a WebSocket connection given urlRecord, protocols,
     //      and client.
-    this.#controller = establishWebSocketConnection(
+    this.#handler.controller = establishWebSocketConnection(
       urlRecord,
       protocols,
       client,
@@ -467,25 +470,13 @@ class WebSocket extends EventTarget {
       })
     }
 
-    // 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)) {
-      closeWebSocketConnection(this.#handler, code, reason, false)
-    } else {
+    if (!this.#handler.wasEverConnected) {
       // If the WebSocket connection could not be established, it is also said
       // that _The WebSocket Connection is Closed_, but not _cleanly_.
       fireEvent('close', this, (type, init) => new CloseEvent(type, init), {
         wasClean: false, code, reason
       })
     }
-
-    this.#controller.abort()
-
-    if (this.#handler.socket && !this.#handler.socket.destroyed) {
-      this.#handler.socket.destroy()
-    }
   }
 
   #onMessage (type, data) {