Skip to content

Commit 86b7c4e

Browse files
committed
fix: handle root exceptions in McpToolCallback
- Sync and Async McpToolCallback can now handle exceptions where cause == null instead of throwing a NPE. Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
1 parent 384478e commit 86b7c4e

File tree

4 files changed

+56
-2
lines changed

4 files changed

+56
-2
lines changed

mcp/common/src/main/java/org/springframework/ai/mcp/AsyncMcpToolCallback.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public String call(String functionInput) {
122122
}).block();
123123
}
124124
catch (Exception ex) {
125-
throw new ToolExecutionException(this.getToolDefinition(), ex.getCause());
125+
throw new ToolExecutionException(this.getToolDefinition(), ex);
126126
}
127127

128128
}

mcp/common/src/main/java/org/springframework/ai/mcp/SyncMcpToolCallback.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public String call(String functionInput) {
131131
}
132132
catch (Exception ex) {
133133
logger.error("Exception while tool calling: ", ex);
134-
throw new ToolExecutionException(this.getToolDefinition(), ex.getCause());
134+
throw new ToolExecutionException(this.getToolDefinition(), ex);
135135
}
136136

137137
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package org.springframework.ai.mcp;
2+
3+
import io.modelcontextprotocol.client.McpAsyncClient;
4+
import io.modelcontextprotocol.spec.McpSchema;
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.extension.ExtendWith;
7+
import org.mockito.Mock;
8+
import org.mockito.junit.jupiter.MockitoExtension;
9+
10+
import org.springframework.ai.tool.execution.ToolExecutionException;
11+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
12+
import static org.mockito.ArgumentMatchers.any;
13+
import static org.mockito.Mockito.when;
14+
15+
@ExtendWith(MockitoExtension.class)
16+
class AsyncMcpToolCallbackTest {
17+
18+
@Mock
19+
private McpAsyncClient mcpClient;
20+
21+
@Mock
22+
private McpSchema.Tool tool;
23+
24+
@Test
25+
void callShouldHandleExceptions() {
26+
when(this.tool.name()).thenReturn("testTool");
27+
var clientInfo = new McpSchema.Implementation("testClient", "1.0.0");
28+
when(this.mcpClient.getClientInfo()).thenReturn(clientInfo);
29+
when(this.mcpClient.callTool(any(McpSchema.CallToolRequest.class))).thenThrow(new RuntimeException("Testing tool error"));
30+
31+
AsyncMcpToolCallback callback = new AsyncMcpToolCallback(this.mcpClient, this.tool);
32+
assertThatThrownBy(() -> callback.call("{\"param\":\"value\"}"))
33+
.isInstanceOf(ToolExecutionException.class)
34+
.rootCause()
35+
.hasMessage("Testing tool error");
36+
}
37+
38+
}

mcp/common/src/test/java/org/springframework/ai/mcp/SyncMcpToolCallbackTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@
2929
import org.mockito.junit.jupiter.MockitoExtension;
3030

3131
import org.springframework.ai.chat.model.ToolContext;
32+
import org.springframework.ai.tool.execution.ToolExecutionException;
3233

3334
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3436
import static org.mockito.ArgumentMatchers.any;
3537
import static org.mockito.Mockito.mock;
3638
import static org.mockito.Mockito.when;
@@ -94,4 +96,18 @@ void callShouldIgnoreToolContext() {
9496
assertThat(response).isNotNull();
9597
}
9698

99+
@Test
100+
void callShouldWrapExceptions() {
101+
when(this.tool.name()).thenReturn("testTool");
102+
var clientInfo = new Implementation("testClient", "1.0.0");
103+
when(this.mcpClient.getClientInfo()).thenReturn(clientInfo);
104+
when(this.mcpClient.callTool(any(CallToolRequest.class))).thenThrow(new RuntimeException("Testing tool error"));
105+
106+
SyncMcpToolCallback callback = new SyncMcpToolCallback(this.mcpClient, this.tool);
107+
assertThatThrownBy(() -> callback.call("{\"param\":\"value\"}"))
108+
.isInstanceOf(ToolExecutionException.class)
109+
.rootCause()
110+
.hasMessage("Testing tool error");
111+
}
112+
97113
}

0 commit comments

Comments
 (0)