fix: Add an exception handling function to WebFluxSseClientTransport#223
fix: Add an exception handling function to WebFluxSseClientTransport#223YunKuiLu wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
| */ | ||
| private BiFunction<Throwable, ServerSentEvent<String>, Mono<? extends JSONRPCMessage>> sseErrorHandler = (error, | ||
| event) -> { | ||
| logger.warn("Failed to handle SSE event {}", event, error); |
There was a problem hiding this comment.
@YunKuiLu I am also facing the same issue. Just wondering whether we should change this to be DEBUG instead of WARN because there will be constant warn logs with the stacktrace.
There was a problem hiding this comment.
Hi @Niranjan-K
The WARN log here is just the default behavior. You can change it by using setSseErrorHandler.
I used a WARN log because I don’t want to hide errors by default.
| s.error(new McpError("Received unrecognized SSE event type: " + event.event())); | ||
| } | ||
| }).transform(handler)).subscribe(); | ||
| }).onErrorResume(e -> sseErrorHandler.apply(e, event)).transform(handler)).subscribe(); |
There was a problem hiding this comment.
The connection will not be closed by an McpError exception:
|
i met the same matter how to solve it？ 2025-05-09 16:07:23 INFO o.a.coyote.http11.Http11NioProtocol - Starting ProtocolHandler ["http-nio-9004"] |
@ai-srcflow This issue arises because the server did not strictly follow the MCP protocol when sending messages. If you are using spring-ai, you need to remove |
- `sseErrorHandler` is used to handle errors when processing SSE events and situations where an unrecognized event type is received. - Add unit tests to verify. Signed-off-by: YunKui Lu <luyunkui95@gmail.com>
|
Me too, calling other product MCP services caused an exception. I need this PR 👍 |
- Replace McpError exceptions with debug/warning logs for unrecognized SSE event types - Continue processing instead of failing when encountering unknown SSE events - Update transport implementations: - WebClientStreamableHttpTransport: return empty tuple instead of throwing - WebFluxSseClientTransport: complete stream instead of erroring - HttpClientSseClientTransport: call sink.success() instead of sink.error() - HttpClientStreamableHttpTransport: return empty Flux for unknown events This improves client resilience when servers send non-standard or future SSE event types. Resolves modelcontextprotocol#272 , modelcontextprotocol#223 , modelcontextprotocol#93 Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
…nts (#423) - Replace McpError exceptions with debug/warning logs for unrecognized SSE event types - Continue processing instead of failing when encountering unknown SSE events - Update transport implementations: - WebClientStreamableHttpTransport: return empty tuple instead of throwing - WebFluxSseClientTransport: complete stream instead of erroring - HttpClientSseClientTransport: call sink.success() instead of sink.error() - HttpClientStreamableHttpTransport: return empty Flux for unknown events This improves client resilience when servers send non-standard or future SSE event types. Resolves #272 , #223 , #93, #421 Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
|
Closing as addressed by the #423 |
5 participants
sseErrorHandleris used to handle errors when processing SSE events and situations where an unrecognized event type is received.Motivation and Context
Users can use custom handlers when they encounter the following exceptions:
#93
spring-projects/spring-ai#2936
All other clients are compatible with this, but the WebFlux client throws an exception, which is necessary to fix.
Although the MCP protocol does not allow any message types other than JSON-PRC, users should have a way to handle it instead of having the client stop. Therefore, I did not attempt to ignore it. Instead, I simply made it clearly visible in the log without interrupting the connection.
How Has This Been Tested?
Breaking Changes
I've provided a default error handler that prints the exception. User can also override it with the set method.
Types of changes
Checklist
Additional context
Since I added a new field to catch exceptions, I need Member to confirm that the design conforms to the future path of Mcp Java Sdk. Thus avoiding the introduction of an AD hoc solution.