Skip to content

Commit 346d767

Browse files
tzolovtzolov
committed
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 <christian.tzolov@broadcom.com>
1 parent 7e3f76e commit 346d767

File tree

6 files changed

+337
-20
lines changed

6 files changed

+337
-20
lines changed

‎mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,6 +1452,62 @@ void testStructuredOutputValidationSuccess(String clientType) {
14521452

14531453
@ParameterizedTest(name = "{0} : {displayName} ")
14541454
@ValueSource(strings = { "httpclient", "webflux" })
1455+
void testStructuredOutputWithInHandlerError(String clientType) {
1456+
var clientBuilder = clientBuilders.get(clientType);
1457+
1458+
// Create a tool with output schema
1459+
Map<String, Object> outputSchema = Map.of(
1460+
"type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation",
1461+
Map.of("type", "string"), "timestamp", Map.of("type", "string")),
1462+
"required", List.of("result", "operation"));
1463+
1464+
Tool calculatorTool = Tool.builder()
1465+
.name("calculator")
1466+
.description("Performs mathematical calculations")
1467+
.outputSchema(outputSchema)
1468+
.build();
1469+
1470+
// Handler that throws an exception to simulate an error
1471+
McpServerFeatures.SyncToolSpecification tool = McpServerFeatures.SyncToolSpecification.builder()
1472+
.tool(calculatorTool)
1473+
.callHandler((exchange, request) -> {
1474+
throw new RuntimeException("Simulated in-handler error");
1475+
})
1476+
.build();
1477+
1478+
var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0")
1479+
.capabilities(ServerCapabilities.builder().tools(true).build())
1480+
.tools(tool)
1481+
.build();
1482+
1483+
try (var mcpClient = clientBuilder.build()) {
1484+
InitializeResult initResult = mcpClient.initialize();
1485+
assertThat(initResult).isNotNull();
1486+
1487+
// Verify tool is listed with output schema
1488+
var toolsList = mcpClient.listTools();
1489+
assertThat(toolsList.tools()).hasSize(1);
1490+
assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator");
1491+
// Note: outputSchema might be null in sync server, but validation still works
1492+
1493+
// Call tool with valid structured output
1494+
CallToolResult response = mcpClient
1495+
.callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3")));
1496+
1497+
assertThat(response).isNotNull();
1498+
assertThat(response.isError()).isTrue();
1499+
assertThat(response.content()).isNotEmpty();
1500+
assertThat(response.content())
1501+
.containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error"));
1502+
assertThat(response.structuredContent()).isNull();
1503+
}
1504+
finally {
1505+
mcpServer.closeGracefully();
1506+
}
1507+
}
1508+
1509+
@ParameterizedTest(name = "{0} : {displayName} ")
1510+
@ValueSource(strings = { "httpclient" })
14551511
void testStructuredOutputValidationFailure(String clientType) {
14561512

14571513
var clientBuilder = clientBuilders.get(clientType);

‎mcp-test/src/main/java/io/modelcontextprotocol/AbstractStatelessIntegrationTests.java‎

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,6 @@
44

55
package io.modelcontextprotocol;
66

7-
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
8-
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json;
9-
import static org.assertj.core.api.Assertions.assertThat;
10-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
11-
import static org.awaitility.Awaitility.await;
12-
137
import java.net.URI;
148
import java.net.http.HttpClient;
159
import java.net.http.HttpRequest;
@@ -20,23 +14,26 @@
2014
import java.util.concurrent.ConcurrentHashMap;
2115
import java.util.concurrent.atomic.AtomicReference;
2216

23-
import org.junit.jupiter.params.ParameterizedTest;
24-
import org.junit.jupiter.params.provider.ValueSource;
25-
2617
import io.modelcontextprotocol.client.McpClient;
2718
import io.modelcontextprotocol.server.McpServer.StatelessAsyncSpecification;
2819
import io.modelcontextprotocol.server.McpServer.StatelessSyncSpecification;
2920
import io.modelcontextprotocol.server.McpStatelessServerFeatures;
3021
import io.modelcontextprotocol.server.McpStatelessSyncServer;
31-
import io.modelcontextprotocol.spec.McpError;
3222
import io.modelcontextprotocol.spec.McpSchema;
3323
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
3424
import io.modelcontextprotocol.spec.McpSchema.InitializeResult;
3525
import io.modelcontextprotocol.spec.McpSchema.ServerCapabilities;
3626
import io.modelcontextprotocol.spec.McpSchema.Tool;
3727
import net.javacrumbs.jsonunit.core.Option;
28+
import org.junit.jupiter.params.ParameterizedTest;
29+
import org.junit.jupiter.params.provider.ValueSource;
3830
import reactor.core.publisher.Mono;
3931

32+
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
33+
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.json;
34+
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.awaitility.Awaitility.await;
36+
4037
public abstract class AbstractStatelessIntegrationTests {
4138

4239
protected ConcurrentHashMap<String, McpClient.SyncSpec> clientBuilders = new ConcurrentHashMap<>();
@@ -159,12 +156,16 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) {
159156
InitializeResult initResult = mcpClient.initialize();
160157
assertThat(initResult).isNotNull();
161158

162-
// We expect the tool call to fail immediately with the exception raised by
163-
// the offending tool
164-
// instead of getting back a timeout.
165-
assertThatExceptionOfType(McpError.class)
166-
.isThrownBy(() -> mcpClient.callTool(new McpSchema.CallToolRequest("tool1", Map.of())))
167-
.withMessageContaining("Timeout on blocking read");
159+
McpSchema.CallToolResult callToolResult = mcpClient
160+
.callTool(new McpSchema.CallToolRequest("tool1", Map.of()));
161+
162+
// Tool errors should be reported within the result object, not as MCP
163+
// protocol-level errors. This allows the LLM to see and potentially
164+
// handle the error.
165+
assertThat(callToolResult).isNotNull();
166+
assertThat(callToolResult.isError()).isTrue();
167+
assertThat(callToolResult.content()).containsExactly(new McpSchema.TextContent(
168+
"Error calling tool: Timeout on blocking read for 1000000000 NANOSECONDS"));
168169
}
169170
finally {
170171
mcpServer.closeGracefully();
@@ -350,6 +351,63 @@ void testStructuredOutputValidationSuccess(String clientType) {
350351
}
351352
}
352353

354+
@ParameterizedTest(name = "{0} : {displayName} ")
355+
@ValueSource(strings = { "httpclient", "webflux" })
356+
void testStructuredOutputWithInHandlerError(String clientType) {
357+
var clientBuilder = clientBuilders.get(clientType);
358+
359+
// Create a tool with output schema
360+
Map<String, Object> outputSchema = Map.of(
361+
"type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation",
362+
Map.of("type", "string"), "timestamp", Map.of("type", "string")),
363+
"required", List.of("result", "operation"));
364+
365+
Tool calculatorTool = Tool.builder()
366+
.name("calculator")
367+
.description("Performs mathematical calculations")
368+
.outputSchema(outputSchema)
369+
.build();
370+
371+
// Handler that throws an exception to simulate an error
372+
McpStatelessServerFeatures.SyncToolSpecification tool = McpStatelessServerFeatures.SyncToolSpecification
373+
.builder()
374+
.tool(calculatorTool)
375+
.callHandler((exchange, request) -> {
376+
throw new RuntimeException("Simulated in-handler error");
377+
})
378+
.build();
379+
380+
var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0")
381+
.capabilities(ServerCapabilities.builder().tools(true).build())
382+
.tools(tool)
383+
.build();
384+
385+
try (var mcpClient = clientBuilder.build()) {
386+
InitializeResult initResult = mcpClient.initialize();
387+
assertThat(initResult).isNotNull();
388+
389+
// Verify tool is listed with output schema
390+
var toolsList = mcpClient.listTools();
391+
assertThat(toolsList.tools()).hasSize(1);
392+
assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator");
393+
// Note: outputSchema might be null in sync server, but validation still works
394+
395+
// Call tool with valid structured output
396+
CallToolResult response = mcpClient
397+
.callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3")));
398+
399+
assertThat(response).isNotNull();
400+
assertThat(response.isError()).isTrue();
401+
assertThat(response.content()).isNotEmpty();
402+
assertThat(response.content())
403+
.containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error"));
404+
assertThat(response.structuredContent()).isNull();
405+
}
406+
finally {
407+
mcpServer.closeGracefully();
408+
}
409+
}
410+
353411
@ParameterizedTest(name = "{0} : {displayName} ")
354412
@ValueSource(strings = { "httpclient", "webflux" })
355413
void testStructuredOutputValidationFailure(String clientType) {

‎mcp/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
import io.modelcontextprotocol.spec.McpError;
1212
import io.modelcontextprotocol.spec.McpSchema;
1313
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
14+
import io.modelcontextprotocol.spec.McpSchema.JSONRPCResponse;
1415
import io.modelcontextprotocol.spec.McpSchema.ResourceTemplate;
16+
import io.modelcontextprotocol.spec.McpSchema.TextContent;
1517
import io.modelcontextprotocol.spec.McpSchema.Tool;
1618
import io.modelcontextprotocol.spec.McpStatelessServerTransport;
1719
import io.modelcontextprotocol.util.Assert;
@@ -380,11 +382,31 @@ private McpStatelessRequestHandler<CallToolResult> toolsCallRequestHandler() {
380382
.findAny();
381383

382384
if (toolSpecification.isEmpty()) {
383-
return Mono.error(new McpError("Tool not found: " + callToolRequest.name()));
385+
return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS,
386+
"Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name())));
384387
}
388+
else {
389+
return toolSpecification.get().callHandler().apply(ctx, callToolRequest).onErrorResume(error -> {
390+
logger.error("Error calling tool: {}", callToolRequest.name(), error);
391+
392+
// TODO: Should we handle the McpError+JsonRcpError specaially (e.g.
393+
// propagate)
394+
// or always return a CallToolResult with isError=true?
395+
if (error instanceof McpError mcpError && mcpError.getJsonRpcError() != null) {
396+
// If the error is already an MCP error, propagate it as is
397+
return Mono.error(mcpError);
398+
}
385399

386-
return toolSpecification.map(tool -> tool.callHandler().apply(ctx, callToolRequest))
387-
.orElse(Mono.error(new McpError("Tool not found: " + callToolRequest.name())));
400+
// Tool errors should be reported within the result object, not as MCP
401+
// protocol-level errors. This allows the LLM to see and potentially
402+
// handle the error.
403+
return Mono.just(CallToolResult.builder()
404+
.isError(true)
405+
.content(List
406+
.of(new TextContent("Error calling tool: " + Utils.findRootCause(error).getMessage())))
407+
.build());
408+
});
409+
}
388410
};
389411
}
390412

