fix(websocket): defer close()/fail() events to a queued task when CONNECTING

lib/web/websocket/connection.js

@@ -315,8 +315,14 @@ function failWebsocketConnection (handler, code, reason, cause) {
   handler.controller.abort()
 
   if (isConnecting(handler.readyState)) {
-    // If the connection was not established, we must still emit an 'error' and 'close' events
-    handler.onSocketClose()
+    // If the connection was not established, there is no underlying socket
+    // whose asynchronous 'close' event would otherwise trigger onSocketClose
+    // later. The spec's "queue a task" is a macrotask, not a microtask, so
+    // use setImmediate (not process.nextTick) to invoke it here instead —
+    // this way the 'error' and 'close' events fire after (not during) the
+    // caller's close()/send()/etc, per
+    // https://websockets.spec.whatwg.org/#feedback-from-the-protocol
+    setImmediate(() => handler.onSocketClose())
   } else if (handler.socket?.destroyed === false) {
     handler.socket.destroy()
   }

lib/web/websocket/stream/websocketstream.js

@@ -136,6 +136,12 @@ class WebSocketStream {
       addAbortListener(signal, () => {
         // 8.3.1. If the WebSocket connection is not yet established : [WSP]
         if (!isEstablished(this.#handler.readyState)) {
+          // Set this 's handshake aborted to true before failing the
+          // connection, so that #onSocketClose (which failWebsocketConnection
+          // triggers, possibly asynchronously) defers to the rejections below
+          // instead of settling these promises itself.
+          this.#handshakeAborted = true
+
           // 8.3.1.1. Fail the WebSocket connection .
           failWebsocketConnection(this.#handler)
 
@@ -145,9 +151,6 @@ class WebSocketStream {
           // Reject this 's opened promise and closed promise with signal ’s abort reason .
           this.#openedPromise.reject(signal.reason)
           this.#closedPromise.reject(signal.reason)
-
-          // Set this 's handshake aborted to true.
-          this.#handshakeAborted = true
         }
       })
     }

lib/web/websocket/websocket.js

@@ -640,7 +640,6 @@ class WebSocket extends EventTarget {
     //    attribute initialized to the result of applying UTF-8
     //    decode without BOM to the WebSocket connection close
     //    reason.
-    // TODO: process.nextTick
     fireEvent('close', this, (type, init) => new CloseEvent(type, init), {
       wasClean, code, reason
     })

test/websocket/close.js

@@ -2,6 +2,7 @@
 
 const { tspl } = require('@matteo.collina/tspl')
 const { describe, test, after } = require('node:test')
+const { createServer } = require('node:net')
 const { WebSocketServer } = require('ws')
 const { WebSocket } = require('../..')
 
@@ -150,4 +151,28 @@ describe('Close', () => {
 
     await t.completed
   })
+
+  // Regression test for https://github.com/nodejs/undici/issues/4741
+  test('close() while CONNECTING fires error/close asynchronously, not during close()', async (t) => {
+    t = tspl(t, { plan: 1 })
+
+    // Accepts the TCP connection but never responds, so the WebSocket
+    // handshake never completes and the client stays in CONNECTING.
+    const server = createServer((socket) => socket.resume())
+    after(() => server.close())
+
+    await new Promise((resolve) => server.listen(0, resolve))
+
+    const ws = new WebSocket(`ws://localhost:${server.address().port}`)
+
+    let closeReturned = false
+    ws.addEventListener('close', () => {
+      t.ok(closeReturned, 'close event must fire after close() returns')
+    })
+
+    ws.close()
+    closeReturned = true
+
+    await t.completed
+  })
 })

test/websocket/issue-4628.js

@@ -4,11 +4,9 @@ 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) => {
+test('closing before connection is established should only fire error and close events once', async (t) => {
   t.plan(2)
 
-  t.after(() => assert.deepStrictEqual(events, ['error', 'close']))
-
   const events = []
   const ws = new WebSocket('wss://example.com/')
 
@@ -19,10 +17,15 @@ test('closing before connection is established should only fire error and close
     events.push('error')
   })
 
-  ws.addEventListener('close', () => {
-    t.assert.ok(true, 'close event fired')
-    events.push('close')
+  await new Promise((resolve) => {
+    ws.addEventListener('close', () => {
+      t.assert.ok(true, 'close event fired')
+      events.push('close')
+      resolve()
+    })
+
+    ws.close()
   })
 
-  ws.close()
+  assert.deepStrictEqual(events, ['error', 'close'])
 })

test/websocket/stream/abort-before-open.js

@@ -33,11 +33,11 @@ test('WebSocketStream aborts before handshake completes', async (t) => {
 
   const [opened, closed] = await Promise.allSettled([wss.opened, wss.closed])
 
+  // Per the spec, aborting before the handshake completes rejects both
+  // promises with the abort signal's own reason.
   t.assert.strictEqual(opened.status, 'rejected')
-  t.assert.strictEqual(opened.reason.name, 'WebSocketError')
-  t.assert.strictEqual(opened.reason.message, 'Socket never opened')
+  t.assert.strictEqual(opened.reason.message, 'abort before open')
 
   t.assert.strictEqual(closed.status, 'rejected')
-  t.assert.strictEqual(closed.reason.name, 'WebSocketError')
-  t.assert.strictEqual(closed.reason.message, 'unclean close')
+  t.assert.strictEqual(closed.reason.message, 'abort before open')
 })