From 7e3f76ef7505f2d685e07cfd2c5ab9be474270e2 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sun, 7 Sep 2025 01:01:52 +0200 Subject: [PATCH 1/3] fix: align tool error handling with MCP specification Implement proper error handling for tool calls according to MCP Tools' Error Handling specification (https://modelcontextprotocol.io/specification/2025-06-18/server/tools#error-handling) - Tool errors are now returned within CallToolResult with isError=true instead of throwing MCP protocol-level errors - This allows LLMs to see and handle tool errors as part of normal flow - Return proper JSON-RPC error with code -32602 in case of "Unknown tools" - Added Utils.findRootCause() helper to extract root cause from exception chains - Skip output schema validation when tool result already indicates an error - Updated all integration tests to verify new error handling behavior BREAKING CHANGE: Tool call errors are no longer thrown as McpError exceptions. They are now returned as CallToolResult objects with isError=true and error details in the content field, following MCP specification guidelines. Resolves #538 Related to #422 Signed-off-by: Christian Tzolov --- ...stractMcpClientServerIntegrationTests.java | 82 +++++++++++------- .../server/McpAsyncServer.java | 27 +++++- .../server/McpStatelessAsyncServer.java | 5 ++ .../io/modelcontextprotocol/util/Utils.java | 10 +++ ...stractMcpClientServerIntegrationTests.java | 84 +++++++++++-------- .../HttpServletSseIntegrationTests.java | 20 ++--- ...HttpServletStreamableIntegrationTests.java | 20 ++--- 7 files changed, 157 insertions(+), 91 deletions(-) diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java index c0e2509c9..125dccb62 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java @@ -4,14 +4,6 @@ package io.modelcontextprotocol; -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertWith; -import static org.awaitility.Awaitility.await; -import static org.mockito.Mockito.mock; - import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; @@ -29,9 +21,6 @@ import java.util.function.Function; import java.util.stream.Collectors; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.common.McpTransportContext; import io.modelcontextprotocol.server.McpServer; @@ -59,9 +48,18 @@ import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.util.Utils; import net.javacrumbs.jsonunit.core.Option; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertWith; +import static org.awaitility.Awaitility.await; +import static org.mockito.Mockito.mock; + public abstract class AbstractMcpClientServerIntegrationTests { protected ConcurrentHashMap clientBuilders = new ConcurrentHashMap<>(); @@ -108,8 +106,8 @@ void testCreateMessageWithoutSamplingCapabilities(String clientType) { McpServerFeatures.AsyncToolSpecification tool = McpServerFeatures.AsyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(emptyJsonSchema).build()) .callHandler((exchange, request) -> { - exchange.createMessage(mock(McpSchema.CreateMessageRequest.class)).block(); - return Mono.just(mock(CallToolResult.class)); + return exchange.createMessage(mock(McpSchema.CreateMessageRequest.class)) + .then(Mono.just(mock(CallToolResult.class))); }) .build(); @@ -122,13 +120,15 @@ void testCreateMessageWithoutSamplingCapabilities(String clientType) { assertThat(client.initialize()).isNotNull(); - try { - client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - } - catch (McpError e) { - assertThat(e).isInstanceOf(McpError.class) - .hasMessage("Client must be configured with sampling capabilities"); - } + McpSchema.CallToolResult callToolResult = client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Client must be configured with sampling capabilities")); } finally { server.closeGracefully().block(); @@ -338,9 +338,16 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - assertThatExceptionOfType(McpError.class).isThrownBy(() -> { - mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - }).withMessageContaining("1000ms"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); } finally { mcpServer.closeGracefully().block(); @@ -556,9 +563,16 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - assertThatExceptionOfType(McpError.class).isThrownBy(() -> { - mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - }).withMessageContaining("within 1000ms"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); ElicitResult elicitResult = resultRef.get(); assertThat(elicitResult).isNull(); @@ -842,12 +856,16 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - // We expect the tool call to fail immediately with the exception raised by - // the offending tool - // instead of getting back a timeout. - assertThatExceptionOfType(McpError.class) - .isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of()))) - .withMessageContaining("Timeout on blocking read"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS")); } finally { mcpServer.closeGracefully(); diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java index dc81e65a8..6498f9007 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java @@ -29,10 +29,12 @@ import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse; import io.modelcontextprotocol.spec.McpSchema.LoggingLevel; import io.modelcontextprotocol.spec.McpSchema.LoggingMessageNotification; import io.modelcontextprotocol.spec.McpSchema.ResourceTemplate; import io.modelcontextprotocol.spec.McpSchema.SetLevelRequest; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpServerSession; import io.modelcontextprotocol.spec.McpServerTransportProvider; @@ -376,6 +378,11 @@ public Mono apply(McpAsyncServerExchange exchange, McpSchema.Cal return this.delegateCallToolResult.apply(exchange, request).map(result -> { + if (result.isError() != null && result.isError()) { + // If the tool call resulted in an error, skip further validation + return result; + } + if (outputSchema == null) { if (result.structuredContent() != null) { logger.warn( @@ -507,11 +514,23 @@ private McpRequestHandler toolsCallRequestHandler() { .findAny(); if (toolSpecification.isEmpty()) { - return Mono.error(new McpError("Tool not found: " + callToolRequest.name())); + return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS, + "Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name()))); + } + else { + return toolSpecification.get().callHandler().apply(exchange, callToolRequest).onErrorResume(error -> { + logger.error("Error calling tool: {}", callToolRequest.name(), error); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + return Mono.just(CallToolResult.builder() + .isError(true) + .content(List + .of(new TextContent("Error calling tool: " + Utils.findRootCause(error).getMessage()))) + .build()); + }); } - - return toolSpecification.map(tool -> Mono.defer(() -> tool.callHandler().apply(exchange, callToolRequest))) - .orElse(Mono.error(new McpError("Tool not found: " + callToolRequest.name()))); }; } diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java index 451771295..d249e3d8d 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java @@ -249,6 +249,11 @@ public Mono apply(McpTransportContext transportContext, McpSchem return this.delegateHandler.apply(transportContext, request).map(result -> { + if (result.isError() != null && result.isError()) { + // If the tool call resulted in an error, skip further validation + return result; + } + if (outputSchema == null) { if (result.structuredContent() != null) { logger.warn( diff --git a/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java b/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java index 039b0d68e..4342256f5 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java +++ b/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java @@ -9,6 +9,7 @@ import java.net.URI; import java.util.Collection; import java.util.Map; +import java.util.Objects; /** * Miscellaneous utility methods. @@ -107,4 +108,13 @@ private static boolean isUnderBaseUri(URI baseUri, URI endpointUri) { return endpointPath.startsWith(basePath); } + public static Throwable findRootCause(Throwable throwable) { + Objects.requireNonNull(throwable); + Throwable rootCause = throwable; + while (rootCause.getCause() != null && rootCause.getCause() != rootCause) { + rootCause = rootCause.getCause(); + } + return rootCause; + } + } diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java index 2e1a12a95..6079a9e00 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java @@ -4,15 +4,6 @@ package io.modelcontextprotocol.server; -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertWith; -import static org.awaitility.Awaitility.await; -import static org.mockito.Mockito.mock; - -import io.modelcontextprotocol.common.McpTransportContext; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; @@ -30,10 +21,8 @@ import java.util.function.Function; import java.util.stream.Collectors; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - import io.modelcontextprotocol.client.McpClient; +import io.modelcontextprotocol.common.McpTransportContext; import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; @@ -55,9 +44,18 @@ import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.util.Utils; import net.javacrumbs.jsonunit.core.Option; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertWith; +import static org.awaitility.Awaitility.await; +import static org.mockito.Mockito.mock; + public abstract class AbstractMcpClientServerIntegrationTests { protected ConcurrentHashMap clientBuilders = new ConcurrentHashMap<>(); @@ -104,8 +102,8 @@ void testCreateMessageWithoutSamplingCapabilities(String clientType) { McpServerFeatures.AsyncToolSpecification tool = McpServerFeatures.AsyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(emptyJsonSchema).build()) .callHandler((exchange, request) -> { - exchange.createMessage(mock(McpSchema.CreateMessageRequest.class)).block(); - return Mono.just(mock(CallToolResult.class)); + return exchange.createMessage(mock(McpSchema.CreateMessageRequest.class)) + .then(Mono.just(mock(CallToolResult.class))); }) .build(); @@ -118,13 +116,15 @@ void testCreateMessageWithoutSamplingCapabilities(String clientType) { assertThat(client.initialize()).isNotNull(); - try { - client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - } - catch (McpError e) { - assertThat(e).isInstanceOf(McpError.class) - .hasMessage("Client must be configured with sampling capabilities"); - } + McpSchema.CallToolResult callToolResult = client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Client must be configured with sampling capabilities")); } finally { server.closeGracefully().block(); @@ -334,9 +334,16 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - assertThatExceptionOfType(McpError.class).isThrownBy(() -> { - mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - }).withMessageContaining("1000ms"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); } finally { mcpServer.closeGracefully().block(); @@ -552,9 +559,16 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - assertThatExceptionOfType(McpError.class).isThrownBy(() -> { - mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - }).withMessageContaining("within 1000ms"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); ElicitResult elicitResult = resultRef.get(); assertThat(elicitResult).isNull(); @@ -838,12 +852,16 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - // We expect the tool call to fail immediately with the exception raised by - // the offending tool - // instead of getting back a timeout. - assertThatExceptionOfType(McpError.class) - .isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of()))) - .withMessageContaining("Timeout on blocking read"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS")); } finally { mcpServer.closeGracefully(); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java index c893acf9a..8e618b9a8 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletSseIntegrationTests.java @@ -4,28 +4,26 @@ package io.modelcontextprotocol.server; -import static org.assertj.core.api.Assertions.assertThat; - -import io.modelcontextprotocol.common.McpTransportContext; import java.time.Duration; import java.util.Map; -import org.apache.catalina.LifecycleException; -import org.apache.catalina.LifecycleState; -import org.apache.catalina.startup.Tomcat; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Timeout; - import com.fasterxml.jackson.databind.ObjectMapper; - import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.client.transport.HttpClientSseClientTransport; +import io.modelcontextprotocol.common.McpTransportContext; import io.modelcontextprotocol.server.McpServer.AsyncSpecification; import io.modelcontextprotocol.server.McpServer.SyncSpecification; import io.modelcontextprotocol.server.transport.HttpServletSseServerTransportProvider; import io.modelcontextprotocol.server.transport.TomcatTestUtil; import jakarta.servlet.http.HttpServletRequest; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.LifecycleState; +import org.apache.catalina.startup.Tomcat; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Timeout; + +import static org.assertj.core.api.Assertions.assertThat; @Timeout(15) class HttpServletSseIntegrationTests extends AbstractMcpClientServerIntegrationTests { diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java index 6899ba474..1f6a1fe58 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStreamableIntegrationTests.java @@ -4,28 +4,26 @@ package io.modelcontextprotocol.server; -import static org.assertj.core.api.Assertions.assertThat; - -import io.modelcontextprotocol.common.McpTransportContext; import java.time.Duration; import java.util.Map; -import org.apache.catalina.LifecycleException; -import org.apache.catalina.LifecycleState; -import org.apache.catalina.startup.Tomcat; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Timeout; - import com.fasterxml.jackson.databind.ObjectMapper; - import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport; +import io.modelcontextprotocol.common.McpTransportContext; import io.modelcontextprotocol.server.McpServer.AsyncSpecification; import io.modelcontextprotocol.server.McpServer.SyncSpecification; import io.modelcontextprotocol.server.transport.HttpServletStreamableServerTransportProvider; import io.modelcontextprotocol.server.transport.TomcatTestUtil; import jakarta.servlet.http.HttpServletRequest; +import org.apache.catalina.LifecycleException; +import org.apache.catalina.LifecycleState; +import org.apache.catalina.startup.Tomcat; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Timeout; + +import static org.assertj.core.api.Assertions.assertThat; @Timeout(15) class HttpServletStreamableIntegrationTests extends AbstractMcpClientServerIntegrationTests { From 346d767ae94e90b514cd39b5510113b8f30b0a23 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sun, 7 Sep 2025 18:16:22 +0200 Subject: [PATCH 2/3] fix: Return tool errors as CallToolResult instead of throwing exceptions - Changed tool error handling to return CallToolResult with isError=true - Tool errors are now reported within the result object, not as MCP protocol-level errors - This allows LLMs to see and potentially handle errors gracefully - Added comprehensive tests for in-handler error scenarios - Added JavaDoc for Utils.findRootCause() method - Updated existing timeout test to expect CallToolResult instead of exception BREAKING CHANGE: Tool call errors no longer throw McpError exceptions but return error results instead. Clients should check CallToolResult.isError() to handle errors. Signed-off-by: Christian Tzolov --- ...stractMcpClientServerIntegrationTests.java | 56 ++++++++ .../AbstractStatelessIntegrationTests.java | 90 ++++++++++--- .../server/McpStatelessAsyncServer.java | 28 +++- .../io/modelcontextprotocol/util/Utils.java | 6 + ...stractMcpClientServerIntegrationTests.java | 56 ++++++++ .../HttpServletStatelessIntegrationTests.java | 121 +++++++++++++++++- 6 files changed, 337 insertions(+), 20 deletions(-) diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java index 125dccb62..9c7057219 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java @@ -1452,6 +1452,62 @@ void testStructuredOutputValidationSuccess(String clientType) { @ParameterizedTest(name = "{0} : {displayName} ") @ValueSource(strings = { "httpclient", "webflux" }) + void testStructuredOutputWithInHandlerError(String clientType) { + var clientBuilder = clientBuilders.get(clientType); + + // Create a tool with output schema + Map outputSchema = Map.of( + "type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation", + Map.of("type", "string"), "timestamp", Map.of("type", "string")), + "required", List.of("result", "operation")); + + Tool calculatorTool = Tool.builder() + .name("calculator") + .description("Performs mathematical calculations") + .outputSchema(outputSchema) + .build(); + + // Handler that throws an exception to simulate an error + McpServerFeatures.SyncToolSpecification tool = McpServerFeatures.SyncToolSpecification.builder() + .tool(calculatorTool) + .callHandler((exchange, request) -> { + throw new RuntimeException("Simulated in-handler error"); + }) + .build(); + + var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .tools(tool) + .build(); + + try (var mcpClient = clientBuilder.build()) { + InitializeResult initResult = mcpClient.initialize(); + assertThat(initResult).isNotNull(); + + // Verify tool is listed with output schema + var toolsList = mcpClient.listTools(); + assertThat(toolsList.tools()).hasSize(1); + assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator"); + // Note: outputSchema might be null in sync server, but validation still works + + // Call tool with valid structured output + CallToolResult response = mcpClient + .callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3"))); + + assertThat(response).isNotNull(); + assertThat(response.isError()).isTrue(); + assertThat(response.content()).isNotEmpty(); + assertThat(response.content()) + .containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error")); + assertThat(response.structuredContent()).isNull(); + } + finally { + mcpServer.closeGracefully(); + } + } + + @ParameterizedTest(name = "{0} : {displayName} ") + @ValueSource(strings = { "httpclient" }) void testStructuredOutputValidationFailure(String clientType) { var clientBuilder = clientBuilders.get(clientType); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java index 777e12a9c..a001b0ec3 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java @@ -4,12 +4,6 @@ package io.modelcontextprotocol; -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.awaitility.Awaitility.await; - import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; @@ -20,23 +14,26 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.server.McpServer.StatelessAsyncSpecification; import io.modelcontextprotocol.server.McpServer.StatelessSyncSpecification; import io.modelcontextprotocol.server.McpStatelessServerFeatures; import io.modelcontextprotocol.server.McpStatelessSyncServer; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import io.modelcontextprotocol.spec.McpSchema.InitializeResult; import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; import io.modelcontextprotocol.spec.McpSchema.Tool; import net.javacrumbs.jsonunit.core.Option; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import reactor.core.publisher.Mono; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; +import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + public abstract class AbstractStatelessIntegrationTests { protected ConcurrentHashMap clientBuilders = new ConcurrentHashMap<>(); @@ -159,12 +156,16 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - // We expect the tool call to fail immediately with the exception raised by - // the offending tool - // instead of getting back a timeout. - assertThatExceptionOfType(McpError.class) - .isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of()))) - .withMessageContaining("Timeout on blocking read"); + McpSchema.CallToolResult callToolResult = mcpClient + .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( + "Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS")); } finally { mcpServer.closeGracefully(); @@ -350,6 +351,63 @@ void testStructuredOutputValidationSuccess(String clientType) { } } + @ParameterizedTest(name = "{0} : {displayName} ") + @ValueSource(strings = { "httpclient", "webflux" }) + void testStructuredOutputWithInHandlerError(String clientType) { + var clientBuilder = clientBuilders.get(clientType); + + // Create a tool with output schema + Map outputSchema = Map.of( + "type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation", + Map.of("type", "string"), "timestamp", Map.of("type", "string")), + "required", List.of("result", "operation")); + + Tool calculatorTool = Tool.builder() + .name("calculator") + .description("Performs mathematical calculations") + .outputSchema(outputSchema) + .build(); + + // Handler that throws an exception to simulate an error + McpStatelessServerFeatures.SyncToolSpecification tool = McpStatelessServerFeatures.SyncToolSpecification + .builder() + .tool(calculatorTool) + .callHandler((exchange, request) -> { + throw new RuntimeException("Simulated in-handler error"); + }) + .build(); + + var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .tools(tool) + .build(); + + try (var mcpClient = clientBuilder.build()) { + InitializeResult initResult = mcpClient.initialize(); + assertThat(initResult).isNotNull(); + + // Verify tool is listed with output schema + var toolsList = mcpClient.listTools(); + assertThat(toolsList.tools()).hasSize(1); + assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator"); + // Note: outputSchema might be null in sync server, but validation still works + + // Call tool with valid structured output + CallToolResult response = mcpClient + .callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3"))); + + assertThat(response).isNotNull(); + assertThat(response.isError()).isTrue(); + assertThat(response.content()).isNotEmpty(); + assertThat(response.content()) + .containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error")); + assertThat(response.structuredContent()).isNull(); + } + finally { + mcpServer.closeGracefully(); + } + } + @ParameterizedTest(name = "{0} : {displayName} ") @ValueSource(strings = { "httpclient", "webflux" }) void testStructuredOutputValidationFailure(String clientType) { diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java index d249e3d8d..208199b9f 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java @@ -11,7 +11,9 @@ import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; +import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse; import io.modelcontextprotocol.spec.McpSchema.ResourceTemplate; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpStatelessServerTransport; import io.modelcontextprotocol.util.Assert; @@ -380,11 +382,31 @@ private McpStatelessRequestHandler toolsCallRequestHandler() { .findAny(); if (toolSpecification.isEmpty()) { - return Mono.error(new McpError("Tool not found: " + callToolRequest.name())); + return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS, + "Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name()))); } + else { + return toolSpecification.get().callHandler().apply(ctx, callToolRequest).onErrorResume(error -> { + logger.error("Error calling tool: {}", callToolRequest.name(), error); + + // TODO: Should we handle the McpError+JsonRcpError specaially (e.g. + // propagate) + // or always return a CallToolResult with isError=true? + if (error instanceof McpError mcpError && mcpError.getJsonRpcError() != null) { + // If the error is already an MCP error, propagate it as is + return Mono.error(mcpError); + } - return toolSpecification.map(tool -> tool.callHandler().apply(ctx, callToolRequest)) - .orElse(Mono.error(new McpError("Tool not found: " + callToolRequest.name()))); + // Tool errors should be reported within the result object, not as MCP + // protocol-level errors. This allows the LLM to see and potentially + // handle the error. + return Mono.just(CallToolResult.builder() + .isError(true) + .content(List + .of(new TextContent("Error calling tool: " + Utils.findRootCause(error).getMessage()))) + .build()); + }); + } }; } diff --git a/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java b/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java index 4342256f5..00e091bd5 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java +++ b/mcp/src/main/java/io/modelcontextprotocol/util/Utils.java @@ -108,6 +108,12 @@ private static boolean isUnderBaseUri(URI baseUri, URI endpointUri) { return endpointPath.startsWith(basePath); } + /** + * Finds the root cause of the given throwable by traversing the cause chain. + * @param throwable The throwable to analyze + * @return The root cause throwable + * @throws NullPointerException if the provided throwable is null + */ public static Throwable findRootCause(Throwable throwable) { Objects.requireNonNull(throwable); Throwable rootCause = throwable; diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java index 6079a9e00..833b0ab08 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java @@ -1446,6 +1446,62 @@ void testStructuredOutputValidationSuccess(String clientType) { } } + @ParameterizedTest(name = "{0} : {displayName} ") + @ValueSource(strings = { "httpclient" }) + void testStructuredOutputWithInHandlerError(String clientType) { + var clientBuilder = clientBuilders.get(clientType); + + // Create a tool with output schema + Map outputSchema = Map.of( + "type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation", + Map.of("type", "string"), "timestamp", Map.of("type", "string")), + "required", List.of("result", "operation")); + + Tool calculatorTool = Tool.builder() + .name("calculator") + .description("Performs mathematical calculations") + .outputSchema(outputSchema) + .build(); + + // Handler that throws an exception to simulate an error + McpServerFeatures.SyncToolSpecification tool = McpServerFeatures.SyncToolSpecification.builder() + .tool(calculatorTool) + .callHandler((exchange, request) -> { + throw new RuntimeException("Simulated in-handler error"); + }) + .build(); + + var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .tools(tool) + .build(); + + try (var mcpClient = clientBuilder.build()) { + InitializeResult initResult = mcpClient.initialize(); + assertThat(initResult).isNotNull(); + + // Verify tool is listed with output schema + var toolsList = mcpClient.listTools(); + assertThat(toolsList.tools()).hasSize(1); + assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator"); + // Note: outputSchema might be null in sync server, but validation still works + + // Call tool with valid structured output + CallToolResult response = mcpClient + .callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3"))); + + assertThat(response).isNotNull(); + assertThat(response.isError()).isTrue(); + assertThat(response.content()).isNotEmpty(); + assertThat(response.content()) + .containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error")); + assertThat(response.structuredContent()).isNull(); + } + finally { + mcpServer.closeGracefully(); + } + } + @ParameterizedTest(name = "{0} : {displayName} ") @ValueSource(strings = { "httpclient" }) void testStructuredOutputValidationFailure(String clientType) { diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java index bb405b728..3b5c2d03e 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java @@ -298,6 +298,64 @@ void testStructuredOutputValidationSuccess(String clientType) { } } + @ParameterizedTest(name = "{0} : {displayName} ") + @ValueSource(strings = { "httpclient" }) + void testStructuredOutputWithInHandlerError(String clientType) { + var clientBuilder = clientBuilders.get(clientType); + + // Create a tool with output schema + Map outputSchema = Map.of( + "type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation", + Map.of("type", "string"), "timestamp", Map.of("type", "string")), + "required", List.of("result", "operation")); + + Tool calculatorTool = Tool.builder() + .name("calculator") + .description("Performs mathematical calculations") + .outputSchema(outputSchema) + .build(); + + // Handler that throws an exception to simulate an error + McpStatelessServerFeatures.SyncToolSpecification tool = McpStatelessServerFeatures.SyncToolSpecification + .builder() + .tool(calculatorTool) + .callHandler((exchange, request) -> { + throw new RuntimeException("Simulated in-handler error"); + }) + .build(); + + var mcpServer = McpServer.sync(mcpStatelessServerTransport) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .tools(tool) + .build(); + + try (var mcpClient = clientBuilder.build()) { + InitializeResult initResult = mcpClient.initialize(); + assertThat(initResult).isNotNull(); + + // Verify tool is listed with output schema + var toolsList = mcpClient.listTools(); + assertThat(toolsList.tools()).hasSize(1); + assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator"); + // Note: outputSchema might be null in sync server, but validation still works + + // Call tool with valid structured output + CallToolResult response = mcpClient + .callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3"))); + + assertThat(response).isNotNull(); + assertThat(response.isError()).isTrue(); + assertThat(response.content()).isNotEmpty(); + assertThat(response.content()) + .containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error")); + assertThat(response.structuredContent()).isNull(); + } + finally { + mcpServer.closeGracefully(); + } + } + @ParameterizedTest(name = "{0} : {displayName} ") @ValueSource(strings = { "httpclient" }) void testStructuredOutputValidationFailure(String clientType) { @@ -477,7 +535,57 @@ void testStructuredOutputRuntimeToolAddition(String clientType) { } @Test - void testThrownMcpError() throws Exception { + void testThrownMcpErrorAndJsonRpcError() throws Exception { + var mcpServer = McpServer.sync(mcpStatelessServerTransport) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .build(); + + Tool testTool = Tool.builder().name("test").description("test").build(); + + McpStatelessServerFeatures.SyncToolSpecification toolSpec = new McpStatelessServerFeatures.SyncToolSpecification( + testTool, (transportContext, request) -> { + throw new RuntimeException("testing"); + }); + + mcpServer.addTool(toolSpec); + + McpSchema.CallToolRequest callToolRequest = new McpSchema.CallToolRequest("test", Map.of()); + McpSchema.JSONRPCRequest jsonrpcRequest = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, + McpSchema.METHOD_TOOLS_CALL, "test", callToolRequest); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", CUSTOM_MESSAGE_ENDPOINT); + MockHttpServletResponse response = new MockHttpServletResponse(); + + byte[] content = new ObjectMapper().writeValueAsBytes(jsonrpcRequest); + request.setContent(content); + request.addHeader("Content-Type", "application/json"); + request.addHeader("Content-Length", Integer.toString(content.length)); + request.addHeader("Content-Length", Integer.toString(content.length)); + request.addHeader("Accept", APPLICATION_JSON + ", " + TEXT_EVENT_STREAM); + request.addHeader("Content-Type", APPLICATION_JSON); + request.addHeader("Cache-Control", "no-cache"); + request.addHeader(HttpHeaders.PROTOCOL_VERSION, ProtocolVersions.MCP_2025_03_26); + + mcpStatelessServerTransport.service(request, response); + + McpSchema.JSONRPCResponse jsonrpcResponse = new ObjectMapper().readValue(response.getContentAsByteArray(), + McpSchema.JSONRPCResponse.class); + + CallToolResult callToolResult = new ObjectMapper().convertValue(jsonrpcResponse.result(), CallToolResult.class); + + assertThat(callToolResult).isNotNull(); + assertThat(callToolResult.isError()).isTrue(); + assertThat(callToolResult.content()).isNotEmpty(); + assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent("Error calling tool: testing")); + assertThat(callToolResult.structuredContent()).isNull(); + + mcpServer.close(); + } + + @Test + @Timeout(15000) + void testThrownNonMcpError() throws Exception { var mcpServer = McpServer.sync(mcpStatelessServerTransport) .serverInfo("test-server", "1.0.0") .capabilities(ServerCapabilities.builder().tools(true).build()) @@ -508,11 +616,22 @@ void testThrownMcpError() throws Exception { request.addHeader("Content-Type", APPLICATION_JSON); request.addHeader("Cache-Control", "no-cache"); request.addHeader(HttpHeaders.PROTOCOL_VERSION, ProtocolVersions.MCP_2025_03_26); + mcpStatelessServerTransport.service(request, response); McpSchema.JSONRPCResponse jsonrpcResponse = new ObjectMapper().readValue(response.getContentAsByteArray(), McpSchema.JSONRPCResponse.class); + // CallToolResult callToolResult = new + // ObjectMapper().convertValue(jsonrpcResponse.result(), CallToolResult.class); + + // assertThat(callToolResult).isNotNull(); + // assertThat(callToolResult.isError()).isTrue(); + // assertThat(callToolResult.content()).isNotEmpty(); + // assertThat(callToolResult.content()) + // .containsExactly(new McpSchema.TextContent("Error calling tool: testing")); + // assertThat(callToolResult.structuredContent()).isNull(); + assertThat(jsonrpcResponse.error()) .isEqualTo(new McpSchema.JSONRPCResponse.JSONRPCError(12345, "testing", Map.of("a", "b"))); From f59dbaf0293759c1b18ee29212295f2c9b847329 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Mon, 8 Sep 2025 12:37:21 +0200 Subject: [PATCH 3/3] fix: Skip structured output validation for error tool results - Add validation bypass when CallToolResult.isError() is true in async/stateless servers - Fix async tool handler chaining to properly use then() instead of block() - Add comprehensive tests for structured output with in-handler errors - Improve error handling to use proper JSON-RPC error codes for unknown tools - Add findRootCause utility method for better error diagnostics - Increase test timeouts for stability in StdioMcp client tests This ensures that when a tool handler returns an error result, the structured output schema validation is skipped, preventing validation failures on error responses that don't conform to the expected output schema. Signed-off-by: Christian Tzolov --- ...stractMcpClientServerIntegrationTests.java | 66 ++++++------- .../AbstractStatelessIntegrationTests.java | 26 +++--- .../server/McpAsyncServer.java | 16 +--- .../server/McpStatelessAsyncServer.java | 22 +---- .../client/StdioMcpAsyncClientTests.java | 7 +- .../client/StdioMcpSyncClientTests.java | 7 +- ...stractMcpClientServerIntegrationTests.java | 69 ++++++-------- .../HttpServletStatelessIntegrationTests.java | 93 ++++--------------- 8 files changed, 99 insertions(+), 207 deletions(-) diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java index 9c7057219..dd3bc59da 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java @@ -45,6 +45,7 @@ import io.modelcontextprotocol.spec.McpSchema.Role; import io.modelcontextprotocol.spec.McpSchema.Root; import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.util.Utils; import net.javacrumbs.jsonunit.core.Option; @@ -56,6 +57,7 @@ import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertWith; import static org.awaitility.Awaitility.await; import static org.mockito.Mockito.mock; @@ -120,15 +122,13 @@ void testCreateMessageWithoutSamplingCapabilities(String clientType) { assertThat(client.initialize()).isNotNull(); - McpSchema.CallToolResult callToolResult = client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Client must be configured with sampling capabilities")); + try { + client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + } + catch (McpError e) { + assertThat(e).isInstanceOf(McpError.class) + .hasMessage("Client must be configured with sampling capabilities"); + } } finally { server.closeGracefully().block(); @@ -338,16 +338,9 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); + assertThatExceptionOfType(McpError.class).isThrownBy(() -> { + mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + }).withMessageContaining("1000ms"); } finally { mcpServer.closeGracefully().block(); @@ -563,16 +556,9 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); + assertThatExceptionOfType(McpError.class).isThrownBy(() -> { + mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + }).withMessageContaining("within 1000ms"); ElicitResult elicitResult = resultRef.get(); assertThat(elicitResult).isNull(); @@ -856,16 +842,12 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS")); + // We expect the tool call to fail immediately with the exception raised by + // the offending tool + // instead of getting back a timeout. + assertThatExceptionOfType(McpError.class) + .isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of()))) + .withMessageContaining("Timeout on blocking read"); } finally { mcpServer.closeGracefully(); @@ -1471,7 +1453,11 @@ void testStructuredOutputWithInHandlerError(String clientType) { McpServerFeatures.SyncToolSpecification tool = McpServerFeatures.SyncToolSpecification.builder() .tool(calculatorTool) .callHandler((exchange, request) -> { - throw new RuntimeException("Simulated in-handler error"); + + return CallToolResult.builder() + .isError(true) + .content(List.of(new TextContent("Error calling tool: Simulated in-handler error"))) + .build(); }) .build(); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java index a001b0ec3..c96f10eda 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java @@ -19,10 +19,12 @@ import io.modelcontextprotocol.server.McpServer.StatelessSyncSpecification; import io.modelcontextprotocol.server.McpStatelessServerFeatures; import io.modelcontextprotocol.server.McpStatelessSyncServer; +import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import io.modelcontextprotocol.spec.McpSchema.InitializeResult; import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import net.javacrumbs.jsonunit.core.Option; import org.junit.jupiter.params.ParameterizedTest; @@ -32,6 +34,7 @@ import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.awaitility.Awaitility.await; public abstract class AbstractStatelessIntegrationTests { @@ -156,16 +159,12 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS")); + // We expect the tool call to fail immediately with the exception raised by + // the offending tool + // instead of getting back a timeout. + assertThatExceptionOfType(McpError.class) + .isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of()))) + .withMessageContaining("Timeout on blocking read"); } finally { mcpServer.closeGracefully(); @@ -372,9 +371,10 @@ void testStructuredOutputWithInHandlerError(String clientType) { McpStatelessServerFeatures.SyncToolSpecification tool = McpStatelessServerFeatures.SyncToolSpecification .builder() .tool(calculatorTool) - .callHandler((exchange, request) -> { - throw new RuntimeException("Simulated in-handler error"); - }) + .callHandler((exchange, request) -> CallToolResult.builder() + .isError(true) + .content(List.of(new TextContent("Error calling tool: Simulated in-handler error"))) + .build()) .build(); var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0") diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java index 6498f9007..3234280a1 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java @@ -517,20 +517,8 @@ private McpRequestHandler toolsCallRequestHandler() { return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS, "Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name()))); } - else { - return toolSpecification.get().callHandler().apply(exchange, callToolRequest).onErrorResume(error -> { - logger.error("Error calling tool: {}", callToolRequest.name(), error); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - return Mono.just(CallToolResult.builder() - .isError(true) - .content(List - .of(new TextContent("Error calling tool: " + Utils.findRootCause(error).getMessage()))) - .build()); - }); - } + + return toolSpecification.get().callHandler().apply(exchange, callToolRequest); }; } diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java index 208199b9f..3aa6b6243 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java @@ -385,28 +385,8 @@ private McpStatelessRequestHandler toolsCallRequestHandler() { return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS, "Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name()))); } - else { - return toolSpecification.get().callHandler().apply(ctx, callToolRequest).onErrorResume(error -> { - logger.error("Error calling tool: {}", callToolRequest.name(), error); - - // TODO: Should we handle the McpError+JsonRcpError specaially (e.g. - // propagate) - // or always return a CallToolResult with isError=true? - if (error instanceof McpError mcpError && mcpError.getJsonRpcError() != null) { - // If the error is already an MCP error, propagate it as is - return Mono.error(mcpError); - } - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - return Mono.just(CallToolResult.builder() - .isError(true) - .content(List - .of(new TextContent("Error calling tool: " + Utils.findRootCause(error).getMessage()))) - .build()); - }); - } + return toolSpecification.get().callHandler().apply(ctx, callToolRequest); }; } diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java index e9356d0c0..c5f25f917 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpAsyncClientTests.java @@ -17,7 +17,7 @@ * @author Christian Tzolov * @author Dariusz Jędrzejczyk */ -@Timeout(15) // Giving extra time beyond the client timeout +@Timeout(25) // Giving extra time beyond the client timeout class StdioMcpAsyncClientTests extends AbstractMcpAsyncClientTests { @Override @@ -40,4 +40,9 @@ protected Duration getInitializationTimeout() { return Duration.ofSeconds(20); } + @Override + protected Duration getRequestTimeout() { + return Duration.ofSeconds(25); + } + } diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java index 4b5f4f9c0..9003c12d0 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java @@ -25,7 +25,7 @@ * @author Christian Tzolov * @author Dariusz Jędrzejczyk */ -@Timeout(15) // Giving extra time beyond the client timeout +@Timeout(25) // Giving extra time beyond the client timeout class StdioMcpSyncClientTests extends AbstractMcpSyncClientTests { @Override @@ -71,4 +71,9 @@ protected Duration getInitializationTimeout() { return Duration.ofSeconds(10); } + @Override + protected Duration getRequestTimeout() { + return Duration.ofSeconds(25); + } + } diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java index 833b0ab08..8dae452f0 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java @@ -41,6 +41,7 @@ import io.modelcontextprotocol.spec.McpSchema.Role; import io.modelcontextprotocol.spec.McpSchema.Root; import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.util.Utils; import net.javacrumbs.jsonunit.core.Option; @@ -52,6 +53,7 @@ import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertWith; import static org.awaitility.Awaitility.await; import static org.mockito.Mockito.mock; @@ -116,15 +118,13 @@ void testCreateMessageWithoutSamplingCapabilities(String clientType) { assertThat(client.initialize()).isNotNull(); - McpSchema.CallToolResult callToolResult = client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Client must be configured with sampling capabilities")); + try { + client.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + } + catch (McpError e) { + assertThat(e).isInstanceOf(McpError.class) + .hasMessage("Client must be configured with sampling capabilities"); + } } finally { server.closeGracefully().block(); @@ -334,16 +334,9 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); + assertThatExceptionOfType(McpError.class).isThrownBy(() -> { + mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + }).withMessageContaining("1000ms"); } finally { mcpServer.closeGracefully().block(); @@ -559,16 +552,9 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Did not observe any item or terminal signal within 1000ms in 'source(MonoCreate)' (and no fallback has been configured)")); + assertThatExceptionOfType(McpError.class).isThrownBy(() -> { + mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())); + }).withMessageContaining("within 1000ms"); ElicitResult elicitResult = resultRef.get(); assertThat(elicitResult).isNull(); @@ -852,16 +838,12 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { InitializeResult initResult = mcpClient.initialize(); assertThat(initResult).isNotNull(); - McpSchema.CallToolResult callToolResult = mcpClient - .callTool(new McpSchema.CallToolRequest("tool1", Map.of())); - - // Tool errors should be reported within the result object, not as MCP - // protocol-level errors. This allows the LLM to see and potentially - // handle the error. - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent( - "Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS")); + // We expect the tool call to fail immediately with the exception raised by + // the offending tool + // instead of getting back a timeout. + assertThatExceptionOfType(McpError.class) + .isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of()))) + .withMessageContaining("Timeout on blocking read"); } finally { mcpServer.closeGracefully(); @@ -1463,12 +1445,13 @@ void testStructuredOutputWithInHandlerError(String clientType) { .outputSchema(outputSchema) .build(); - // Handler that throws an exception to simulate an error + // Handler that returns an error result McpServerFeatures.SyncToolSpecification tool = McpServerFeatures.SyncToolSpecification.builder() .tool(calculatorTool) - .callHandler((exchange, request) -> { - throw new RuntimeException("Simulated in-handler error"); - }) + .callHandler((exchange, request) -> CallToolResult.builder() + .isError(true) + .content(List.of(new TextContent("Error calling tool: Simulated in-handler error"))) + .build()) .build(); var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0") diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java index 3b5c2d03e..5cc7d61be 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/HttpServletStatelessIntegrationTests.java @@ -4,6 +4,13 @@ package io.modelcontextprotocol.server; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiFunction; + import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport; @@ -11,16 +18,17 @@ import io.modelcontextprotocol.server.transport.HttpServletStatelessServerTransport; import io.modelcontextprotocol.server.transport.TomcatTestUtil; import io.modelcontextprotocol.spec.HttpHeaders; -import io.modelcontextprotocol.spec.McpError; import io.modelcontextprotocol.spec.McpSchema; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; import io.modelcontextprotocol.spec.McpSchema.CompleteRequest; import io.modelcontextprotocol.spec.McpSchema.CompleteResult; +import io.modelcontextprotocol.spec.McpSchema.ErrorCodes; import io.modelcontextprotocol.spec.McpSchema.InitializeResult; import io.modelcontextprotocol.spec.McpSchema.Prompt; import io.modelcontextprotocol.spec.McpSchema.PromptArgument; import io.modelcontextprotocol.spec.McpSchema.PromptReference; import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.ProtocolVersions; import net.javacrumbs.jsonunit.core.Option; @@ -33,17 +41,11 @@ import org.junit.jupiter.api.Timeout; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; + import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.web.client.RestClient; -import java.time.Duration; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BiFunction; - import static io.modelcontextprotocol.server.transport.HttpServletStatelessServerTransport.APPLICATION_JSON; import static io.modelcontextprotocol.server.transport.HttpServletStatelessServerTransport.TEXT_EVENT_STREAM; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; @@ -315,13 +317,14 @@ void testStructuredOutputWithInHandlerError(String clientType) { .outputSchema(outputSchema) .build(); - // Handler that throws an exception to simulate an error + // Handler that returns an error result McpStatelessServerFeatures.SyncToolSpecification tool = McpStatelessServerFeatures.SyncToolSpecification .builder() .tool(calculatorTool) - .callHandler((exchange, request) -> { - throw new RuntimeException("Simulated in-handler error"); - }) + .callHandler((exchange, request) -> CallToolResult.builder() + .isError(true) + .content(List.of(new TextContent("Error calling tool: Simulated in-handler error"))) + .build()) .build(); var mcpServer = McpServer.sync(mcpStatelessServerTransport) @@ -572,68 +575,10 @@ void testThrownMcpErrorAndJsonRpcError() throws Exception { McpSchema.JSONRPCResponse jsonrpcResponse = new ObjectMapper().readValue(response.getContentAsByteArray(), McpSchema.JSONRPCResponse.class); - CallToolResult callToolResult = new ObjectMapper().convertValue(jsonrpcResponse.result(), CallToolResult.class); - - assertThat(callToolResult).isNotNull(); - assertThat(callToolResult.isError()).isTrue(); - assertThat(callToolResult.content()).isNotEmpty(); - assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent("Error calling tool: testing")); - assertThat(callToolResult.structuredContent()).isNull(); - - mcpServer.close(); - } - - @Test - @Timeout(15000) - void testThrownNonMcpError() throws Exception { - var mcpServer = McpServer.sync(mcpStatelessServerTransport) - .serverInfo("test-server", "1.0.0") - .capabilities(ServerCapabilities.builder().tools(true).build()) - .build(); - - Tool testTool = Tool.builder().name("test").description("test").build(); - - McpStatelessServerFeatures.SyncToolSpecification toolSpec = new McpStatelessServerFeatures.SyncToolSpecification( - testTool, (transportContext, request) -> { - throw new McpError(new McpSchema.JSONRPCResponse.JSONRPCError(12345, "testing", Map.of("a", "b"))); - }); - - mcpServer.addTool(toolSpec); - - McpSchema.CallToolRequest callToolRequest = new McpSchema.CallToolRequest("test", Map.of()); - McpSchema.JSONRPCRequest jsonrpcRequest = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, - McpSchema.METHOD_TOOLS_CALL, "test", callToolRequest); - - MockHttpServletRequest request = new MockHttpServletRequest("POST", CUSTOM_MESSAGE_ENDPOINT); - MockHttpServletResponse response = new MockHttpServletResponse(); - - byte[] content = new ObjectMapper().writeValueAsBytes(jsonrpcRequest); - request.setContent(content); - request.addHeader("Content-Type", "application/json"); - request.addHeader("Content-Length", Integer.toString(content.length)); - request.addHeader("Content-Length", Integer.toString(content.length)); - request.addHeader("Accept", APPLICATION_JSON + ", " + TEXT_EVENT_STREAM); - request.addHeader("Content-Type", APPLICATION_JSON); - request.addHeader("Cache-Control", "no-cache"); - request.addHeader(HttpHeaders.PROTOCOL_VERSION, ProtocolVersions.MCP_2025_03_26); - - mcpStatelessServerTransport.service(request, response); - - McpSchema.JSONRPCResponse jsonrpcResponse = new ObjectMapper().readValue(response.getContentAsByteArray(), - McpSchema.JSONRPCResponse.class); - - // CallToolResult callToolResult = new - // ObjectMapper().convertValue(jsonrpcResponse.result(), CallToolResult.class); - - // assertThat(callToolResult).isNotNull(); - // assertThat(callToolResult.isError()).isTrue(); - // assertThat(callToolResult.content()).isNotEmpty(); - // assertThat(callToolResult.content()) - // .containsExactly(new McpSchema.TextContent("Error calling tool: testing")); - // assertThat(callToolResult.structuredContent()).isNull(); - - assertThat(jsonrpcResponse.error()) - .isEqualTo(new McpSchema.JSONRPCResponse.JSONRPCError(12345, "testing", Map.of("a", "b"))); + assertThat(jsonrpcResponse).isNotNull(); + assertThat(jsonrpcResponse.error()).isNotNull(); + assertThat(jsonrpcResponse.error().code()).isEqualTo(ErrorCodes.INTERNAL_ERROR); + assertThat(jsonrpcResponse.error().message()).isEqualTo("testing"); mcpServer.close(); }