Skip to content

If a handler throws McpError, use its values for the RPC error #465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Randgalt
Copy link
Contributor

@Randgalt Randgalt commented Aug 7, 2025

Handlers should be able to throw RPC errors and McpError is the right exception for that.

Improve DefaultMcpStatelessServerHandler error handler to check if the exception is McpError and, if so, use it to build the RPC error result instead of re-writing as INTERNAL_ERROR.

Motivation and Context

Handlers will want to send the client errors such as not found, etc. The MCP spec allows for RPC errors to be returned.

How Has This Been Tested?

New test, testThrownMcpError() in HttpServletStatelessIntegrationTests.

Breaking Changes

none

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@tzolov
Copy link
Contributor

tzolov commented Aug 7, 2025

Thanks @Randgalt . I like this.
Also we are working toward removing the McpError(Object) constructor (having it was a mistake), leaving only the McpError(JSONRPCError jsonRpcError)

@tzolov tzolov self-assigned this Aug 7, 2025
@tzolov tzolov added this to the 0.12.0 milestone Aug 7, 2025
.onErrorResume(t -> {
McpSchema.JSONRPCResponse.JSONRPCError error;
if (t instanceof McpError mcpError) {
error = mcpError.getJsonRpcError();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until we remove the McpError(Object) constructor we need to check that the getJsonRpcError() is not null otherwise create new JSONRCPError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Handlers should be able to throw RPC errors and `McpError` is the
right exception for that.

Improve `DefaultMcpStatelessServerHandler` error handler to check if the
exception is `McpError` and, if so, use it to build the RPC error
result instead of re-writing as `INTERNAL_ERROR`.
@Randgalt Randgalt force-pushed the jordanz/passthrough-mcp-error branch from b1cc84b to 11ec3e2 Compare August 8, 2025 06:04
@Randgalt Randgalt requested a review from tzolov August 8, 2025 06:05
@tzolov tzolov merged commit a14ef42 into modelcontextprotocol:main Aug 8, 2025
1 check passed
@tzolov
Copy link
Contributor

tzolov commented Aug 8, 2025

back-ported to 0.11.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants