Skip to content

Conversation

dak2
Copy link
Contributor

@dak2 dak2 commented Jun 21, 2025

Motivation and Context

Even though the tools/call request to notification_tool is successful and an MCP::Tool::Response is returned, the logger still prints Error:.

% ruby examples/streamable_http_client.rb
=== MCP SSE Test Client ===

[CLIENT] INFO 15:41:04.696 - Initializing session...
[CLIENT] INFO 15:41:04.698 - Session initialized: SESSION_ID
[CLIENT] INFO 15:41:04.698 - Server info: {"name" => "sse_test_server", "version" => "0.1.0"}
[CLIENT] INFO 15:41:04.698 - Connecting to SSE stream...
[CLIENT] INFO 15:41:04.698 - SSE stream connected successfully

=== Available Actions ===
1. Send custom notification
2. Test echo
3. List tools
0. Exit

Choose an action: 1
Enter notification message: notification test
Enter delay in seconds (0 for immediate):
[CLIENT] INFO 15:41:11.248 - SSE data: {"jsonrpc":"2.0","id":"0f657eeb-3e05-47bb-b7b4-898e2cb503af","result":{"content":[{"type":"text","text":"Notification: notification test (timestamp: 2025-06-21T15:41:11+09:00)"}],"isError":false}}
[CLIENT] ERROR 15:41:11.249 - Error:

=== Available Actions ===
1. Send custom notification
2. Test echo
3. List tools
0. Exit

It is probably trying to output an MCP::Tool::Response, but these appear in the SSE stream. Therefore, the response it is handling this time should either be accepted or an error.

I modified it so that it could handle them.

How Has This Been Tested?

I have confirmed by running the modified examples/streamable_http_client.rb that the tools/call request to notification_tool is successful, but does not print an error in the logger.

Breaking Changes

Nothing

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

@dak2 dak2 force-pushed the fix-error-handling-when-sending-notification branch from 9a4fa58 to efb7689 Compare June 21, 2025 06:52
if response[:body]["result"]
logger.info("Notification tool response: #{response[:body]["result"]["content"]}")
if response[:body]["accepted"]
logger.info("Notification was sent successfully")
Copy link
Member

Choose a reason for hiding this comment

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

I’ve confirmed the behavior, and indeed the response here is {"accepted" => true}.
https://github.com/modelcontextprotocol/ruby-sdk/blob/0fe0289/lib/mcp/server/transports/streamable_http_transport.rb#L205

Just one thing. The message seems quite simple. What do you think?

Suggested change
logger.info("Notification was sent successfully")
logger.info("Notification sent successfully")

Copy link
Contributor Author

@dak2 dak2 Jun 21, 2025

Choose a reason for hiding this comment

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

…ble_http_client.rb`

Even though the 'tools/call' request to 'notification_tool' is successful and an 'MCP::Tool::Response' is returned, the logger still prints 'Error:'.

```
% ruby examples/streamable_http_client.rb
=== MCP SSE Test Client ===

[CLIENT] INFO 15:41:04.696 - Initializing session...
[CLIENT] INFO 15:41:04.698 - Session initialized: SESSION_ID
[CLIENT] INFO 15:41:04.698 - Server info: {"name" => "sse_test_server", "version" => "0.1.0"}
[CLIENT] INFO 15:41:04.698 - Connecting to SSE stream...
[CLIENT] INFO 15:41:04.698 - SSE stream connected successfully

=== Available Actions ===
1. Send custom notification
2. Test echo
3. List tools
0. Exit

Choose an action: 1
Enter notification message: notification test
Enter delay in seconds (0 for immediate):
[CLIENT] INFO 15:41:11.248 - SSE data: {"jsonrpc":"2.0","id":"0f657eeb-3e05-47bb-b7b4-898e2cb503af","result":{"content":[{"type":"text","text":"Notification: notification test (timestamp: 2025-06-21T15:41:11+09:00)"}],"isError":false}}
[CLIENT] ERROR 15:41:11.249 - Error:

=== Available Actions ===
1. Send custom notification
2. Test echo
3. List tools
0. Exit
```

It is probably trying to output an MCP::Tool::Response, but these appear in the SSE stream. Therefore, the response it is handling this time should either be accepted or an error.

I modified it so that it could handle them.
@dak2 dak2 force-pushed the fix-error-handling-when-sending-notification branch from efb7689 to 7a5bd7c Compare June 21, 2025 10:38
@dak2 dak2 requested a review from koic June 21, 2025 10:40
@koic koic merged commit b28a477 into modelcontextprotocol:main Jun 21, 2025
5 checks passed
@koic
Copy link
Member

koic commented Jun 21, 2025

Thanks!

@dak2 dak2 deleted the fix-error-handling-when-sending-notification branch July 1, 2025 05:21
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