fix(ws) onSocketClose being called multiple times (#4632)

Author: KhafraDev

lib/web/websocket/connection.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { uid, states, sentCloseFrameState, emptyBuffer, opcodes } = require('./constants')
-const { parseExtensions, isClosed, isClosing, isEstablished, validateCloseCodeAndReason } = require('./util')
+const { parseExtensions, isClosed, isClosing, isEstablished, isConnecting, validateCloseCodeAndReason } = require('./util')
 const { makeRequest } = require('../fetch/request')
 const { fetching } = require('../fetch/index')
 const { Headers, getHeadersList } = require('../fetch/headers')
@@ -91,12 +91,6 @@ 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) {
@@ -299,10 +293,10 @@ function failWebsocketConnection (handler, code, reason, cause) {
 
   handler.controller.abort()
 
-  if (!handler.socket) {
+  if (isConnecting(handler.readyState)) {
     // If the connection was not established, we must still emit an 'error' and 'close' events
     handler.onSocketClose()
-  } else if (handler.socket.destroyed === false) {
+  } else if (handler.socket?.destroyed === false) {
     handler.socket.destroy()
   }
 }

package.json

@@ -113,7 +113,7 @@
     "@matteo.collina/tspl": "^0.2.0",
     "@metcoder95/https-pem": "^1.0.0",
     "@sinonjs/fake-timers": "^12.0.0",
-    "@types/node": "^18.19.50",
+    "@types/node": "^20.19.22",
     "abort-controller": "^3.0.0",
     "borp": "^0.20.0",
     "c8": "^10.0.0",

test/websocket/issue-4628.js

@@ -0,0 +1,28 @@
+'use strict'
+
+const assert = require('node:assert')
+const { test } = require('node:test')
+const { WebSocket } = require('../..')
+
+test('closing before connection is established should only fire error and close events once', (t) => {
+  t.plan(2)
+
+  t.after(() => assert.deepStrictEqual(events, ['error', 'close']))
+
+  const events = []
+  const ws = new WebSocket('wss://example.com/')
+
+  ws.onopen = t.assert.fail
+
+  ws.addEventListener('error', () => {
+    t.assert.ok(true, 'error event fired')
+    events.push('error')
+  })
+
+  ws.addEventListener('close', () => {
+    t.assert.ok(true, 'close event fired')
+    events.push('close')
+  })
+
+  ws.close()
+})