Skip to content

Conversation

@tonyxwz
Copy link

@tonyxwz tonyxwz commented Dec 3, 2025

Motivation and Context

Returns the error when initialization response is a json rpc error. Otherwise the client would be waiting in the loop and hangs until timeout.

fixes: #563

Before the change:

  • If the server sent a JSON-RPC error during initialization, it fell through to the catch-all _ pattern
  • The error was only logged with tracing::warn! and ignored
  • The client would hang waiting for a valid response that never comes
  • User gets a timeout or connection closed error instead of the actual error

After the change:

  • JSON-RPC error is caught and converted to ClientInitializeError::JsonRpcError
  • The error propagates up through the ? operator at line 241: .await?
  • Returns immediately from serve_client_with_ct_inner with the error
  • User receives Err(ClientInitializeError::JsonRpcError(error_data)) from .serve()
  • User can see the actual error code and message from the server

How Has This Been Tested?

Breaking Changes

This is not a breaking change. But it should fix the issues where the client response with a default error message when the session connection failed.

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

Additional context

@github-actions github-actions bot added T-test Testing related changes T-core Core library changes T-service Service layer changes labels Dec 3, 2025
@tonyxwz tonyxwz marked this pull request as ready for review December 3, 2025 13:48
@alexhancock alexhancock self-requested a review December 15, 2025 18:18
alexhancock
alexhancock previously approved these changes Dec 15, 2025
Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

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

Good catch - thank you

@alexhancock
Copy link
Collaborator

@tonyxwz Can you run cargo fmt? I should really add this to a precommit hook. Sorry!

@tonyxwz
Copy link
Author

tonyxwz commented Dec 15, 2025

Yes, will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-service Service layer changes T-test Testing related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle JsonRpcError in initialize

2 participants