FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: propagate HTTP errors from transports instead of silently logging by MaxwellCalkin · Pull Request #2249 · modelcontextprotocol/python-sdk · GitHub

/ python-sdk Public

fix: propagate HTTP errors from transports instead of silently logging #2249

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

Closed
MaxwellCalkin wants to merge 4 commits into from
Conversation 5 Commits 4 Checks 28 Files changed
Closed

fix: propagate HTTP errors from transports instead of silently logging #2249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
642ca88
fix: add HttpError exception class for HTTP error propagation
MaxwellCalkin Mar 8, 2026
7605dfd
fix: propagate HTTP errors in StreamableHTTP transport instead of sil…
MaxwellCalkin Mar 8, 2026
7d60257
fix: propagate HTTP errors in SSE transport instead of silently logging
MaxwellCalkin Mar 8, 2026
715d47d
fix: export HttpError from mcp package
MaxwellCalkin Mar 8, 2026
File filter

Filter by extension

Filter by extension .py  (4) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
3 changes: 2 additions & 1 deletion src/mcp/__init__.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from .client.stdio import StdioServerParameters, stdio_client
from .server.session import ServerSession
from .server.stdio import stdio_server
from .shared.exceptions import MCPError, UrlElicitationRequiredError
from .shared.exceptions import HttpError, MCPError, UrlElicitationRequiredError
from .types import (
CallToolRequest,
ClientCapabilities,
Expand Down Expand Up @@ -81,6 +81,7 @@
"ErrorData",
"GetPromptRequest",
"GetPromptResult",
"HttpError",
"Implementation",
"IncludeContext",
"InitializeRequest",
Expand Down
21 changes: 20 additions & 1 deletion src/mcp/client/sse.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from mcp import types
from mcp.shared._httpx_utils import McpHttpClientFactory, create_mcp_http_client
from mcp.shared.exceptions import HttpError
from mcp.shared.message import SessionMessage

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -142,10 +143,28 @@ async def post_writer(endpoint_url: str):
exclude_unset=True,
),
)
if response.status_code in (401, 403):
status_label = "Unauthorized" if response.status_code == 401 else "Forbidden"
exc = HttpError(
response.status_code,
f"HTTP {response.status_code} {status_label}",

Bortlesboat Mar 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Re-raising HttpError here means it escapes post_writer entirely — nothing above catches it, so it propagates into the task group and crashes the whole SSE client connection. The original code intentionally swallowed all exceptions to keep the writer alive for the connection lifetime.

For the SSE transport I think you want the same pattern as the other exceptions: send it through the read stream but don't re-raise. The caller can handle it from the stream without killing the connection.

slykar Mar 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is exactly the issue I'm having. I've fixed it in this PR: #2273
)
await read_stream_writer.send(exc)
raise exc
response.raise_for_status()
logger.debug(f"Client message sent successfully: {response.status_code}")
except Exception: # pragma: lax no cover
except HttpError: # pragma: lax no cover
raise
except httpx.HTTPStatusError as exc: # pragma: lax no cover
logger.exception("Error in post_writer")
http_exc = HttpError(
exc.response.status_code,
f"HTTP {exc.response.status_code}",
)
await read_stream_writer.send(http_exc)
except Exception as exc: # pragma: lax no cover
logger.exception("Error in post_writer")
await read_stream_writer.send(exc)
finally:
await write_stream.aclose()

Expand Down
46 changes: 38 additions & 8 deletions src/mcp/client/streamable_http.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from mcp.client._transport import TransportStreams
from mcp.shared._httpx_utils import create_mcp_http_client
from mcp.shared.exceptions import HttpError
from mcp.shared.message import ClientMessageMetadata, SessionMessage
from mcp.types import (
INTERNAL_ERROR,
Expand Down Expand Up @@ -269,17 +270,41 @@ async def _handle_post_request(self, ctx: RequestContext) -> None:

if response.status_code == 404: # pragma: no branch
if isinstance(message, JSONRPCRequest): # pragma: no branch
error_data = ErrorData(code=INVALID_REQUEST, message="Session terminated")
error_data = ErrorData(
code=INVALID_REQUEST,
message="Session terminated (HTTP 404)",
data={"http_status": 404},
)
session_message = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data))
await ctx.read_stream_writer.send(session_message)
else:
raise HttpError(404, "Session terminated (HTTP 404)")
return

if response.status_code in (401, 403):
status_label = "Unauthorized" if response.status_code == 401 else "Forbidden"
error_message = f"HTTP {response.status_code} {status_label}"
if isinstance(message, JSONRPCRequest):

Bortlesboat Mar 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This raises after already sending a JSONRPCError to the read stream (line 26). But handle_request_async wraps _handle_post_request in a try/except that catches this HttpError and sends it to read_stream_writer again. So for JSONRPCRequest messages hitting 401/403 (and same for the generic >=400 path below), the consumer receives both a SessionMessage(JSONRPCError) and then an HttpError exception from the same stream — two messages for one request.

I think you either want to not raise here when the JSONRPCError was already sent, or skip the JSONRPCError and just raise (letting the wrapper forward it). The current approach of doing both will confuse callers.
error_data = ErrorData(
code=INTERNAL_ERROR,
message=error_message,
data={"http_status": response.status_code},
)
session_message = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data))
await ctx.read_stream_writer.send(session_message)
raise HttpError(response.status_code, error_message)

if response.status_code >= 400:
error_message = f"HTTP {response.status_code}"
if isinstance(message, JSONRPCRequest):
error_data = ErrorData(code=INTERNAL_ERROR, message="Server returned an error response")
error_data = ErrorData(
code=INTERNAL_ERROR,
message=error_message,
data={"http_status": response.status_code},
)
session_message = SessionMessage(JSONRPCError(jsonrpc="2.0", id=message.id, error=error_data))
await ctx.read_stream_writer.send(session_message)
return
raise HttpError(response.status_code, error_message)

if is_initialization:
self._maybe_extract_session_id_from_response(response)
Expand Down Expand Up @@ -467,19 +492,24 @@ async def post_writer(
)

async def handle_request_async():
if is_resumption:
await self._handle_resumption_request(ctx)
else:
await self._handle_post_request(ctx)
try:
if is_resumption:
await self._handle_resumption_request(ctx)
else:
await self._handle_post_request(ctx)
except Exception as exc:
logger.exception("Error handling request")
await read_stream_writer.send(exc)

# If this is a request, start a new task to handle it
if isinstance(message, JSONRPCRequest):
tg.start_soon(handle_request_async)
else:
await handle_request_async()

except Exception: # pragma: lax no cover
except Exception as exc: # pragma: lax no cover
logger.exception("Error in post_writer")
await read_stream_writer.send(exc)
finally:
await read_stream_writer.aclose()
await write_stream.aclose()
Expand Down
20 changes: 20 additions & 0 deletions src/mcp/shared/exceptions.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ def __str__(self) -> str:
return self.message


class HttpError(Exception):
"""Raised when an MCP HTTP transport receives a non-2xx response.

Preserves the original HTTP status code so callers can distinguish
auth errors (401/403) from other failures (404, 5xx, etc.).
"""

def __init__(self, status_code: int, message: str | None = None, body: str | None = None):
self.status_code = status_code
self.body = body
if message is None:
message = f"HTTP {status_code}"
super().__init__(message)

@property
def is_auth_error(self) -> bool:
"""True for 401 Unauthorized or 403 Forbidden responses."""
return self.status_code in (401, 403)


class StatelessModeNotSupported(RuntimeError):
"""Raised when attempting to use a method that is not supported in stateless mode.

Expand Down
Loading
Back | FazBrowse Home | New Git URL