fix: improve compatibility with non-compliant MCP servers (#413)
Addresses issues with servers like Shopify that violate MCP/HTTP specs: - Prioritize application/json in Accept headers to fix content-type issues - Handle non-compliant notification non-empty responses - Add status code validation and null safety improvements Resolves #406 Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
This commit is contained in:
parent
1e93776354
commit
4937fc1ef5
4 changed files with 37 additions and 10 deletions
|
|
@ -30,6 +30,7 @@ import io.modelcontextprotocol.spec.McpTransportSession;
|
||||||
import io.modelcontextprotocol.spec.McpTransportSessionNotFoundException;
|
import io.modelcontextprotocol.spec.McpTransportSessionNotFoundException;
|
||||||
import io.modelcontextprotocol.spec.McpTransportStream;
|
import io.modelcontextprotocol.spec.McpTransportStream;
|
||||||
import io.modelcontextprotocol.util.Assert;
|
import io.modelcontextprotocol.util.Assert;
|
||||||
|
import io.modelcontextprotocol.util.Utils;
|
||||||
import reactor.core.Disposable;
|
import reactor.core.Disposable;
|
||||||
import reactor.core.publisher.Flux;
|
import reactor.core.publisher.Flux;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
@ -244,7 +245,7 @@ public class WebClientStreamableHttpTransport implements McpClientTransport {
|
||||||
|
|
||||||
Disposable connection = webClient.post()
|
Disposable connection = webClient.post()
|
||||||
.uri(this.endpoint)
|
.uri(this.endpoint)
|
||||||
.accept(MediaType.TEXT_EVENT_STREAM, MediaType.APPLICATION_JSON)
|
.accept(MediaType.APPLICATION_JSON, MediaType.TEXT_EVENT_STREAM)
|
||||||
.headers(httpHeaders -> {
|
.headers(httpHeaders -> {
|
||||||
transportSession.sessionId().ifPresent(id -> httpHeaders.add("mcp-session-id", id));
|
transportSession.sessionId().ifPresent(id -> httpHeaders.add("mcp-session-id", id));
|
||||||
})
|
})
|
||||||
|
|
@ -287,7 +288,7 @@ public class WebClientStreamableHttpTransport implements McpClientTransport {
|
||||||
logger.trace("Received response to POST for session {}", sessionRepresentation);
|
logger.trace("Received response to POST for session {}", sessionRepresentation);
|
||||||
// communicate to caller the message was delivered
|
// communicate to caller the message was delivered
|
||||||
sink.success();
|
sink.success();
|
||||||
return responseFlux(response);
|
return directResponseFlux(message, response);
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
logger.warn("Unknown media type {} returned for POST in session {}", contentType,
|
logger.warn("Unknown media type {} returned for POST in session {}", contentType,
|
||||||
|
|
@ -384,14 +385,22 @@ public class WebClientStreamableHttpTransport implements McpClientTransport {
|
||||||
return transportSession.sessionId().orElse("[missing_session_id]");
|
return transportSession.sessionId().orElse("[missing_session_id]");
|
||||||
}
|
}
|
||||||
|
|
||||||
private Flux<McpSchema.JSONRPCMessage> responseFlux(ClientResponse response) {
|
private Flux<McpSchema.JSONRPCMessage> directResponseFlux(McpSchema.JSONRPCMessage sentMessage,
|
||||||
|
ClientResponse response) {
|
||||||
return response.bodyToMono(String.class).<Iterable<McpSchema.JSONRPCMessage>>handle((responseMessage, s) -> {
|
return response.bodyToMono(String.class).<Iterable<McpSchema.JSONRPCMessage>>handle((responseMessage, s) -> {
|
||||||
try {
|
try {
|
||||||
|
if (sentMessage instanceof McpSchema.JSONRPCNotification && Utils.hasText(responseMessage)) {
|
||||||
|
logger.warn("Notification: {} received non-compliant response: {}", sentMessage, responseMessage);
|
||||||
|
s.complete();
|
||||||
|
}
|
||||||
|
else {
|
||||||
McpSchema.JSONRPCMessage jsonRpcResponse = McpSchema.deserializeJsonRpcMessage(objectMapper,
|
McpSchema.JSONRPCMessage jsonRpcResponse = McpSchema.deserializeJsonRpcMessage(objectMapper,
|
||||||
responseMessage);
|
responseMessage);
|
||||||
s.next(List.of(jsonRpcResponse));
|
s.next(List.of(jsonRpcResponse));
|
||||||
}
|
}
|
||||||
|
}
|
||||||
catch (IOException e) {
|
catch (IOException e) {
|
||||||
|
// TODO: this should be a McpTransportError
|
||||||
s.error(e);
|
s.error(e);
|
||||||
}
|
}
|
||||||
}).flatMapIterable(Function.identity());
|
}).flatMapIterable(Function.identity());
|
||||||
|
|
|
||||||
|
|
@ -342,9 +342,9 @@ public class HttpClientStreamableHttpTransport implements McpClientTransport {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public Mono<Void> sendMessage(McpSchema.JSONRPCMessage sendMessage) {
|
public Mono<Void> sendMessage(McpSchema.JSONRPCMessage sentMessage) {
|
||||||
return Mono.create(messageSink -> {
|
return Mono.create(messageSink -> {
|
||||||
logger.debug("Sending message {}", sendMessage);
|
logger.debug("Sending message {}", sentMessage);
|
||||||
|
|
||||||
final AtomicReference<Disposable> disposableRef = new AtomicReference<>();
|
final AtomicReference<Disposable> disposableRef = new AtomicReference<>();
|
||||||
final McpTransportSession<Disposable> transportSession = this.activeSession.get();
|
final McpTransportSession<Disposable> transportSession = this.activeSession.get();
|
||||||
|
|
@ -355,10 +355,10 @@ public class HttpClientStreamableHttpTransport implements McpClientTransport {
|
||||||
requestBuilder = requestBuilder.header("mcp-session-id", transportSession.sessionId().get());
|
requestBuilder = requestBuilder.header("mcp-session-id", transportSession.sessionId().get());
|
||||||
}
|
}
|
||||||
|
|
||||||
String jsonBody = this.toString(sendMessage);
|
String jsonBody = this.toString(sentMessage);
|
||||||
|
|
||||||
HttpRequest request = requestBuilder.uri(Utils.resolveUri(this.baseUri, this.endpoint))
|
HttpRequest request = requestBuilder.uri(Utils.resolveUri(this.baseUri, this.endpoint))
|
||||||
.header("Accept", TEXT_EVENT_STREAM + ", " + APPLICATION_JSON)
|
.header("Accept", APPLICATION_JSON + ", " + TEXT_EVENT_STREAM)
|
||||||
.header("Content-Type", APPLICATION_JSON)
|
.header("Content-Type", APPLICATION_JSON)
|
||||||
.header("Cache-Control", "no-cache")
|
.header("Cache-Control", "no-cache")
|
||||||
.POST(HttpRequest.BodyPublishers.ofString(jsonBody))
|
.POST(HttpRequest.BodyPublishers.ofString(jsonBody))
|
||||||
|
|
@ -436,10 +436,16 @@ public class HttpClientStreamableHttpTransport implements McpClientTransport {
|
||||||
else if (contentType.contains(APPLICATION_JSON)) {
|
else if (contentType.contains(APPLICATION_JSON)) {
|
||||||
messageSink.success();
|
messageSink.success();
|
||||||
String data = ((ResponseSubscribers.AggregateResponseEvent) responseEvent).data();
|
String data = ((ResponseSubscribers.AggregateResponseEvent) responseEvent).data();
|
||||||
|
if (sentMessage instanceof McpSchema.JSONRPCNotification && Utils.hasText(data)) {
|
||||||
|
logger.warn("Notification: {} received non-compliant response: {}", sentMessage, data);
|
||||||
|
return Mono.empty();
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return Mono.just(McpSchema.deserializeJsonRpcMessage(objectMapper, data));
|
return Mono.just(McpSchema.deserializeJsonRpcMessage(objectMapper, data));
|
||||||
}
|
}
|
||||||
catch (IOException e) {
|
catch (IOException e) {
|
||||||
|
// TODO: this should be a McpTransportError
|
||||||
return Mono.error(e);
|
return Mono.error(e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@ import java.util.regex.Pattern;
|
||||||
import org.reactivestreams.FlowAdapters;
|
import org.reactivestreams.FlowAdapters;
|
||||||
import org.reactivestreams.Subscription;
|
import org.reactivestreams.Subscription;
|
||||||
|
|
||||||
|
import io.modelcontextprotocol.spec.McpError;
|
||||||
import reactor.core.publisher.BaseSubscriber;
|
import reactor.core.publisher.BaseSubscriber;
|
||||||
import reactor.core.publisher.FluxSink;
|
import reactor.core.publisher.FluxSink;
|
||||||
|
|
||||||
|
|
@ -135,6 +136,7 @@ class ResponseSubscribers {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected void hookOnNext(String line) {
|
protected void hookOnNext(String line) {
|
||||||
|
|
||||||
if (line.isEmpty()) {
|
if (line.isEmpty()) {
|
||||||
// Empty line means end of event
|
// Empty line means end of event
|
||||||
if (this.eventBuilder.length() > 0) {
|
if (this.eventBuilder.length() > 0) {
|
||||||
|
|
@ -164,6 +166,13 @@ class ResponseSubscribers {
|
||||||
this.currentEventType.set(matcher.group(1).trim());
|
this.currentEventType.set(matcher.group(1).trim());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
else {
|
||||||
|
// If the response is not successful, emit an error
|
||||||
|
// TODO: This should be a McpTransportError
|
||||||
|
this.sink.error(new McpError(
|
||||||
|
"Invalid SSE response. Status code: " + this.responseInfo.statusCode() + " Line: " + line));
|
||||||
|
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -69,6 +69,9 @@ public final class Utils {
|
||||||
* base URL or URI is malformed
|
* base URL or URI is malformed
|
||||||
*/
|
*/
|
||||||
public static URI resolveUri(URI baseUrl, String endpointUrl) {
|
public static URI resolveUri(URI baseUrl, String endpointUrl) {
|
||||||
|
if (!Utils.hasText(endpointUrl)) {
|
||||||
|
return baseUrl;
|
||||||
|
}
|
||||||
URI endpointUri = URI.create(endpointUrl);
|
URI endpointUri = URI.create(endpointUrl);
|
||||||
if (endpointUri.isAbsolute() && !isUnderBaseUri(baseUrl, endpointUri)) {
|
if (endpointUri.isAbsolute() && !isUnderBaseUri(baseUrl, endpointUri)) {
|
||||||
throw new IllegalArgumentException("Absolute endpoint URL does not match the base URL.");
|
throw new IllegalArgumentException("Absolute endpoint URL does not match the base URL.");
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue