fix: improve compatibility with non-compliant MCP servers #413
Conversation
…xtprotocol#413) Addresses issues with servers like Shopify that violate MCP/HTTP specs: - Prioritize application/json in Accept headers to fix content-type issues - Handle empty notification bodies ({}) that should be bodiless per spec - Add status code validation and null safety improvements Resolves modelcontextprotocol#406 Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
tzolov
changed the title
tzolov
changed the title
| s.next(List.of(jsonRpcResponse)); | ||
| } | ||
| else { | ||
| logger.warn("Received empty response message: {}", responseMessage); |
There was a problem hiding this comment.
I am a bit worried that if the request body was an MCP Request then we will never complete a pending operation that awaits for it. Please consider adding a check whether the original message was a request and if so send an error to the Flux (using handle's sink).
There was a problem hiding this comment.
Unfortunately if we receive an empty response message (e.g. null, " " or {}) there is no way to determine if it is a bogus response from a previous request, compliment notification response or bogus request.
Here we assume that if an empty even is received thread it as incorrect notification response.
| else { | ||
| // No content type means no response body | ||
| logger.debug("No content type returned for POST in session {}", sessionRepresentation); | ||
| return Mono.empty(); |
There was a problem hiding this comment.
Same comment as for the WebClient implementation.
| System.out.println("Received non-successful response: " + this.responseInfo.statusCode()); | ||
| SseEvent sseEvent = new SseEvent(null, null, null); | ||
| this.sink.next(new SseResponseEvent(responseInfo, sseEvent)); |
There was a problem hiding this comment.
This looks unfinished. Did you intend to use the logger and call sink.error ?
There was a problem hiding this comment.
Yeh good catch. I've mistakenly commit some experiments. Above logic is incorrect.
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Addresses issues with servers like Shopify that violate MCP/HTTP specs:
Resolves #406
Improves HTTP transport robustness to handle non-compliant MCP servers that violate MCP and HTTP specifications.
Motivation and Context
This change addresses compatibility issues with real-world MCP servers that don't strictly follow MCP and HTTP specifications. Specifically, the Shopify MCP server exhibits two problematic behaviors:
text/streamablecontent-type when the Accept header prioritizestext/streamoverapplication/json, but correctly returnsapplication/jsonwhen the latter is first{}) for notification messages that should be bodiless according to the MCP specificationThese violations cause the Java SDK to fail when communicating with such servers, limiting its real-world applicability.
How Has This Been Tested?
Breaking Changes
No breaking changes. This is a backward-compatible improvement that maintains existing API contracts while adding robustness.
Types of changes
Checklist