From b0264927f00e9e549a9291e1a7e88da9a8982d15 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sat, 27 Sep 2025 11:24:38 +0200 Subject: [PATCH 1/3] refactor: improve MCP server resilience and error handling - Replace McpError with appropriate standard exceptions (IllegalArgumentException, IllegalStateException) for validation errors - Change tool/prompt registration to replace existing items instead of rejecting duplicates with warningsi - Change addTool behavior to replace existing tools instead of throwing errors - Change addPrompt() to allow replacing existing prompts with warning instead of throwing error - Make removal operations idempotent - log warnings instead of throwing errors for non-existent items - Change removePrompt() to log warning instead of throwing error for non-existent prompts - Change removeTool() to gracefully handle non-existent tools with warnings instead of errors - Add listTools() and listPrompts() methods to all server variants (async, sync, stateless) - Add listTools() method to all server classes for tool enumeration - Add listPrompts() method to all server implementations for retrieving registered prompts - Improve error construction using McpError.builder() pattern for protocol-specific errors - Update error handling in session classes to properly propagate McpError JSON-RPC errors - Use proper error codes (INVALID_PARAMS) for prompt not found scenarios - Update tests to reflect new lenient behavior for duplicate registrations and removals This change makes the MCP server APIs more resilient and user-friendly by using appropriate exception types, supporting item replacement, and providing listing capabilities while maintaining backward compatibility. Signed-off-by: Christian Tzolov --- .../server/McpAsyncServer.java | 94 +++++++++++-------- .../server/McpStatelessAsyncServer.java | 86 +++++++++++------ .../server/McpStatelessSyncServer.java | 16 ++++ .../server/McpSyncServer.java | 16 ++++ .../spec/McpServerSession.java | 13 ++- .../spec/McpStreamableServerSession.java | 9 +- .../server/AbstractMcpAsyncServerTests.java | 34 ++----- .../server/AbstractMcpSyncServerTests.java | 27 +++--- .../server/AbstractMcpAsyncServerTests.java | 34 ++----- .../server/AbstractMcpSyncServerTests.java | 27 +++--- 10 files changed, 203 insertions(+), 153 deletions(-) diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java index c07fdf2af..dcba3af1f 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java @@ -322,25 +322,24 @@ private McpNotificationHandler asyncRootsListChangedNotificationHandler( */ public Mono addTool(McpServerFeatures.AsyncToolSpecification toolSpecification) { if (toolSpecification == null) { - return Mono.error(new McpError("Tool specification must not be null")); + return Mono.error(new IllegalArgumentException("Tool specification must not be null")); } if (toolSpecification.tool() == null) { - return Mono.error(new McpError("Tool must not be null")); + return Mono.error(new IllegalArgumentException("Tool must not be null")); } if (toolSpecification.call() == null && toolSpecification.callHandler() == null) { - return Mono.error(new McpError("Tool call handler must not be null")); + return Mono.error(new IllegalArgumentException("Tool call handler must not be null")); } if (this.serverCapabilities.tools() == null) { - return Mono.error(new McpError("Server must be configured with tool capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with tool capabilities")); } var wrappedToolSpecification = withStructuredOutputHandling(this.jsonSchemaValidator, toolSpecification); return Mono.defer(() -> { - // Check for duplicate tool names - if (this.tools.stream().anyMatch(th -> th.tool().name().equals(wrappedToolSpecification.tool().name()))) { - return Mono.error( - new McpError("Tool with name '" + wrappedToolSpecification.tool().name() + "' already exists")); + // Remove tools with duplicate tool names first + if (this.tools.removeIf(th -> th.tool().name().equals(wrappedToolSpecification.tool().name()))) { + logger.warn("Replace existing Tool with name '{}'", wrappedToolSpecification.tool().name()); } this.tools.add(wrappedToolSpecification); @@ -464,6 +463,14 @@ private static McpServerFeatures.AsyncToolSpecification withStructuredOutputHand .build(); } + /** + * List all registered tools. + * @return A Flux stream of all registered tools + */ + public Flux listTools() { + return Flux.fromIterable(this.tools).map(McpServerFeatures.AsyncToolSpecification::tool); + } + /** * Remove a tool handler at runtime. * @param toolName The name of the tool handler to remove @@ -471,23 +478,25 @@ private static McpServerFeatures.AsyncToolSpecification withStructuredOutputHand */ public Mono removeTool(String toolName) { if (toolName == null) { - return Mono.error(new McpError("Tool name must not be null")); + return Mono.error(new IllegalArgumentException("Tool name must not be null")); } if (this.serverCapabilities.tools() == null) { - return Mono.error(new McpError("Server must be configured with tool capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with tool capabilities")); } return Mono.defer(() -> { - boolean removed = this.tools - .removeIf(toolSpecification -> toolSpecification.tool().name().equals(toolName)); - if (removed) { + if (this.tools.removeIf(toolSpecification -> toolSpecification.tool().name().equals(toolName))) { + logger.debug("Removed tool handler: {}", toolName); if (this.serverCapabilities.tools().listChanged()) { return notifyToolsListChanged(); } - return Mono.empty(); } - return Mono.error(new McpError("Tool with name '" + toolName + "' not found")); + else { + logger.warn("Ignore as a Tool with name '{}' not found", toolName); + } + + return Mono.empty(); }); } @@ -518,8 +527,10 @@ private McpRequestHandler toolsCallRequestHandler() { .findAny(); if (toolSpecification.isEmpty()) { - return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS, - "Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name()))); + return Mono.error(McpError.builder(McpSchema.ErrorCodes.INVALID_PARAMS) + .message("Unknown tool: invalid_tool_name") + .data("Tool not found: " + callToolRequest.name()) + .build()); } return toolSpecification.get().callHandler().apply(exchange, callToolRequest); @@ -747,32 +758,36 @@ private Optional findResou */ public Mono addPrompt(McpServerFeatures.AsyncPromptSpecification promptSpecification) { if (promptSpecification == null) { - return Mono.error(new McpError("Prompt specification must not be null")); + return Mono.error(new IllegalArgumentException("Prompt specification must not be null")); } if (this.serverCapabilities.prompts() == null) { - return Mono.error(new McpError("Server must be configured with prompt capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with prompt capabilities")); } return Mono.defer(() -> { - McpServerFeatures.AsyncPromptSpecification specification = this.prompts - .putIfAbsent(promptSpecification.prompt().name(), promptSpecification); - if (specification != null) { - return Mono.error( - new McpError("Prompt with name '" + promptSpecification.prompt().name() + "' already exists")); + var previous = this.prompts.put(promptSpecification.prompt().name(), promptSpecification); + if (previous != null) { + logger.warn("Replace existing Prompt with name '{}'", promptSpecification.prompt().name()); + } + else { + logger.debug("Added prompt handler: {}", promptSpecification.prompt().name()); } - - logger.debug("Added prompt handler: {}", promptSpecification.prompt().name()); - - // Servers that declared the listChanged capability SHOULD send a - // notification, - // when the list of available prompts changes if (this.serverCapabilities.prompts().listChanged()) { - return notifyPromptsListChanged(); + return this.notifyPromptsListChanged(); } + return Mono.empty(); }); } + /** + * List all registered prompts. + * @return A Flux stream of all registered prompts + */ + public Flux listPrompts() { + return Flux.fromIterable(this.prompts.values()).map(McpServerFeatures.AsyncPromptSpecification::prompt); + } + /** * Remove a prompt handler at runtime. * @param promptName The name of the prompt handler to remove @@ -780,10 +795,10 @@ public Mono addPrompt(McpServerFeatures.AsyncPromptSpecification promptSpe */ public Mono removePrompt(String promptName) { if (promptName == null) { - return Mono.error(new McpError("Prompt name must not be null")); + return Mono.error(new IllegalArgumentException("Prompt name must not be null")); } if (this.serverCapabilities.prompts() == null) { - return Mono.error(new McpError("Server must be configured with prompt capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with prompt capabilities")); } return Mono.defer(() -> { @@ -791,14 +806,15 @@ public Mono removePrompt(String promptName) { if (removed != null) { logger.debug("Removed prompt handler: {}", promptName); - // Servers that declared the listChanged capability SHOULD send a - // notification, when the list of available prompts changes if (this.serverCapabilities.prompts().listChanged()) { return this.notifyPromptsListChanged(); } return Mono.empty(); } - return Mono.error(new McpError("Prompt with name '" + promptName + "' not found")); + else { + logger.warn("Ignore as a Prompt with name '{}' not found", promptName); + } + return Mono.empty(); }); } @@ -834,8 +850,12 @@ private McpRequestHandler promptsGetRequestHandler() // Implement prompt retrieval logic here McpServerFeatures.AsyncPromptSpecification specification = this.prompts.get(promptRequest.name()); + if (specification == null) { - return Mono.error(new McpError("Prompt not found: " + promptRequest.name())); + return Mono.error(McpError.builder(ErrorCodes.INVALID_PARAMS) + .message("Invalid prompt name") + .data("Prompt not found: " + promptRequest.name()) + .build()); } return Mono.defer(() -> specification.promptHandler().apply(exchange, promptRequest)); diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java index 81b50eb2e..823aca41d 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java @@ -319,25 +319,24 @@ public Mono apply(McpTransportContext transportContext, McpSchem */ public Mono addTool(McpStatelessServerFeatures.AsyncToolSpecification toolSpecification) { if (toolSpecification == null) { - return Mono.error(new McpError("Tool specification must not be null")); + return Mono.error(new IllegalArgumentException("Tool specification must not be null")); } if (toolSpecification.tool() == null) { - return Mono.error(new McpError("Tool must not be null")); + return Mono.error(new IllegalArgumentException("Tool must not be null")); } if (toolSpecification.callHandler() == null) { - return Mono.error(new McpError("Tool call handler must not be null")); + return Mono.error(new IllegalArgumentException("Tool call handler must not be null")); } if (this.serverCapabilities.tools() == null) { - return Mono.error(new McpError("Server must be configured with tool capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with tool capabilities")); } var wrappedToolSpecification = withStructuredOutputHandling(this.jsonSchemaValidator, toolSpecification); return Mono.defer(() -> { - // Check for duplicate tool names - if (this.tools.stream().anyMatch(th -> th.tool().name().equals(wrappedToolSpecification.tool().name()))) { - return Mono.error( - new McpError("Tool with name '" + wrappedToolSpecification.tool().name() + "' already exists")); + // Remove tools with duplicate tool names first + if (this.tools.removeIf(th -> th.tool().name().equals(wrappedToolSpecification.tool().name()))) { + logger.warn("Replace existing Tool with name '{}'", wrappedToolSpecification.tool().name()); } this.tools.add(wrappedToolSpecification); @@ -347,6 +346,14 @@ public Mono addTool(McpStatelessServerFeatures.AsyncToolSpecification tool }); } + /** + * List all registered tools. + * @return A Flux stream of all registered tools + */ + public Flux listTools() { + return Flux.fromIterable(this.tools).map(McpStatelessServerFeatures.AsyncToolSpecification::tool); + } + /** * Remove a tool handler at runtime. * @param toolName The name of the tool handler to remove @@ -354,20 +361,22 @@ public Mono addTool(McpStatelessServerFeatures.AsyncToolSpecification tool */ public Mono removeTool(String toolName) { if (toolName == null) { - return Mono.error(new McpError("Tool name must not be null")); + return Mono.error(new IllegalArgumentException("Tool name must not be null")); } if (this.serverCapabilities.tools() == null) { - return Mono.error(new McpError("Server must be configured with tool capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with tool capabilities")); } return Mono.defer(() -> { - boolean removed = this.tools - .removeIf(toolSpecification -> toolSpecification.tool().name().equals(toolName)); - if (removed) { + if (this.tools.removeIf(toolSpecification -> toolSpecification.tool().name().equals(toolName))) { + logger.debug("Removed tool handler: {}", toolName); - return Mono.empty(); } - return Mono.error(new McpError("Tool with name '" + toolName + "' not found")); + else { + logger.warn("Ignore as a Tool with name '{}' not found", toolName); + } + + return Mono.empty(); }); } @@ -391,8 +400,10 @@ private McpStatelessRequestHandler toolsCallRequestHandler() { .findAny(); if (toolSpecification.isEmpty()) { - return Mono.error(new McpError(new JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INVALID_PARAMS, - "Unknown tool: invalid_tool_name", "Tool not found: " + callToolRequest.name()))); + return Mono.error(McpError.builder(McpSchema.ErrorCodes.INVALID_PARAMS) + .message("Unknown tool: invalid_tool_name") + .data("Tool not found: " + callToolRequest.name()) + .build()); } return toolSpecification.get().callHandler().apply(ctx, callToolRequest); @@ -593,26 +604,34 @@ private Optional */ public Mono addPrompt(McpStatelessServerFeatures.AsyncPromptSpecification promptSpecification) { if (promptSpecification == null) { - return Mono.error(new McpError("Prompt specification must not be null")); + return Mono.error(new IllegalArgumentException("Prompt specification must not be null")); } if (this.serverCapabilities.prompts() == null) { - return Mono.error(new McpError("Server must be configured with prompt capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with prompt capabilities")); } return Mono.defer(() -> { - McpStatelessServerFeatures.AsyncPromptSpecification specification = this.prompts - .putIfAbsent(promptSpecification.prompt().name(), promptSpecification); - if (specification != null) { - return Mono.error( - new McpError("Prompt with name '" + promptSpecification.prompt().name() + "' already exists")); + var previous = this.prompts.put(promptSpecification.prompt().name(), promptSpecification); + if (previous != null) { + logger.warn("Replace existing Prompt with name '{}'", promptSpecification.prompt().name()); + } + else { + logger.debug("Added prompt handler: {}", promptSpecification.prompt().name()); } - - logger.debug("Added prompt handler: {}", promptSpecification.prompt().name()); return Mono.empty(); }); } + /** + * List all registered prompts. + * @return A Flux stream of all registered prompts + */ + public Flux listPrompts() { + return Flux.fromIterable(this.prompts.values()) + .map(McpStatelessServerFeatures.AsyncPromptSpecification::prompt); + } + /** * Remove a prompt handler at runtime. * @param promptName The name of the prompt handler to remove @@ -620,10 +639,10 @@ public Mono addPrompt(McpStatelessServerFeatures.AsyncPromptSpecification */ public Mono removePrompt(String promptName) { if (promptName == null) { - return Mono.error(new McpError("Prompt name must not be null")); + return Mono.error(new IllegalArgumentException("Prompt name must not be null")); } if (this.serverCapabilities.prompts() == null) { - return Mono.error(new McpError("Server must be configured with prompt capabilities")); + return Mono.error(new IllegalStateException("Server must be configured with prompt capabilities")); } return Mono.defer(() -> { @@ -633,7 +652,11 @@ public Mono removePrompt(String promptName) { logger.debug("Removed prompt handler: {}", promptName); return Mono.empty(); } - return Mono.error(new McpError("Prompt with name '" + promptName + "' not found")); + else { + logger.warn("Ignore as a Prompt with name '{}' not found", promptName); + } + + return Mono.empty(); }); } @@ -662,7 +685,10 @@ private McpStatelessRequestHandler promptsGetRequestH // Implement prompt retrieval logic here McpStatelessServerFeatures.AsyncPromptSpecification specification = this.prompts.get(promptRequest.name()); if (specification == null) { - return Mono.error(new McpError("Prompt not found: " + promptRequest.name())); + return Mono.error(McpError.builder(ErrorCodes.INVALID_PARAMS) + .message("Invalid prompt name") + .data("Prompt not found: " + promptRequest.name()) + .build()); } return specification.promptHandler().apply(ctx, promptRequest); diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessSyncServer.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessSyncServer.java index 65833d135..6849eb8ed 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessSyncServer.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessSyncServer.java @@ -74,6 +74,14 @@ public void addTool(McpStatelessServerFeatures.SyncToolSpecification toolSpecifi .block(); } + /** + * List all registered tools. + * @return A list of all registered tools + */ + public List listTools() { + return this.asyncServer.listTools().collectList().block(); + } + /** * Remove a tool handler at runtime. * @param toolName The name of the tool handler to remove @@ -148,6 +156,14 @@ public void addPrompt(McpStatelessServerFeatures.SyncPromptSpecification promptS .block(); } + /** + * List all registered prompts. + * @return A list of all registered prompts + */ + public List listPrompts() { + return this.asyncServer.listPrompts().collectList().block(); + } + /** * Remove a prompt handler at runtime. * @param promptName The name of the prompt handler to remove diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java index 2852937ab..10f0e5a31 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java @@ -89,6 +89,14 @@ public void addTool(McpServerFeatures.SyncToolSpecification toolHandler) { .block(); } + /** + * List all registered tools. + * @return A list of all registered tools + */ + public List listTools() { + return this.asyncServer.listTools().collectList().block(); + } + /** * Remove a tool handler. * @param toolName The name of the tool handler to remove @@ -162,6 +170,14 @@ public void addPrompt(McpServerFeatures.SyncPromptSpecification promptSpecificat .block(); } + /** + * List all registered prompts. + * @return A list of all registered prompts + */ + public List listPrompts() { + return this.asyncServer.listPrompts().collectList().block(); + } + /** * Remove a prompt handler. * @param promptName The name of the prompt handler to remove diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java index b9ff041a9..11fb47e5f 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java @@ -282,10 +282,15 @@ private Mono handleIncomingRequest(McpSchema.JSONRPCR } return resultMono .map(result -> new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), result, null)) - .onErrorResume(error -> Mono.just(new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), - null, new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - error.getMessage(), null)))); // TODO: add error message - // through the data field + .onErrorResume(error -> { + McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (error instanceof McpError mcpError + && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() + // TODO: add error message through the data field + : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + error.getMessage(), null); + return Mono.just( + new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), null, jsonRpcError)); + }); }); } diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java index ec03dd424..f951131c9 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java @@ -179,9 +179,14 @@ public Mono responseStream(McpSchema.JSONRPCRequest jsonrpcRequest, McpStr .map(result -> new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, jsonrpcRequest.id(), result, null)) .onErrorResume(e -> { + McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (e instanceof McpError mcpError + && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() + // TODO: add error message through the data field + : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + e.getMessage(), null); + var errorResponse = new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, jsonrpcRequest.id(), - null, new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - e.getMessage(), null)); + null, jsonRpcError); return Mono.just(errorResponse); }) .flatMap(transport::sendMessage) diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index f8f17cdfb..aa68203dd 100644 --- a/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -145,13 +145,9 @@ void testAddDuplicateTool() { .tool(duplicateTool, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))) .build(); - StepVerifier - .create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolSpecification(duplicateTool, - (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))))) - .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); - }); + StepVerifier.create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolSpecification(duplicateTool, + (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))))) + .verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } @@ -172,10 +168,7 @@ void testAddDuplicateToolCall() { StepVerifier.create(mcpAsyncServer.addTool(McpServerFeatures.AsyncToolSpecification.builder() .tool(duplicateTool) .callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) - .build())).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); - }); + .build())).verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } @@ -269,9 +262,7 @@ void testRemoveNonexistentTool() { .capabilities(ServerCapabilities.builder().tools(true).build()) .build(); - StepVerifier.create(mcpAsyncServer.removeTool("nonexistent-tool")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Tool with name 'nonexistent-tool' not found"); - }); + StepVerifier.create(mcpAsyncServer.removeTool("nonexistent-tool")).verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } @@ -512,9 +503,6 @@ void testRemoveNonexistentResourceTemplate() { .capabilities(ServerCapabilities.builder().resources(true, false).build()) .build(); - // Removing a non-existent resource template should complete successfully (no - // error) - // as per the new implementation that just logs a warning StepVerifier.create(mcpAsyncServer.removeResourceTemplate("nonexistent://template/{id}")).verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); @@ -570,7 +558,8 @@ void testAddPromptWithNullSpecification() { StepVerifier.create(mcpAsyncServer.addPrompt((McpServerFeatures.AsyncPromptSpecification) null)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Prompt specification must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Prompt specification must not be null"); }); } @@ -585,7 +574,7 @@ void testAddPromptWithoutCapability() { .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content")))))); StepVerifier.create(serverWithoutPrompts.addPrompt(specification)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -596,7 +585,7 @@ void testRemovePromptWithoutCapability() { McpAsyncServer serverWithoutPrompts = prepareAsyncServerBuilder().serverInfo("test-server", "1.0.0").build(); StepVerifier.create(serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -626,10 +615,7 @@ void testRemoveNonexistentPrompt() { .capabilities(ServerCapabilities.builder().prompts(true).build()) .build(); - StepVerifier.create(mcpAsyncServer2.removePrompt("nonexistent-prompt")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) - .hasMessage("Prompt with name 'nonexistent-prompt' not found"); - }); + StepVerifier.create(mcpAsyncServer2.removePrompt("nonexistent-prompt")).verifyComplete(); assertThatCode(() -> mcpAsyncServer2.closeGracefully().block(Duration.ofSeconds(10))) .doesNotThrowAnyException(); diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index 619bb7aa4..976eb8c2c 100644 --- a/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -154,10 +154,9 @@ void testAddDuplicateTool() { .tool(duplicateTool, (exchange, args) -> new CallToolResult(List.of(), false)) .build(); - assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolSpecification(duplicateTool, + assertThatCode(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolSpecification(duplicateTool, (exchange, args) -> new CallToolResult(List.of(), false)))) - .isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + .doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } @@ -175,11 +174,10 @@ void testAddDuplicateToolCall() { .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) .build(); - assertThatThrownBy(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder() + assertThatCode(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder() .tool(duplicateTool) .callHandler((exchange, request) -> new CallToolResult(List.of(), false)) - .build())).isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + .build())).doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } @@ -272,8 +270,7 @@ void testRemoveNonexistentTool() { .capabilities(ServerCapabilities.builder().tools(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removeTool("nonexistent-tool")).isInstanceOf(McpError.class) - .hasMessage("Tool with name 'nonexistent-tool' not found"); + assertThatCode(() -> mcpSyncServer.removeTool("nonexistent-tool")).doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } @@ -496,9 +493,6 @@ void testRemoveNonexistentResourceTemplate() { .capabilities(ServerCapabilities.builder().resources(true, false).build()) .build(); - // Removing a non-existent resource template should complete successfully (no - // error) - // as per the new implementation that just logs a warning assertThatCode(() -> mcpSyncServer.removeResourceTemplate("nonexistent://template/{id}")) .doesNotThrowAnyException(); @@ -549,7 +543,7 @@ void testAddPromptWithNullSpecification() { .build(); assertThatThrownBy(() -> mcpSyncServer.addPrompt((McpServerFeatures.SyncPromptSpecification) null)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt specification must not be null"); } @@ -562,7 +556,8 @@ void testAddPromptWithoutCapability() { (exchange, req) -> new GetPromptResult("Test prompt description", List .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content"))))); - assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -570,7 +565,8 @@ void testAddPromptWithoutCapability() { void testRemovePromptWithoutCapability() { var serverWithoutPrompts = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build(); - assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -597,8 +593,7 @@ void testRemoveNonexistentPrompt() { .capabilities(ServerCapabilities.builder().prompts(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removePrompt("nonexistent-prompt")).isInstanceOf(McpError.class) - .hasMessage("Prompt with name 'nonexistent-prompt' not found"); + assertThatCode(() -> mcpSyncServer.removePrompt("nonexistent://template/{id}")).doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index 4c4e49dc5..c24bcd622 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -149,13 +149,9 @@ void testAddDuplicateTool() { .tool(duplicateTool, (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))) .build(); - StepVerifier - .create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolSpecification(duplicateTool, - (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))))) - .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); - }); + StepVerifier.create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolSpecification(duplicateTool, + (exchange, args) -> Mono.just(new CallToolResult(List.of(), false))))) + .verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } @@ -176,10 +172,7 @@ void testAddDuplicateToolCall() { StepVerifier.create(mcpAsyncServer.addTool(McpServerFeatures.AsyncToolSpecification.builder() .tool(duplicateTool) .callHandler((exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) - .build())).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); - }); + .build())).verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } @@ -273,9 +266,7 @@ void testRemoveNonexistentTool() { .capabilities(ServerCapabilities.builder().tools(true).build()) .build(); - StepVerifier.create(mcpAsyncServer.removeTool("nonexistent-tool")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Tool with name 'nonexistent-tool' not found"); - }); + StepVerifier.create(mcpAsyncServer.removeTool("nonexistent-tool")).verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } @@ -516,9 +507,6 @@ void testRemoveNonexistentResourceTemplate() { .capabilities(ServerCapabilities.builder().resources(true, false).build()) .build(); - // Removing a non-existent resource template should complete successfully (no - // error) - // as per the new implementation that just logs a warning StepVerifier.create(mcpAsyncServer.removeResourceTemplate("nonexistent://template/{id}")).verifyComplete(); assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); @@ -574,7 +562,8 @@ void testAddPromptWithNullSpecification() { StepVerifier.create(mcpAsyncServer.addPrompt((McpServerFeatures.AsyncPromptSpecification) null)) .verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class).hasMessage("Prompt specification must not be null"); + assertThat(error).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Prompt specification must not be null"); }); } @@ -589,7 +578,7 @@ void testAddPromptWithoutCapability() { .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content")))))); StepVerifier.create(serverWithoutPrompts.addPrompt(specification)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -600,7 +589,7 @@ void testRemovePromptWithoutCapability() { McpAsyncServer serverWithoutPrompts = prepareAsyncServerBuilder().serverInfo("test-server", "1.0.0").build(); StepVerifier.create(serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) + assertThat(error).isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); }); } @@ -630,10 +619,7 @@ void testRemoveNonexistentPrompt() { .capabilities(ServerCapabilities.builder().prompts(true).build()) .build(); - StepVerifier.create(mcpAsyncServer2.removePrompt("nonexistent-prompt")).verifyErrorSatisfies(error -> { - assertThat(error).isInstanceOf(McpError.class) - .hasMessage("Prompt with name 'nonexistent-prompt' not found"); - }); + StepVerifier.create(mcpAsyncServer2.removePrompt("nonexistent-prompt")).verifyComplete(); assertThatCode(() -> mcpAsyncServer2.closeGracefully().block(Duration.ofSeconds(10))) .doesNotThrowAnyException(); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index ff37abd74..591f750cb 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -153,10 +153,9 @@ void testAddDuplicateTool() { .tool(duplicateTool, (exchange, args) -> new CallToolResult(List.of(), false)) .build(); - assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolSpecification(duplicateTool, + assertThatCode(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolSpecification(duplicateTool, (exchange, args) -> new CallToolResult(List.of(), false)))) - .isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + .doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } @@ -174,11 +173,10 @@ void testAddDuplicateToolCall() { .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) .build(); - assertThatThrownBy(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder() + assertThatCode(() -> mcpSyncServer.addTool(McpServerFeatures.SyncToolSpecification.builder() .tool(duplicateTool) .callHandler((exchange, request) -> new CallToolResult(List.of(), false)) - .build())).isInstanceOf(McpError.class) - .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + .build())).doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } @@ -271,8 +269,7 @@ void testRemoveNonexistentTool() { .capabilities(ServerCapabilities.builder().tools(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removeTool("nonexistent-tool")).isInstanceOf(McpError.class) - .hasMessage("Tool with name 'nonexistent-tool' not found"); + assertThatCode(() -> mcpSyncServer.removeTool("nonexistent-tool")).doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } @@ -495,9 +492,6 @@ void testRemoveNonexistentResourceTemplate() { .capabilities(ServerCapabilities.builder().resources(true, false).build()) .build(); - // Removing a non-existent resource template should complete successfully (no - // error) - // as per the new implementation that just logs a warning assertThatCode(() -> mcpSyncServer.removeResourceTemplate("nonexistent://template/{id}")) .doesNotThrowAnyException(); @@ -548,7 +542,7 @@ void testAddPromptWithNullSpecification() { .build(); assertThatThrownBy(() -> mcpSyncServer.addPrompt((McpServerFeatures.SyncPromptSpecification) null)) - .isInstanceOf(McpError.class) + .isInstanceOf(IllegalArgumentException.class) .hasMessage("Prompt specification must not be null"); } @@ -561,7 +555,8 @@ void testAddPromptWithoutCapability() { (exchange, req) -> new GetPromptResult("Test prompt description", List .of(new PromptMessage(McpSchema.Role.ASSISTANT, new McpSchema.TextContent("Test content"))))); - assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.addPrompt(specification)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -569,7 +564,8 @@ void testAddPromptWithoutCapability() { void testRemovePromptWithoutCapability() { var serverWithoutPrompts = prepareSyncServerBuilder().serverInfo("test-server", "1.0.0").build(); - assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)).isInstanceOf(McpError.class) + assertThatThrownBy(() -> serverWithoutPrompts.removePrompt(TEST_PROMPT_NAME)) + .isInstanceOf(IllegalStateException.class) .hasMessage("Server must be configured with prompt capabilities"); } @@ -596,8 +592,7 @@ void testRemoveNonexistentPrompt() { .capabilities(ServerCapabilities.builder().prompts(true).build()) .build(); - assertThatThrownBy(() -> mcpSyncServer.removePrompt("nonexistent-prompt")).isInstanceOf(McpError.class) - .hasMessage("Prompt with name 'nonexistent-prompt' not found"); + assertThatCode(() -> mcpSyncServer.removePrompt("nonexistent://template/{id}")).doesNotThrowAnyException(); assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } From a360b7d930d8c261275fcf428442ba9c8ab8bb1f Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sun, 28 Sep 2025 14:43:23 +0200 Subject: [PATCH 2/3] feat: improve error handling by extracting root cause messages - Add findRootCause() utility method utility to traverse exception chains - Update McpServerSession and McpStreamableServerSession to use root cause messages - Ensures meaningful error messages are returned instead of wrapped exception messages Signed-off-by: Christian Tzolov --- .../main/java/io/modelcontextprotocol/spec/McpError.java | 9 +++++++++ .../io/modelcontextprotocol/spec/McpServerSession.java | 9 ++++++--- .../spec/McpStreamableServerSession.java | 3 +-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java index 5e6f5990b..33a8972ad 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java @@ -78,4 +78,13 @@ public McpError build() { } + public static Throwable findRootCause(Throwable throwable) { + Assert.notNull(throwable, "throwable must not be null"); + Throwable rootCause = throwable; + while (rootCause.getCause() != null && rootCause.getCause() != rootCause) { + rootCause = rootCause.getCause(); + } + return rootCause; + } + } \ No newline at end of file diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java index 11fb47e5f..08bd5cae8 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java @@ -224,9 +224,12 @@ public Mono handle(McpSchema.JSONRPCMessage message) { else if (message instanceof McpSchema.JSONRPCRequest request) { logger.debug("Received request: {}", request); return handleIncomingRequest(request, transportContext).onErrorResume(error -> { + McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (error instanceof McpError mcpError + && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() + : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, + McpError.findRootCause(error).getMessage(), null); var errorResponse = new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), null, - new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - error.getMessage(), null)); + jsonRpcError); // TODO: Should the error go to SSE or back as POST return? return this.transport.sendMessage(errorResponse).then(Mono.empty()); }).flatMap(this.transport::sendMessage); @@ -287,7 +290,7 @@ private Mono handleIncomingRequest(McpSchema.JSONRPCR && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() // TODO: add error message through the data field : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - error.getMessage(), null); + McpError.findRootCause(error).getMessage(), null); return Mono.just( new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), null, jsonRpcError)); }); diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java index f951131c9..bfc9acdf3 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java @@ -181,9 +181,8 @@ public Mono responseStream(McpSchema.JSONRPCRequest jsonrpcRequest, McpStr .onErrorResume(e -> { McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (e instanceof McpError mcpError && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() - // TODO: add error message through the data field : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - e.getMessage(), null); + McpError.findRootCause(e).getMessage(), null); var errorResponse = new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, jsonrpcRequest.id(), null, jsonRpcError); From af2f6e5e51f2a52df55a1a7ffdfbd28cce9f8f83 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Mon, 29 Sep 2025 15:02:24 +0200 Subject: [PATCH 3/3] improve the json rpc error content Signed-off-by: Christian Tzolov --- .../modelcontextprotocol/spec/McpError.java | 32 ++++++++++-- .../spec/McpServerSession.java | 4 +- .../spec/McpStreamableServerSession.java | 2 +- ...stractMcpClientServerIntegrationTests.java | 49 ++++++++++++------- ...stractMcpClientServerIntegrationTests.java | 49 ++++++++++++------- 5 files changed, 92 insertions(+), 44 deletions(-) diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java index 33a8972ad..d6e549fdc 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpError.java @@ -38,11 +38,12 @@ public JSONRPCError getJsonRpcError() { @Override public String toString() { - var message = super.toString(); + var builder = new StringBuilder(super.toString()); if (jsonRpcError != null) { - return message + jsonRpcError.toString(); + builder.append("\n"); + builder.append(jsonRpcError.toString()); } - return message; + return builder.toString(); } public static Builder builder(int errorCode) { @@ -87,4 +88,29 @@ public static Throwable findRootCause(Throwable throwable) { return rootCause; } + public static String aggregateExceptionMessages(Throwable throwable) { + Assert.notNull(throwable, "throwable must not be null"); + + StringBuilder messages = new StringBuilder(); + Throwable current = throwable; + + while (current != null) { + if (messages.length() > 0) { + messages.append("\n Caused by: "); + } + + messages.append(current.getClass().getSimpleName()); + if (current.getMessage() != null) { + messages.append(": ").append(current.getMessage()); + } + + if (current.getCause() == current) { + break; + } + current = current.getCause(); + } + + return messages.toString(); + } + } \ No newline at end of file diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java index 08bd5cae8..241f7d8b5 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpServerSession.java @@ -227,7 +227,7 @@ else if (message instanceof McpSchema.JSONRPCRequest request) { McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (error instanceof McpError mcpError && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - McpError.findRootCause(error).getMessage(), null); + error.getMessage(), McpError.aggregateExceptionMessages(error)); var errorResponse = new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), null, jsonRpcError); // TODO: Should the error go to SSE or back as POST return? @@ -290,7 +290,7 @@ private Mono handleIncomingRequest(McpSchema.JSONRPCR && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() // TODO: add error message through the data field : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - McpError.findRootCause(error).getMessage(), null); + error.getMessage(), McpError.aggregateExceptionMessages(error)); return Mono.just( new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, request.id(), null, jsonRpcError)); }); diff --git a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java index bfc9acdf3..95f8959f5 100644 --- a/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java +++ b/mcp-core/src/main/java/io/modelcontextprotocol/spec/McpStreamableServerSession.java @@ -182,7 +182,7 @@ public Mono responseStream(McpSchema.JSONRPCRequest jsonrpcRequest, McpStr McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (e instanceof McpError mcpError && mcpError.getJsonRpcError() != null) ? mcpError.getJsonRpcError() : new McpSchema.JSONRPCResponse.JSONRPCError(McpSchema.ErrorCodes.INTERNAL_ERROR, - McpError.findRootCause(e).getMessage(), null); + e.getMessage(), McpError.aggregateExceptionMessages(e)); var errorResponse = new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, jsonrpcRequest.id(), null, jsonRpcError); diff --git a/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java b/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java index 3fd0f2cfb..603324631 100644 --- a/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java +++ b/mcp-core/src/test/java/io/modelcontextprotocol/server/AbstractMcpClientServerIntegrationTests.java @@ -146,8 +146,9 @@ void testCreateMessageSuccess(String clientType) { CreateMessageResult.StopReason.STOP_SEQUENCE); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); AtomicReference samplingResult = new AtomicReference<>(); @@ -224,8 +225,9 @@ void testCreateMessageWithRequestTimeoutSuccess(String clientType) throws Interr // Server - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); AtomicReference samplingResult = new AtomicReference<>(); @@ -300,8 +302,9 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt CreateMessageResult.StopReason.STOP_SEQUENCE); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); McpServerFeatures.AsyncToolSpecification tool = McpServerFeatures.AsyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) @@ -393,8 +396,9 @@ void testCreateElicitationSuccess(String clientType) { Map.of("message", request.message())); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); McpServerFeatures.AsyncToolSpecification tool = McpServerFeatures.AsyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) @@ -448,8 +452,9 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) { return new ElicitResult(ElicitResult.Action.ACCEPT, Map.of("message", request.message())); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); AtomicReference resultRef = new AtomicReference<>(); @@ -520,7 +525,7 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { return new ElicitResult(ElicitResult.Action.ACCEPT, Map.of("message", request.message())); }; - CallToolResult callResponse = new CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), null); + CallToolResult callResponse = CallToolResult.builder().addContent(new TextContent("CALL RESPONSE")).build(); AtomicReference resultRef = new AtomicReference<>(); @@ -761,7 +766,9 @@ void testToolCallSuccess(String clientType) { var clientBuilder = clientBuilders.get(clientType); var responseBodyIsNullOrBlank = new AtomicBoolean(false); - var callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), null); + var callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE; ctx=importantValue")) + .build(); McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) .callHandler((exchange, request) -> { @@ -832,8 +839,7 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { 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. + // 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"); @@ -853,8 +859,9 @@ void testToolCallSuccessWithTranportContextExtraction(String clientType) { var transportContextIsEmpty = new AtomicBoolean(false); var responseBodyIsNullOrBlank = new AtomicBoolean(false); - var expectedCallResponse = new McpSchema.CallToolResult( - List.of(new McpSchema.TextContent("CALL RESPONSE; ctx=value")), null); + var expectedCallResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE; ctx=value")) + .build(); McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) .callHandler((exchange, request) -> { @@ -872,8 +879,9 @@ void testToolCallSuccessWithTranportContextExtraction(String clientType) { e.printStackTrace(); } - return new McpSchema.CallToolResult( - List.of(new McpSchema.TextContent("CALL RESPONSE; ctx=" + ctxValue)), null); + return McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE; ctx=" + ctxValue)) + .build(); }) .build(); @@ -906,7 +914,10 @@ void testToolListChangeHandlingSuccess(String clientType) { var clientBuilder = clientBuilders.get(clientType); - var callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), null); + var callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); + McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) .callHandler((exchange, request) -> { diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java index 1b34c43a7..37a1ef31d 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/AbstractMcpClientServerIntegrationTests.java @@ -150,8 +150,9 @@ void testCreateMessageSuccess(String clientType) { CreateMessageResult.StopReason.STOP_SEQUENCE); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); AtomicReference samplingResult = new AtomicReference<>(); @@ -228,8 +229,9 @@ void testCreateMessageWithRequestTimeoutSuccess(String clientType) throws Interr // Server - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); AtomicReference samplingResult = new AtomicReference<>(); @@ -304,8 +306,9 @@ void testCreateMessageWithRequestTimeoutFail(String clientType) throws Interrupt CreateMessageResult.StopReason.STOP_SEQUENCE); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); McpServerFeatures.AsyncToolSpecification tool = McpServerFeatures.AsyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) @@ -397,8 +400,9 @@ void testCreateElicitationSuccess(String clientType) { Map.of("message", request.message())); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); McpServerFeatures.AsyncToolSpecification tool = McpServerFeatures.AsyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) @@ -452,8 +456,9 @@ void testCreateElicitationWithRequestTimeoutSuccess(String clientType) { return new ElicitResult(ElicitResult.Action.ACCEPT, Map.of("message", request.message())); }; - CallToolResult callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), - null); + CallToolResult callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); AtomicReference resultRef = new AtomicReference<>(); @@ -524,7 +529,7 @@ void testCreateElicitationWithRequestTimeoutFail(String clientType) { return new ElicitResult(ElicitResult.Action.ACCEPT, Map.of("message", request.message())); }; - CallToolResult callResponse = new CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), null); + CallToolResult callResponse = CallToolResult.builder().addContent(new TextContent("CALL RESPONSE")).build(); AtomicReference resultRef = new AtomicReference<>(); @@ -765,7 +770,9 @@ void testToolCallSuccess(String clientType) { var clientBuilder = clientBuilders.get(clientType); var responseBodyIsNullOrBlank = new AtomicBoolean(false); - var callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), null); + var callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE; ctx=importantValue")) + .build(); McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) .callHandler((exchange, request) -> { @@ -836,8 +843,7 @@ void testThrowingToolCallIsCaughtBeforeTimeout(String clientType) { 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. + // 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"); @@ -857,8 +863,9 @@ void testToolCallSuccessWithTranportContextExtraction(String clientType) { var transportContextIsEmpty = new AtomicBoolean(false); var responseBodyIsNullOrBlank = new AtomicBoolean(false); - var expectedCallResponse = new McpSchema.CallToolResult( - List.of(new McpSchema.TextContent("CALL RESPONSE; ctx=value")), null); + var expectedCallResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE; ctx=value")) + .build(); McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) .callHandler((exchange, request) -> { @@ -876,8 +883,9 @@ void testToolCallSuccessWithTranportContextExtraction(String clientType) { e.printStackTrace(); } - return new McpSchema.CallToolResult( - List.of(new McpSchema.TextContent("CALL RESPONSE; ctx=" + ctxValue)), null); + return McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE; ctx=" + ctxValue)) + .build(); }) .build(); @@ -910,7 +918,10 @@ void testToolListChangeHandlingSuccess(String clientType) { var clientBuilder = clientBuilders.get(clientType); - var callResponse = new McpSchema.CallToolResult(List.of(new McpSchema.TextContent("CALL RESPONSE")), null); + var callResponse = McpSchema.CallToolResult.builder() + .addContent(new McpSchema.TextContent("CALL RESPONSE")) + .build(); + McpServerFeatures.SyncToolSpecification tool1 = McpServerFeatures.SyncToolSpecification.builder() .tool(Tool.builder().name("tool1").description("tool1 description").inputSchema(EMPTY_JSON_SCHEMA).build()) .callHandler((exchange, request) -> {