feat(ws): adds ws transport client #139
Conversation
Aliaksie
changed the title
| .version(HttpClient.Version.HTTP_1_1) | ||
| .connectTimeout(Duration.ofSeconds(10)); | ||
|
|
||
| private ObjectMapper objectMapper = new ObjectMapper(); |
There was a problem hiding this comment.
Lots of object mappers defined in lots of files. It would seem like it might be time to build a Jackson module and lock in on one that can be used across the code base.
|
|
||
| public Mono<Void> connect(final Function<Mono<McpSchema.JSONRPCMessage>, Mono<McpSchema.JSONRPCMessage>> handler) { | ||
| if (!state.compareAndSet(TransportState.DISCONNECTED, TransportState.CONNECTING)) { | ||
| return Mono.error(new IllegalStateException("WebSocket is already connecting or connected")); |
There was a problem hiding this comment.
It might be better to point the actual state in the exception. I always hate error messages like "its either this or that", instead tell me what it was.
| fullMessage); | ||
| handler.apply(Mono.just(msg)).subscribe(); | ||
| } | ||
| catch (Exception e) { |
There was a problem hiding this comment.
I fixed this in #156. It would be nice to merge that and then we can stop catch Exception use a more fine grained type.
| WebSocket webSocket = webSocketRef.getAndSet(null); | ||
| if (webSocket != null && state.get() == TransportState.CONNECTED) { | ||
| state.set(TransportState.CLOSED); | ||
| return Mono.fromFuture(webSocket.sendClose(WebSocket.NORMAL_CLOSURE, "Closing")).then(); |
There was a problem hiding this comment.
A constant here would be better possibly.
| String json = objectMapper.writeValueAsString(message); | ||
| return Mono.fromFuture(ws.sendText(json, true)).then(); | ||
| } | ||
| catch (Exception e) { |
There was a problem hiding this comment.
UncheckedIOException seems nice here.
| catch (Exception e) { | ||
| return Mono.error(e); | ||
| } | ||
| }).retryWhen(Retry.backoff(3, Duration.ofSeconds(3)).filter(err -> { |
There was a problem hiding this comment.
Externalize or at least put in constants.
This pull request introduces the initial implementation for a WebSocket (WS) client transport in the system. It focuses on enabling communication over WebSocket for MCP client-side transport, laying the foundation for future enhancements such as adding a WebSocket server provider.
Motivation and Context
How Has This Been Tested?
This functionality has been tested in a local environment using WebSocket server containers.
Breaking Changes
NO
Types of changes
Checklist
Additional context
This pull request introduces the first implementation for the ws client transport. The ws server provider is planned for future implementation. Further improvements and features will be added in subsequent PRs.
let me know if i missed something and need to add or if these changes don't make sense no problem just close this pr :-)