Skip to content

Refactor: Extract WebSocketClientBase (DRY #63)#64

Open
shanselman wants to merge 1 commit intomasterfrom
refactor/websocket-base-class
Open

Refactor: Extract WebSocketClientBase (DRY #63)#64
shanselman wants to merge 1 commit intomasterfrom
refactor/websocket-base-class

Conversation

@shanselman
Copy link
Collaborator

WebSocket DRY Extraction (Issue #63)

Extracts ~200 lines of duplicated WebSocket lifecycle code from OpenClawGatewayClient and WindowsNodeClient into a shared abstract base class.

Design (Opus + Codex consensus)

  • Template method pattern: base class owns lifecycle, subclasses override hooks
  • Key decision: ProcessMessageAsync is always async (Task). Gateway wraps sync with Task.CompletedTask (zero-alloc cached singleton). Both models rejected async void.
  • Private _webSocket: subclasses never touch the socket directly

What moved to base class

  • ConnectAsync, ListenForMessagesAsync, ReconnectWithBackoffAsync, SendRawAsync, Dispose
  • All WebSocket fields (socket, URL, token, credentials, CTS, backoff)
  • StatusChanged event + RaiseStatusChanged helper

Subclass hooks

Hook Type Gateway Node
ProcessMessageAsync abstract wraps sync ProcessMessage direct async
ReceiveBufferSize abstract 16384 65536
ClientRole abstract "gateway" "node"
OnConnectedAsync virtual ResetUnsupportedMethodFlags (default)
OnDisconnected virtual ClearPendingRequests _isConnected=false
OnError virtual (default) _isConnected=false
OnDisposing virtual ClearPendingRequests (default)

Safety improvements

Adopted Node's safer patterns everywhere:

  • ObjectDisposedException catch in listen loop
  • Post-delay CTS check in reconnect
  • Constructor argument validation (null/empty URL and token)

Tests

  • 20 new tests via TestWebSocketClient test double
  • All 596 tests pass (503 shared + 93 tray)

Stats

  • 399 lines removed from clients, 573 lines added (269 base class + 244 tests + 60 overrides)
  • Net code: ~170 lines less production code, significantly more testable

Closes #63

Extract ~200 lines of duplicated WebSocket lifecycle code into a shared
abstract base class. Both OpenClawGatewayClient and WindowsNodeClient
now inherit from WebSocketClientBase.

Shared in base class:
- Connection lifecycle: ConnectAsync, ListenForMessagesAsync, ReconnectWithBackoffAsync
- SendRawAsync (thread-safe with TOCTOU protection)
- Dispose (defensive pattern, skip CTS dispose)
- Fields: WebSocket, URL, token, credentials, CTS, backoff array
- StatusChanged event with RaiseStatusChanged helper
- Constructor: URL normalization, credential extraction, validation

Subclass hooks (template method pattern):
- ProcessMessageAsync (abstract) - Gateway wraps sync, Node uses async
- ReceiveBufferSize (abstract) - Gateway 16KB, Node 64KB
- ClientRole (abstract) - for log messages
- OnConnectedAsync, OnDisconnected, OnError, OnDisposing (virtual)

Safety improvements (Node's safer patterns adopted everywhere):
- ObjectDisposedException catch in listen loop
- Post-delay CTS check in reconnect
- Try-catch around reconnect guard
- Constructor argument validation

20 new tests via TestWebSocketClient test double.
596 total tests pass (503 shared + 93 tray).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Extract shared WebSocket lifecycle from Gateway/Node clients

1 participant