fix: Add exception handling for POST request failures in StreamableHTTP #1008
Conversation
|
When will this commit merge to the master? |
There was a problem hiding this comment.
Hi @Kulaxyz thank you for this contribution! And apologies for the time it took to get back to you on this.
The problem raised here looks valid, but we should be less broad with Exception handling.
It would also be great if we could add tests to demonstrate the functionality and prevent regressions in future.
If you have the bandwidth to work on this I'd be happy to come back to this quickly!
| content_type, | ||
| ctx.read_stream_writer, | ||
| ) | ||
| except Exception as exc: |
There was a problem hiding this comment.
This is too broad for the specific case you mention - we shouldn't be doing this broad exception handling specifically for 5xx= failures as we might mask other issues this way.
| message.root.id, | ||
| ) | ||
| return | ||
| if response.status_code == 404: |
There was a problem hiding this comment.
We should instead expand this or add a case like
if response.status_code in (500, 502, 503, ...):
That does specific handling of only those specific errors you're encountering and that are expected that we want to handle. That would prevent us from masking other errors that might occur.
Other unexpected HTTP errors should be handled by raise_for_status() lower down.
felixweinberger
added
needs more work
|
Similar PR: #1194 |
felixweinberger
added
needs maintainer action
Added exception handling in the
_handle_post_requestmethod to gracefully handle transport errors and return proper JSON-RPC error responses.Motivation and Context
When an MCP server returns HTTP 5xx errors (e.g., when the server is down) after initial connection, these errors were not being propagated to the caller. This made it impossible for client applications to detect and handle server failures appropriately.
How Has This Been Tested?
Breaking Changes
no
Types of changes
Checklist
Additional context
Strands SDK is an example of an implementation that is hanging when MCP Server is down.