refactor: improve MCP server resilience and error handling #579
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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 <christian.tzolov@broadcom.com>
tzolov
commented
Sep 28, 2025
| error.getMessage(), null)))); // TODO: add error message | ||
| // through the data field | ||
| .onErrorResume(error -> { | ||
| McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (error instanceof McpError mcpError |
There was a problem hiding this comment.
Propagate existing JSONRPCError instead of wrapping it
tzolov
commented
Sep 28, 2025
| .map(result -> new McpSchema.JSONRPCResponse(McpSchema.JSONRPC_VERSION, jsonrpcRequest.id(), result, | ||
| null)) | ||
| .onErrorResume(e -> { | ||
| McpSchema.JSONRPCResponse.JSONRPCError jsonRpcError = (e instanceof McpError mcpError |
There was a problem hiding this comment.
Propagate existing JSONRPCError instead of wrapp
tzolov
commented
Sep 28, 2025
| public Mono<Void> 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")); |
There was a problem hiding this comment.
Note that those are runtime server errors that are not propagated over the wire to the client.
- 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 <christian.tzolov@broadcom.com>
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Improve MCP server resilience and error handling by replacing custom exceptions with standard Java exceptions, making tool/prompt registration more forgiving, adding listing capabilities, and implementing idempotent removal operations.
Motivation and Context
This change addresses several usability and robustness issues in the MCP Java SDK:
McpErrorfor validation errors that should use standard Java exceptionsThese issues made the API less developer-friendly and harder to use in dynamic scenarios where tools and prompts might be reconfigured at runtime.
How Has This Been Tested?
listTools()andlistPrompts()methods return correct resultsBreaking Changes
Minimal breaking changes:
McpErrortoIllegalArgumentException/IllegalStateException)Most users should not be affected as the core API remains the same. Users who were specifically catching
McpErrorfor validation scenarios may need to update their exception handling.Types of changes
Checklist
Additional context
IllegalArgumentException,IllegalStateException) for validation errors while preservingMcpErrorfor protocol-specific errors using the builder patternlistTools()andlistPrompts()methods to all server variants (async, sync, stateless) returningFlux/Listof registered items for introspectionMcpErrorJSON-RPC errors while falling back to generic internal errors