Skip to content

Process JSONRPCRequest with default param (#42) #46

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

JsFlo
Copy link
Contributor

@JsFlo JsFlo commented Feb 21, 2025

Fixes issue #42 by setting the default to JSONRPTCRequest.params from null to EmptyJsonObject. This makes it so that the absence of params in a request is treated as an empty array which is what the Python sdk also does.

Motivation and Context

This makes it so that these two request bodies are treated similarly where previously not including params would result in an empty response.

{"jsonrpc": "2.0", "method": "tools/list","params": {}, "id": 1}
and
{"jsonrpc": "2.0", "method": "tools/list", "id": 1}

How Has This Been Tested?

I tested this with the sample Kotlin server project (with a bit of modification to update sample to use more up to date code) and a basic python SSE client.

I would use the python client to get the sessionId and then I would run

curl -v -X POST "http://localhost:3001/message?sessionId=9799477a-945f-4498-ac8e-fc9156d5898f" \
     -H "Accept: text/event-stream" \
     -H "Content-Type: application/json" \
     -d '{"jsonrpc": "2.0", "method": "tools/list","params": {}, "id": 1}'

with "params": {}, I would get the listed tools and without it I would get an empty response(EmptyRequestResult).

I also added a unit test that would fail before my changes (would be EmptyRequestResult) and pass after my changes

Breaking Changes

No

Types of changes

  • [x ] 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)
    • this will technically change behavior from certain requests getting empty results to those requests getting correct results
  • 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
  • [x ] I have added or updated documentation as needed

Copy link
Contributor

@e5l e5l left a comment

Choose a reason for hiding this comment

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

Nice catch, looks good

@e5l e5l enabled auto-merge (squash) February 21, 2025 17:16
auto-merge was automatically disabled February 21, 2025 20:18

Head branch was pushed to by a user without write access

@JsFlo JsFlo force-pushed the joseflores/42_defaultParamsEmptyJsonObj branch from 4373649 to d7efe41 Compare February 21, 2025 20:18
@e5l e5l enabled auto-merge (squash) February 22, 2025 16:34
@e5l e5l merged commit 78ef0f9 into modelcontextprotocol:main Feb 23, 2025
1 check passed
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