Skip to content

fix: accept 200 with empty body notification responses in addition to 202#849

Merged
alexhancock merged 1 commit into
mainfrom
fix-empty-2xx-notification
May 14, 2026
Merged

fix: accept 200 with empty body notification responses in addition to 202#849
alexhancock merged 1 commit into
mainfrom
fix-empty-2xx-notification

Conversation

@alexhancock
Copy link
Copy Markdown
Contributor

Motivation and Context

Some Streamable HTTP MCP servers return 200 OK with an empty body for JSON-RPC notifications like notifications/initialized, instead of the spec-required 202 Accepted. Previously this could fail with UnexpectedContentType(None). This change treats empty successful responses to notifications/responses/errors as accepted.

How Has This Been Tested?

Added a focused test for an empty 200 OK response to a client notification.

Ran:

cargo test -p rmcp --test test_streamable_http_empty_2xx_notification --features client,transport-streamable-http-client,transport-streamable-http-client-reqwest
cargo fmt -- --check
git diff --check

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](https://modelcontextprotocol.io)
  • 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

This improves compatibility with non-compliant servers while preserving the existing behavior for normal JSON/SSE responses.

@alexhancock alexhancock requested a review from a team as a code owner May 14, 2026 14:24
@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes T-transport Transport layer changes labels May 14, 2026
@alexhancock alexhancock changed the title fix: accept 200 with empty body in response to notifications in addit… fix: accept 200 with empty body notification responses in addition to 202 May 14, 2026
jamadeo
jamadeo previously approved these changes May 14, 2026
.get(HEADER_SESSION_ID)
.and_then(|v| v.to_str().ok())
.map(|s| s.to_string());
if status.is_success()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the past, we've opted to be pretty strict about non-compliant clients. This does have the benefit of exposing issues that other tools might mask.

But other than that, I think it is probably harmless to accept empty 200s as equivalent to 202s.

I would suggest putting a comment note here though, just to state we're deliberately allowing something outside of the spec.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a comment

@alexhancock alexhancock merged commit cc66e30 into main May 14, 2026
18 checks passed
@alexhancock alexhancock deleted the fix-empty-2xx-notification branch May 14, 2026 18:52
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-test Testing related changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants