Close response after sending error in McpStreamableServerSession#responseStream (#1041)
Closes #1040 Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
This commit is contained in:
parent
f7d9a64b3c
commit
ff78191ebd
2 changed files with 55 additions and 3 deletions
|
|
@ -214,9 +214,13 @@ public class McpStreamableServerSession implements McpLoggableSession {
|
||||||
// (sink)
|
// (sink)
|
||||||
if (requestHandler == null) {
|
if (requestHandler == null) {
|
||||||
MethodNotFoundError error = getMethodNotFoundError(jsonrpcRequest.method());
|
MethodNotFoundError error = getMethodNotFoundError(jsonrpcRequest.method());
|
||||||
return transport.sendMessage(
|
return transport
|
||||||
McpSchema.JSONRPCResponse.error(jsonrpcRequest.id(), new McpSchema.JSONRPCResponse.JSONRPCError(
|
.sendMessage(
|
||||||
McpSchema.ErrorCodes.METHOD_NOT_FOUND, error.message(), error.data())));
|
McpSchema.JSONRPCResponse
|
||||||
|
.error(jsonrpcRequest.id(),
|
||||||
|
new McpSchema.JSONRPCResponse.JSONRPCError(
|
||||||
|
McpSchema.ErrorCodes.METHOD_NOT_FOUND, error.message(), error.data())))
|
||||||
|
.then(transport.closeGracefully());
|
||||||
}
|
}
|
||||||
return requestHandler
|
return requestHandler
|
||||||
.handle(new McpAsyncServerExchange(this.id, stream, clientCapabilities.get(), clientInfo.get(),
|
.handle(new McpAsyncServerExchange(this.id, stream, clientCapabilities.get(), clientInfo.get(),
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,8 @@ package io.modelcontextprotocol.server;
|
||||||
|
|
||||||
import java.time.Duration;
|
import java.time.Duration;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
|
import java.util.function.Function;
|
||||||
import java.util.stream.Stream;
|
import java.util.stream.Stream;
|
||||||
|
|
||||||
import io.modelcontextprotocol.AbstractMcpClientServerIntegrationTests;
|
import io.modelcontextprotocol.AbstractMcpClientServerIntegrationTests;
|
||||||
|
|
@ -16,14 +18,19 @@ import io.modelcontextprotocol.server.McpServer.AsyncSpecification;
|
||||||
import io.modelcontextprotocol.server.McpServer.SyncSpecification;
|
import io.modelcontextprotocol.server.McpServer.SyncSpecification;
|
||||||
import io.modelcontextprotocol.server.transport.HttpServletStreamableServerTransportProvider;
|
import io.modelcontextprotocol.server.transport.HttpServletStreamableServerTransportProvider;
|
||||||
import io.modelcontextprotocol.server.transport.TomcatTestUtil;
|
import io.modelcontextprotocol.server.transport.TomcatTestUtil;
|
||||||
|
import io.modelcontextprotocol.spec.McpSchema;
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
import jakarta.servlet.http.HttpServletRequest;
|
||||||
import org.apache.catalina.LifecycleException;
|
import org.apache.catalina.LifecycleException;
|
||||||
import org.apache.catalina.LifecycleState;
|
import org.apache.catalina.LifecycleState;
|
||||||
import org.apache.catalina.startup.Tomcat;
|
import org.apache.catalina.startup.Tomcat;
|
||||||
|
import org.awaitility.Awaitility;
|
||||||
import org.junit.jupiter.api.AfterEach;
|
import org.junit.jupiter.api.AfterEach;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.Timeout;
|
import org.junit.jupiter.api.Timeout;
|
||||||
import org.junit.jupiter.params.provider.Arguments;
|
import org.junit.jupiter.params.provider.Arguments;
|
||||||
|
import reactor.core.publisher.Mono;
|
||||||
|
import reactor.test.StepVerifier;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
|
||||||
|
|
@ -96,6 +103,47 @@ class HttpServletStreamableIntegrationTests extends AbstractMcpClientServerInteg
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void testMissingHandlerReturnsMethodNotFoundError() {
|
||||||
|
var mcpServer = prepareAsyncServerBuilder().serverInfo("test-server", "1.0.0")
|
||||||
|
.capabilities(McpSchema.ServerCapabilities.builder().tools(true).build())
|
||||||
|
.build();
|
||||||
|
var clientTransport = HttpClientStreamableHttpTransport.builder("http://localhost:" + PORT)
|
||||||
|
.endpoint(MESSAGE_ENDPOINT)
|
||||||
|
.build();
|
||||||
|
|
||||||
|
try (var mcpClient = McpClient.sync(clientTransport).build()) {
|
||||||
|
// Create a session using an MCP client
|
||||||
|
McpSchema.InitializeResult initResult = mcpClient.initialize();
|
||||||
|
assertThat(initResult).isNotNull();
|
||||||
|
|
||||||
|
// Override the response handler in the client to capture responses
|
||||||
|
AtomicReference<McpSchema.JSONRPCResponse> response = new AtomicReference<>();
|
||||||
|
var handler = (Function<Mono<McpSchema.JSONRPCMessage>, Mono<McpSchema.JSONRPCMessage>>) (
|
||||||
|
message) -> message.doOnNext(r -> {
|
||||||
|
if (r instanceof McpSchema.JSONRPCResponse resp) {
|
||||||
|
response.set(resp);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
StepVerifier.create(clientTransport.connect(handler)).verifyComplete();
|
||||||
|
|
||||||
|
// Send an incorrect request through the transport
|
||||||
|
StepVerifier
|
||||||
|
.create(clientTransport.sendMessage(new McpSchema.JSONRPCRequest("foo/bar", "test-request-123")))
|
||||||
|
.verifyComplete();
|
||||||
|
|
||||||
|
// Wait until we've received the response
|
||||||
|
Awaitility.await().atMost(Duration.ofSeconds(1)).until(() -> response.get() != null);
|
||||||
|
|
||||||
|
assertThat(response.get().error().code()).isEqualTo(McpSchema.ErrorCodes.METHOD_NOT_FOUND);
|
||||||
|
assertThat(response.get().error().message()).isEqualTo("Method not found: foo/bar");
|
||||||
|
}
|
||||||
|
finally {
|
||||||
|
mcpServer.close();
|
||||||
|
}
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
static McpTransportContextExtractor<HttpServletRequest> TEST_CONTEXT_EXTRACTOR = (r) -> McpTransportContext
|
static McpTransportContextExtractor<HttpServletRequest> TEST_CONTEXT_EXTRACTOR = (r) -> McpTransportContext
|
||||||
.create(Map.of("important", "value"));
|
.create(Map.of("important", "value"));
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue