refactor(cli): fix onerror ErrorEvent handling, DRY adapter types, simplify factory

diff --git a/ui/cli/src/client/lifecycle.ts b/ui/cli/src/client/lifecycle.ts
index 343e3238857a9b0b843c40952885348d8f4461ae..bf1f69e75938877ceae65753305659e77d8c9123 100644 (file)
--- a/ui/cli/src/client/lifecycle.ts
+++ b/ui/cli/src/client/lifecycle.ts
@@ -8,7 +8,6 @@ import {
   type UIServerConfig,
   WebSocketClient,
   type WebSocketFactory,
-  type WebSocketLike,
 } from 'ui-common'
 import { WebSocket as WsWebSocket } from 'ws'
 
@@ -17,11 +16,8 @@ import type { Formatter } from '../output/formatter.js'
 import { ConnectionError } from './errors.js'
 import { createWsAdapter } from './ws-adapter.js'
 
-const createWsFactory = (): WebSocketFactory => {
-  return (url: string, protocols: string | string[]): WebSocketLike => {
-    return createWsAdapter(new WsWebSocket(url, protocols))
-  }
-}
+const wsFactory: WebSocketFactory = (url, protocols) =>
+  createWsAdapter(new WsWebSocket(url, protocols))
 
 let activeClient: undefined | WebSocketClient
 let activeSpinner: null | ReturnType<typeof ora> | undefined
@@ -38,8 +34,7 @@ export interface ExecuteOptions {
 export const executeCommand = async (options: ExecuteOptions): Promise<void> => {
   const { config, formatter, payload, procedureName, timeoutMs } = options
 
-  const factory = createWsFactory()
-  const client = new WebSocketClient(factory, config, timeoutMs)
+  const client = new WebSocketClient(wsFactory, config, timeoutMs)
   const { url } = client
 
   const isInteractive = process.stderr.isTTY

diff --git a/ui/cli/src/client/ws-adapter.ts b/ui/cli/src/client/ws-adapter.ts
index bdb7fe1ca6d8b2175950aebe6500402f5da326c0..0408153f55b1d65048a76e6512ec08a287c6ec28 100644 (file)
--- a/ui/cli/src/client/ws-adapter.ts
+++ b/ui/cli/src/client/ws-adapter.ts
@@ -17,10 +17,10 @@ const toDataString = (data: WsWebSocket.Data): string => {
 }
 
 export const createWsAdapter = (ws: WsWebSocket): WebSocketLike => {
-  let onmessageCallback: ((event: { data: string }) => void) | null = null
-  let onerrorCallback: ((event: { error: unknown; message: string }) => void) | null = null
-  let oncloseCallback: ((event: { code: number; reason: string }) => void) | null = null
-  let onopenCallback: (() => void) | null = null
+  let onmessageCallback: WebSocketLike['onmessage'] = null
+  let onerrorCallback: WebSocketLike['onerror'] = null
+  let oncloseCallback: WebSocketLike['onclose'] = null
+  let onopenCallback: WebSocketLike['onopen'] = null
 
   ws.onmessage = event => {
     if (onmessageCallback != null) {
@@ -31,15 +31,10 @@ export const createWsAdapter = (ws: WsWebSocket): WebSocketLike => {
 
   ws.onerror = event => {
     if (onerrorCallback != null) {
-      let error: Error
-      let message: string
-      if (event instanceof Error) {
-        error = event
-        message = event.message
-      } else {
-        message = typeof event === 'string' ? event : 'Unknown error'
-        error = new Error(message)
-      }
+      const raw = event as { error?: unknown; message?: string }
+      const error =
+        raw.error instanceof Error ? raw.error : new Error(raw.message ?? 'Unknown error')
+      const message = raw.message ?? error.message
       onerrorCallback({ error, message })
     }
   }
@@ -60,32 +55,32 @@ export const createWsAdapter = (ws: WsWebSocket): WebSocketLike => {
     close (code?: number, reason?: string): void {
       ws.close(code, reason)
     },
-    get onclose (): ((event: { code: number; reason: string }) => void) | null {
+    get onclose () {
       return oncloseCallback
     },
 
-    set onclose (callback: ((event: { code: number; reason: string }) => void) | null) {
+    set onclose (callback) {
       oncloseCallback = callback
     },
-    get onerror (): ((event: { error: unknown; message: string }) => void) | null {
+    get onerror () {
       return onerrorCallback
     },
 
-    set onerror (callback: ((event: { error: unknown; message: string }) => void) | null) {
+    set onerror (callback) {
       onerrorCallback = callback
     },
-    get onmessage (): ((event: { data: string }) => void) | null {
+    get onmessage () {
       return onmessageCallback
     },
 
-    set onmessage (callback: ((event: { data: string }) => void) | null) {
+    set onmessage (callback) {
       onmessageCallback = callback
     },
-    get onopen (): (() => void) | null {
+    get onopen () {
       return onopenCallback
     },
 
-    set onopen (callback: (() => void) | null) {
+    set onopen (callback) {
       onopenCallback = callback
     },
 

diff --git a/ui/cli/tests/lifecycle.test.ts b/ui/cli/tests/lifecycle.test.ts
index 60c41f928c0803dbdb1194ae5ae5f9fe82be1e0b..b8a1e74011b3a7cb66a2d693b902860291c30790 100644 (file)
--- a/ui/cli/tests/lifecycle.test.ts
+++ b/ui/cli/tests/lifecycle.test.ts
@@ -1,7 +1,4 @@
-/**
- * @file Unit tests for CLI lifecycle and error types
- * @description Tests for connection lifecycle management and error handling
- */
+/** @file Unit tests for CLI lifecycle and error types */
 
 import assert from 'node:assert'
 import { describe, it } from 'node:test'

diff --git a/ui/cli/tests/output.test.ts b/ui/cli/tests/output.test.ts
index 2120004a21080b5a46604e2541b8219bf3a0033a..ef2460925a183cdefb15072e8a1bbbbb4e0e970e 100644 (file)
--- a/ui/cli/tests/output.test.ts
+++ b/ui/cli/tests/output.test.ts
@@ -1,7 +1,4 @@
-/**
- * @file Unit tests for CLI output formatters (JSON and table)
- * @description Tests for JSON and table output formatting functions
- */
+/** @file Unit tests for CLI output formatters (JSON and table) */
 
 import assert from 'node:assert'
 import { describe, it } from 'node:test'

diff --git a/ui/cli/tests/ws-adapter.test.ts b/ui/cli/tests/ws-adapter.test.ts
index 1e50e91fa4d746208dc68027ddac965820734a03..2a38f635b0bbd6924b407628850ce7793ca0e073 100644 (file)
--- a/ui/cli/tests/ws-adapter.test.ts
+++ b/ui/cli/tests/ws-adapter.test.ts
@@ -1,7 +1,4 @@
-/**
- * @file Unit tests for the WebSocket adapter (ws → WebSocketLike)
- * @description Tests for converting ws library WebSocket to WebSocketLike interface
- */
+/** @file Unit tests for the WebSocket adapter (ws → WebSocketLike) */
 
 import type { WebSocket } from 'ws'
 
@@ -136,7 +133,6 @@ await describe('WS Adapter', async () => {
 
   await it('should forward onerror event with error shape', () => {
     const mockWs = createMockWs()
-
     const adapter = createWsAdapter(mockWs as unknown as WebSocket)
 
     let receivedError: unknown
@@ -146,13 +142,14 @@ await describe('WS Adapter', async () => {
       receivedMessage = event.message
     }
 
-    const testError = new Error('connection failed')
-    mockWs.onerror?.(testError)
+    const cause = new Error('connection failed')
+    // Simulate ws ErrorEvent (has .error and .message properties)
+    mockWs.onerror?.({ error: cause, message: 'connection failed' })
 
     assert.ok(receivedError instanceof Error)
-    // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
-    const error = receivedError as Error
-    assert.strictEqual(error.message, 'connection failed')
+    if (receivedError instanceof Error) {
+      assert.strictEqual(receivedError.message, 'connection failed')
+    }
     assert.strictEqual(receivedMessage, 'connection failed')
   })
 
@@ -164,7 +161,7 @@ await describe('WS Adapter', async () => {
       receivedMessage = event.message
     }
     mockWs.onerror?.('connection refused')
-    assert.strictEqual(receivedMessage, 'connection refused')
+    assert.strictEqual(receivedMessage, 'Unknown error')
   })
 
   await it('should forward onerror with fallback for unknown event type', () => {
@@ -178,6 +175,21 @@ await describe('WS Adapter', async () => {
     assert.strictEqual(receivedMessage, 'Unknown error')
   })
 
+  await it('should forward onerror when event has error but no message', () => {
+    const mockWs = createMockWs()
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+    let receivedMessage = ''
+    let receivedError: unknown
+    adapter.onerror = event => {
+      receivedError = event.error
+      receivedMessage = event.message
+    }
+    const cause = new Error('ECONNREFUSED')
+    mockWs.onerror?.({ error: cause })
+    assert.ok(receivedError instanceof Error)
+    assert.strictEqual(receivedMessage, 'ECONNREFUSED')
+  })
+
   await it('should forward onclose event with code and reason', () => {
     const mockWs = createMockWs()
     mockWs.readyState = WebSocketReadyState.CLOSED

diff --git a/ui/common/tests/WebSocketClient.test.ts b/ui/common/tests/WebSocketClient.test.ts
index ec9422c19d99670d5fc305af9de0fc5eefa7a536..240650114ba660028afaa3316f2623d820a69bee 100644 (file)
--- a/ui/common/tests/WebSocketClient.test.ts
+++ b/ui/common/tests/WebSocketClient.test.ts
@@ -1,7 +1,4 @@
-/**
- * @file Unit tests for the SRPC WebSocket client
- * @description Tests for WebSocketClient connection, request/response handling, and timeout validation
- */
+/** @file Unit tests for the SRPC WebSocket client */
 
 import assert from 'node:assert'
 import { describe, it } from 'node:test'