Skip to content

fix: align tool error handling with MCP specification #540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

fix: align tool error handling with MCP specification #540

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e3f76e
fix: align tool error handling with MCP specification
tzolov Sep 6, 2025
346d767
fix: Return tool errors as CallToolResult instead of throwing exceptions
tzolov Sep 7, 2025
f59dbaf
fix: Skip structured output validation for error tool results
tzolov Sep 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 106 additions & 32 deletions mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, McpClient.SyncSpec> clientBuilders = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -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();

Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1434,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<String, Object> 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);
Expand Down
90 changes: 74 additions & 16 deletions mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String, McpClient.SyncSpec> clientBuilders = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<String, Object> 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) {
Expand Down
27 changes: 23 additions & 4 deletions mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -376,6 +378,11 @@ public Mono<CallToolResult> apply(McpAsyncServerExchange exchange, McpSchema.Cal

return this.delegateCallToolResult.apply(exchange, request).map(result -> {

if (result.isError() != null && result.isError()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can simplify to Boolean.TRUE.equals(result.isError())

// If the tool call resulted in an error, skip further validation
return result;
}

if (outputSchema == null) {
if (result.structuredContent() != null) {
logger.warn(
Expand Down Expand Up @@ -507,11 +514,23 @@ private McpRequestHandler<CallToolResult> 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())));
};
}

Expand Down
Loading
Loading