Skip to content

Commit 09d8df4

Browse files
tzolovtzolov
committed
feat: enhance error handling with custom error code preservation
- Improve McpClientSession error handling to preserve custom error codes and data from McpError instances. - Add aggregated exception messages to error data field for better debugging. - Include test coverage for various McpClientSession error scenarios. Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
1 parent 6e9af40 commit 09d8df4

File tree

2 files changed

+157
-14
lines changed

2 files changed

+157
-14
lines changed

‎mcp-core/src/main/java/io/modelcontextprotocol/spec/McpClientSession.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,15 @@ private void handle(McpSchema.JSONRPCMessage message) {
166166
else if (message instanceof McpSchema.JSONRPCRequest request) {
167167
logger.debug("Received request: {}", request);
168168
handleIncomingRequest(request).onErrorResume(error -> {
169+
170+
McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (error instanceof McpError mcpError
171+
&& mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError()
172+
// TODO: add error message through the data field
173+
: new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR,
174+
error.getMessage(), McpError.aggregateExceptionMessages(error));
175+
169176
var errorResponse = new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), null,
170-
new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR,
171-
error.getMessage(), null));
177+
jsonRpcError);
172178
return Mono.just(errorResponse);
173179
}).flatMap(this.transport::sendMessage).onErrorComplete(t -> {
174180
logger.warn("Issue sending response to the client, ", t);

‎mcp-core/src/test/java/io/modelcontextprotocol/spec/McpClientSessionTests.java‎

Lines changed: 149 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import reactor.test.StepVerifier;
2020

2121
import static org.assertj.core.api.Assertions.assertThat;
22-
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2322

2423
/**
2524
* Test suite for {@link McpClientSession} that verifies its JSON-RPC message handling,
@@ -57,17 +56,6 @@ void tearDown() {
5756
}
5857
}
5958

60-
@Test
61-
void testConstructorWithInvalidArguments() {
62-
assertThatThrownBy(() -> new McpClientSession(null, transport, Map.of(), Map.of()))
63-
.isInstanceOf(IllegalArgumentException.class)
64-
.hasMessageContaining("The requestTimeout can not be null");
65-
66-
assertThatThrownBy(() -> new McpClientSession(TIMEOUT, null, Map.of(), Map.of()))
67-
.isInstanceOf(IllegalArgumentException.class)
68-
.hasMessageContaining("transport can not be null");
69-
}
70-
7159
TypeRef<String> responseType = new TypeRef<>() {
7260
};
7361

@@ -190,6 +178,155 @@ void testUnknownMethodHandling() {
190178
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.METHOD_NOT_FOUND);
191179
}
192180

181+
@Test
182+
void testRequestHandlerThrowsMcpErrorWithJsonRpcError() {
183+
// Setup: Create a request handler that throws McpError with custom error code and
184+
// data
185+
String testMethod = "test.customError";
186+
Map<String, Object> errorData = Map.of("customField", "customValue");
187+
McpClientSession.RequestHandler<?> failingHandler = params -> Mono
188+
.error(McpError.builder(123).message("Custom error message").data(errorData).build());
189+
190+
transport = new MockMcpClientTransport();
191+
session = new McpClientSession(TIMEOUT, transport, Map.of(testMethod, failingHandler), Map.of());
192+
193+
// Simulate incoming request that will trigger the error
194+
McpSchema.JSONRPCRequest request = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, testMethod,
195+
"test-id", null);
196+
transport.simulateIncomingMessage(request);
197+
198+
// Verify: The response should contain the custom error from McpError
199+
McpSchema.JSONRPCMessage sentMessage = transport.getLastSentMessage();
200+
assertThat(sentMessage).isInstanceOf(McpSchema.JSONRPCResponse.class);
201+
McpSchema.JSONRPCResponse response = (McpSchema.JSONRPCResponse) sentMessage;
202+
assertThat(response.error()).isNotNull();
203+
assertThat(response.error().code()).isEqualTo(123);
204+
assertThat(response.error().message()).isEqualTo("Custom error message");
205+
assertThat(response.error().data()).isEqualTo(errorData);
206+
}
207+
208+
@Test
209+
void testRequestHandlerThrowsGenericException() {
210+
// Setup: Create a request handler that throws a generic RuntimeException
211+
String testMethod = "test.genericError";
212+
RuntimeException exception = new RuntimeException("Something went wrong");
213+
McpClientSession.RequestHandler<?> failingHandler = params -> Mono.error(exception);
214+
215+
transport = new MockMcpClientTransport();
216+
session = new McpClientSession(TIMEOUT, transport, Map.of(testMethod, failingHandler), Map.of());
217+
218+
// Simulate incoming request that will trigger the error
219+
McpSchema.JSONRPCRequest request = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, testMethod,
220+
"test-id", null);
221+
transport.simulateIncomingMessage(request);
222+
223+
// Verify: The response should contain INTERNAL_ERROR with aggregated exception
224+
// messages in data field
225+
McpSchema.JSONRPCMessage sentMessage = transport.getLastSentMessage();
226+
assertThat(sentMessage).isInstanceOf(McpSchema.JSONRPCResponse.class);
227+
McpSchema.JSONRPCResponse response = (McpSchema.JSONRPCResponse) sentMessage;
228+
assertThat(response.error()).isNotNull();
229+
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.INTERNAL_ERROR);
230+
assertThat(response.error().message()).isEqualTo("Something went wrong");
231+
// Verify data field contains aggregated exception messages
232+
assertThat(response.error().data()).isNotNull();
233+
assertThat(response.error().data().toString()).contains("RuntimeException");
234+
assertThat(response.error().data().toString()).contains("Something went wrong");
235+
}
236+
237+
@Test
238+
void testRequestHandlerThrowsExceptionWithCause() {
239+
// Setup: Create a request handler that throws an exception with a cause chain
240+
String testMethod = "test.chainedError";
241+
RuntimeException rootCause = new IllegalArgumentException("Root cause message");
242+
RuntimeException middleCause = new IllegalStateException("Middle cause message", rootCause);
243+
RuntimeException topException = new RuntimeException("Top level message", middleCause);
244+
McpClientSession.RequestHandler<?> failingHandler = params -> Mono.error(topException);
245+
246+
transport = new MockMcpClientTransport();
247+
session = new McpClientSession(TIMEOUT, transport, Map.of(testMethod, failingHandler), Map.of());
248+
249+
// Simulate incoming request that will trigger the error
250+
McpSchema.JSONRPCRequest request = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, testMethod,
251+
"test-id", null);
252+
transport.simulateIncomingMessage(request);
253+
254+
// Verify: The response should contain INTERNAL_ERROR with full exception chain
255+
// in data field
256+
McpSchema.JSONRPCMessage sentMessage = transport.getLastSentMessage();
257+
assertThat(sentMessage).isInstanceOf(McpSchema.JSONRPCResponse.class);
258+
McpSchema.JSONRPCResponse response = (McpSchema.JSONRPCResponse) sentMessage;
259+
assertThat(response.error()).isNotNull();
260+
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.INTERNAL_ERROR);
261+
assertThat(response.error().message()).isEqualTo("Top level message");
262+
// Verify data field contains the full exception chain
263+
String dataString = response.error().data().toString();
264+
assertThat(dataString).contains("RuntimeException");
265+
assertThat(dataString).contains("Top level message");
266+
assertThat(dataString).contains("IllegalStateException");
267+
assertThat(dataString).contains("Middle cause message");
268+
assertThat(dataString).contains("IllegalArgumentException");
269+
assertThat(dataString).contains("Root cause message");
270+
}
271+
272+
@Test
273+
void testRequestHandlerThrowsMcpErrorWithoutJsonRpcError() {
274+
// Setup: Create a request handler that throws deprecated McpError without
275+
// JSONRPCError
276+
String testMethod = "test.deprecatedError";
277+
@SuppressWarnings("deprecation")
278+
McpError deprecatedError = new McpError("Deprecated error format");
279+
McpClientSession.RequestHandler<?> failingHandler = params -> Mono.error(deprecatedError);
280+
281+
transport = new MockMcpClientTransport();
282+
session = new McpClientSession(TIMEOUT, transport, Map.of(testMethod, failingHandler), Map.of());
283+
284+
// Simulate incoming request that will trigger the error
285+
McpSchema.JSONRPCRequest request = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, testMethod,
286+
"test-id", null);
287+
transport.simulateIncomingMessage(request);
288+
289+
// Verify: The response should create a new INTERNAL_ERROR with aggregated
290+
// messages
291+
McpSchema.JSONRPCMessage sentMessage = transport.getLastSentMessage();
292+
assertThat(sentMessage).isInstanceOf(McpSchema.JSONRPCResponse.class);
293+
McpSchema.JSONRPCResponse response = (McpSchema.JSONRPCResponse) sentMessage;
294+
assertThat(response.error()).isNotNull();
295+
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.INTERNAL_ERROR);
296+
assertThat(response.error().message()).isEqualTo("Deprecated error format");
297+
// Verify data field contains aggregated exception messages
298+
assertThat(response.error().data()).isNotNull();
299+
assertThat(response.error().data().toString()).contains("McpError");
300+
assertThat(response.error().data().toString()).contains("Deprecated error format");
301+
}
302+
303+
@Test
304+
void testRequestHandlerThrowsResourceNotFoundError() {
305+
// Setup: Create a request handler that throws RESOURCE_NOT_FOUND error
306+
String testMethod = "test.resourceError";
307+
String resourceUri = "file:///missing/resource.txt";
308+
McpClientSession.RequestHandler<?> failingHandler = params -> Mono
309+
.error(McpError.RESOURCE_NOT_FOUND.apply(resourceUri));
310+
311+
transport = new MockMcpClientTransport();
312+
session = new McpClientSession(TIMEOUT, transport, Map.of(testMethod, failingHandler), Map.of());
313+
314+
// Simulate incoming request that will trigger the error
315+
McpSchema.JSONRPCRequest request = new McpSchema.JSONRPCRequest(McpSchema.JSONRPC_VERSION, testMethod,
316+
"test-id", null);
317+
transport.simulateIncomingMessage(request);
318+
319+
// Verify: The response should preserve the RESOURCE_NOT_FOUND error code and
320+
// data
321+
McpSchema.JSONRPCMessage sentMessage = transport.getLastSentMessage();
322+
assertThat(sentMessage).isInstanceOf(McpSchema.JSONRPCResponse.class);
323+
McpSchema.JSONRPCResponse response = (McpSchema.JSONRPCResponse) sentMessage;
324+
assertThat(response.error()).isNotNull();
325+
assertThat(response.error().code()).isEqualTo(McpSchema.ErrorCodes.RESOURCE_NOT_FOUND);
326+
assertThat(response.error().message()).isEqualTo("Resource not found");
327+
assertThat(response.error().data()).isEqualTo(Map.of("uri", resourceUri));
328+
}
329+
193330
@Test
194331
void testGracefulShutdown() {
195332
StepVerifier.create(session.closeGracefully()).verifyComplete();

0 commit comments

Comments
 (0)