‎mcp/src/main/java/io/modelcontextprotocol/util/Utils.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,12 @@ private static boolean isUnderBaseUri(URI baseUri, URI endpointUri) {
108108
return endpointPath.startsWith(basePath);
109109
}
110110

111+
/**
112+
* Finds the root cause of the given throwable by traversing the cause chain.
113+
* @param throwable The throwable to analyze
114+
* @return The root cause throwable
115+
* @throws NullPointerException if the provided throwable is null
116+
*/
111117
public static Throwable findRootCause(Throwable throwable) {
112118
Objects.requireNonNull(throwable);
113119
Throwable rootCause = throwable;

‎mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,6 +1446,62 @@ void testStructuredOutputValidationSuccess(String clientType) {
14461446
}
14471447
}
14481448

1449+
@ParameterizedTest(name = "{0} : {displayName} ")
1450+
@ValueSource(strings = { "httpclient" })
1451+
void testStructuredOutputWithInHandlerError(String clientType) {
1452+
var clientBuilder = clientBuilders.get(clientType);
1453+
1454+
// Create a tool with output schema
1455+
Map<String, Object> outputSchema = Map.of(
1456+
"type", "object", "properties", Map.of("result", Map.of("type", "number"), "operation",
1457+
Map.of("type", "string"), "timestamp", Map.of("type", "string")),
1458+
"required", List.of("result", "operation"));
1459+
1460+
Tool calculatorTool = Tool.builder()
1461+
.name("calculator")
1462+
.description("Performs mathematical calculations")
1463+
.outputSchema(outputSchema)
1464+
.build();
1465+
1466+
// Handler that throws an exception to simulate an error
1467+
McpServerFeatures.SyncToolSpecification tool = McpServerFeatures.SyncToolSpecification.builder()
1468+
.tool(calculatorTool)
1469+
.callHandler((exchange, request) -> {
1470+
throw new RuntimeException("Simulated in-handler error");
1471+
})
1472+
.build();
1473+
1474+
var mcpServer = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0")
1475+
.capabilities(ServerCapabilities.builder().tools(true).build())
1476+
.tools(tool)
1477+
.build();
1478+
1479+
try (var mcpClient = clientBuilder.build()) {
1480+
InitializeResult initResult = mcpClient.initialize();
1481+
assertThat(initResult).isNotNull();
1482+
1483+
// Verify tool is listed with output schema
1484+
var toolsList = mcpClient.listTools();
1485+
assertThat(toolsList.tools()).hasSize(1);
1486+
assertThat(toolsList.tools().get(0).name()).isEqualTo("calculator");
1487+
// Note: outputSchema might be null in sync server, but validation still works
1488+
1489+
// Call tool with valid structured output
1490+
CallToolResult response = mcpClient
1491+
.callTool(new McpSchema.CallToolRequest("calculator", Map.of("expression", "2 + 3")));
1492+
1493+
assertThat(response).isNotNull();
1494+
assertThat(response.isError()).isTrue();
1495+
assertThat(response.content()).isNotEmpty();
1496+
assertThat(response.content())
1497+
.containsExactly(new McpSchema.TextContent("Error calling tool: Simulated in-handler error"));
1498+
assertThat(response.structuredContent()).isNull();
1499+
}
1500+
finally {
1501+
mcpServer.closeGracefully();
1502+
}
1503+
}
1504+
14491505
@ParameterizedTest(name = "{0} : {displayName} ")
14501506
@ValueSource(strings = { "httpclient" })
14511507
void testStructuredOutputValidationFailure(String clientType) {

0 commit comments

Comments
 (0)