Skip to content

Fix httpstreamable errorcode #1194

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix httpstreamable errorcode #1194

wants to merge 4 commits into from
+22 −0

Conversation

CriticalGameEror
Copy link

Added additional if statements to catch a 400 bad request error messages within the streamable_http.py file

Motivation and Context

This change is needed to fix an issue with the current way that a client uses streamable HTTP to connect to a MCP server. Currently, an unavoidable exception is raised when creating a SessionGroup and adding a steamable HTTP connection to an MCP server that returns an error 4xx code when a primitive (prompts, resources and tools) is requested. As long as at least one of these primitives returns an error 4xx code (except a 404 error), it will cause the exception, which crashes the MCP client.

In cases with independent sessions outside of SessionGroup, this error can be handled by the developer building the MCP client. However, when using SessionGroup, the developer cannot handle these errors, and so cannot prevent the program from crashing. This is because the SessionGroup automatically sends web requests to prompts, resources and tools without checking if they are supported, which I think goes against the specification.

The change is needed to ensure that an MCP client doesn't crash when an MCP server sends an error code for a bad request (which it can do as defined in the specifciation). The change was made to the streamable_http.py file, as this avoids a possible breaking change if the SessionGroup was changed instead.

How Has This Been Tested?

This has been tested with the AWS Knowledge MCP Server, which returns an error code 400 when interacting with a prompt or resource primitive.

Breaking Changes

No breaking changes. The only possible scenario of a breaking change is if a developer relies on the httpx status error exception currently raised to detect an error 400 code for an unsupported capability. In this case, the developer should be following the specification, and check if a capability is supported before it is requested.

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

I apologise in advance if I have missunderstood anything about the code regarding this issue or anything about the MCP specification. Please correct me on any errors I have made so that I can learn from them.

@CriticalGameEror
fix: ensures MCP client can handle 4xx error codes
2f365d8 
Ensures that any error code response from the MCP server is handled.
@CriticalGameEror
refactor: added a custom error message
c143464 
This ensures the error message is more specific and focused to a 400 bad request error only
@CriticalGameEror
refactor: minor linting adjustments
3a45353
@CriticalGameEror CriticalGameEror requested a review from a team as a code owner July 23, 2025 22:34
@CriticalGameEror CriticalGameEror requested a review from ochafik July 23, 2025 22:34
@Kludex
Copy link
Member

Kludex commented Jul 26, 2025

It seems we are missing a try..except on a lower level that would send the JSON RPC message instead of crashing the server.

@CriticalGameEror
Copy link
Author

@Kludex

It seems we are missing a try..except on a lower level that would send the JSON RPC message instead of crashing the server.

Just a small correction to avoid confusion. The issue is entirely related with the MCP client. The MCP server does not crash in any way, only the client is affected.

python-sdk/src/mcp/client/streamable_http.py

Lines 253 to 299 in 3a45353

async def _handle_post_request(self, ctx: RequestContext) -> None:
"""Handle a POST request with response processing."""
headers = self._prepare_request_headers(ctx.headers)
message = ctx.session_message.message
is_initialization = self._is_initialization_request(message)
async with ctx.client.stream(
"POST",
self.url,
json=message.model_dump(by_alias=True, mode="json", exclude_none=True),
headers=headers,
) as response:
if response.status_code == 202:
logger.debug("Received 202 Accepted")
return
if response.status_code == 400:
if isinstance(message.root, JSONRPCRequest):
await self._send_bad_request_error(
ctx.read_stream_writer,
message.root.id,
)
return
if response.status_code == 404:
if isinstance(message.root, JSONRPCRequest):
await self._send_session_terminated_error(
ctx.read_stream_writer,
message.root.id,
)
return
response.raise_for_status()
if is_initialization:
self._maybe_extract_session_id_from_response(response)
content_type = response.headers.get(CONTENT_TYPE, "").lower()
if content_type.startswith(JSON):
await self._handle_json_response(response, ctx.read_stream_writer, is_initialization)
elif content_type.startswith(SSE):
await self._handle_sse_response(response, ctx, is_initialization)
else:
await self._handle_unexpected_content_type(
content_type,
ctx.read_stream_writer,
)

The main issue stems from line 285 above, since it raises a HTTPStatusError, which isn't caught. In the current main branch code, only an error 404 response is caught. All other responses would result in a crash because of line 285. I'm not sure if there is a lower level in the code than this before the response received from the server.

Would it be best to put a try...except around line 285? Since an error 404 is already caught in the main branch, wouldn't it be best to catch any type of 4xx or 5xx error?

Thank you for taking the time to review my pull request, I really appreciate it.

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

@ochafik ochafik Awaiting requested review from ochafik ochafik is a code owner automatically assigned from modelcontextprotocol/python-sdk

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CriticalGameEror @Kludex