mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-22 13:34:17 +02:00
Preserve transitional httpx error handling
This commit is contained in:
Jeremiah Lowin
parent
f11db192a3
commit
1cbd1652f9
5 changed files with 143 additions and 28 deletions
|
|
@ -18,6 +18,7 @@ from fastmcp.resources import (
|
|||
)
|
||||
from fastmcp.server.dependencies import get_http_headers
|
||||
from fastmcp.tools.base import Tool, ToolResult
|
||||
from fastmcp.utilities.exceptions import is_request_error, is_timeout_error
|
||||
from fastmcp.utilities.logging import get_logger
|
||||
from fastmcp.utilities.openapi import HTTPRoute
|
||||
from fastmcp.utilities.openapi.director import RequestDirector
|
||||
|
|
@ -73,6 +74,21 @@ def _raise_for_status(response: httpx2.Response) -> None:
|
|||
raise ValueError(error_message)
|
||||
|
||||
|
||||
async def _send_request(
|
||||
client: httpx2.AsyncClient,
|
||||
request: httpx2.Request,
|
||||
) -> httpx2.Response:
|
||||
"""Send a request while preserving transitional legacy-client errors."""
|
||||
try:
|
||||
return await client.send(request)
|
||||
except Exception as exc:
|
||||
if is_timeout_error(exc):
|
||||
raise ValueError(f"HTTP request timed out ({type(exc).__name__})") from exc
|
||||
if is_request_error(exc):
|
||||
raise ValueError(f"Request error ({type(exc).__name__}): {exc!s}") from exc
|
||||
raise
|
||||
|
||||
|
||||
def _extract_mime_type_from_route(route: HTTPRoute) -> str:
|
||||
"""Extract the primary MIME type from an HTTPRoute's response definitions.
|
||||
|
||||
|
|
@ -216,7 +232,7 @@ class OpenAPITool(Tool):
|
|||
f"run - sending request; headers: {_redact_headers(request.headers)}"
|
||||
)
|
||||
|
||||
response = await self._client.send(request)
|
||||
response = await _send_request(self._client, request)
|
||||
_raise_for_status(response)
|
||||
|
||||
# Try to parse as JSON first
|
||||
|
|
@ -244,11 +260,11 @@ class OpenAPITool(Tool):
|
|||
except json.JSONDecodeError:
|
||||
return ToolResult(content=response.text)
|
||||
|
||||
except httpx2.TimeoutException as e:
|
||||
raise ValueError(f"HTTP request timed out ({type(e).__name__})") from e
|
||||
except httpx2.TimeoutException as exc:
|
||||
raise ValueError(f"HTTP request timed out ({type(exc).__name__})") from exc
|
||||
|
||||
except httpx2.RequestError as e:
|
||||
raise ValueError(f"Request error ({type(e).__name__}): {e!s}") from e
|
||||
except httpx2.RequestError as exc:
|
||||
raise ValueError(f"Request error ({type(exc).__name__}): {exc!s}") from exc
|
||||
|
||||
|
||||
class OpenAPIResource(Resource):
|
||||
|
|
@ -305,7 +321,7 @@ class OpenAPIResource(Resource):
|
|||
if mcp_headers:
|
||||
request.headers.update(mcp_headers)
|
||||
|
||||
response = await self._client.send(request)
|
||||
response = await _send_request(self._client, request)
|
||||
_raise_for_status(response)
|
||||
|
||||
content_type = response.headers.get("content-type", "").lower()
|
||||
|
|
@ -334,11 +350,11 @@ class OpenAPIResource(Resource):
|
|||
]
|
||||
)
|
||||
|
||||
except httpx2.TimeoutException as e:
|
||||
raise ValueError(f"HTTP request timed out ({type(e).__name__})") from e
|
||||
except httpx2.TimeoutException as exc:
|
||||
raise ValueError(f"HTTP request timed out ({type(exc).__name__})") from exc
|
||||
|
||||
except httpx2.RequestError as e:
|
||||
raise ValueError(f"Request error ({type(e).__name__}): {e!s}") from e
|
||||
except httpx2.RequestError as exc:
|
||||
raise ValueError(f"Request error ({type(exc).__name__}): {exc!s}") from exc
|
||||
|
||||
|
||||
def _path_argument_name(route: HTTPRoute, parameter_name: str) -> str:
|
||||
|
|
|
|||
|
|
@ -88,6 +88,7 @@ from fastmcp.tools.base import Tool, ToolResult
|
|||
from fastmcp.tools.function_tool import FunctionTool
|
||||
from fastmcp.tools.tool_transform import ToolTransformConfig
|
||||
from fastmcp.utilities.components import FastMCPComponent, _coerce_version
|
||||
from fastmcp.utilities.exceptions import get_http_status_code, is_timeout_error
|
||||
from fastmcp.utilities.logging import get_logger
|
||||
from fastmcp.utilities.tasks import TaskConfig
|
||||
from fastmcp.utilities.types import AnyFunction, FastMCPBaseModel, NotSet, NotSetT
|
||||
|
|
@ -1540,12 +1541,11 @@ class FastMCP(
|
|||
logger.exception(f"Error calling tool {name!r}")
|
||||
# Handle actionable errors that should reach the LLM
|
||||
# even when masking is enabled
|
||||
if isinstance(e, httpx2.HTTPStatusError):
|
||||
if e.response.status_code == 429:
|
||||
raise ToolError(
|
||||
"Rate limited by upstream API, please retry later"
|
||||
) from e
|
||||
if isinstance(e, httpx2.TimeoutException):
|
||||
if get_http_status_code(e) == 429:
|
||||
raise ToolError(
|
||||
"Rate limited by upstream API, please retry later"
|
||||
) from e
|
||||
if is_timeout_error(e):
|
||||
raise ToolError(
|
||||
"Upstream request timed out, please retry"
|
||||
) from e
|
||||
|
|
@ -1640,12 +1640,11 @@ class FastMCP(
|
|||
except Exception as e:
|
||||
logger.exception(f"Error reading resource {uri!r}")
|
||||
# Handle actionable errors that should reach the LLM
|
||||
if isinstance(e, httpx2.HTTPStatusError):
|
||||
if e.response.status_code == 429:
|
||||
raise ResourceError(
|
||||
"Rate limited by upstream API, please retry later"
|
||||
) from e
|
||||
if isinstance(e, httpx2.TimeoutException):
|
||||
if get_http_status_code(e) == 429:
|
||||
raise ResourceError(
|
||||
"Rate limited by upstream API, please retry later"
|
||||
) from e
|
||||
if is_timeout_error(e):
|
||||
raise ResourceError(
|
||||
"Upstream request timed out, please retry"
|
||||
) from e
|
||||
|
|
@ -1700,12 +1699,11 @@ class FastMCP(
|
|||
except Exception as e:
|
||||
logger.exception(f"Error reading resource {uri!r}")
|
||||
# Handle actionable errors that should reach the LLM
|
||||
if isinstance(e, httpx2.HTTPStatusError):
|
||||
if e.response.status_code == 429:
|
||||
raise ResourceError(
|
||||
"Rate limited by upstream API, please retry later"
|
||||
) from e
|
||||
if isinstance(e, httpx2.TimeoutException):
|
||||
if get_http_status_code(e) == 429:
|
||||
raise ResourceError(
|
||||
"Rate limited by upstream API, please retry later"
|
||||
) from e
|
||||
if is_timeout_error(e):
|
||||
raise ResourceError(
|
||||
"Upstream request timed out, please retry"
|
||||
) from e
|
||||
|
|
|
|||
|
|
@ -8,6 +8,43 @@ from mcp import MCPError
|
|||
import fastmcp
|
||||
|
||||
|
||||
def _is_legacy_httpx_exception(exc: BaseException, exception_type: str) -> bool:
|
||||
"""Check a legacy-httpx exception without importing the legacy package."""
|
||||
return any(
|
||||
cls.__module__.partition(".")[0] == "httpx" and cls.__name__ == exception_type
|
||||
for cls in type(exc).__mro__
|
||||
)
|
||||
|
||||
|
||||
def is_http_status_error(exc: BaseException) -> bool:
|
||||
"""Return whether an exception is an httpx2 or legacy-httpx status error."""
|
||||
return isinstance(exc, httpx2.HTTPStatusError) or _is_legacy_httpx_exception(
|
||||
exc, "HTTPStatusError"
|
||||
)
|
||||
|
||||
|
||||
def get_http_status_code(exc: BaseException) -> int | None:
|
||||
"""Return the response status code from a recognized HTTP status error."""
|
||||
if not is_http_status_error(exc):
|
||||
return None
|
||||
status_code = getattr(getattr(exc, "response", None), "status_code", None)
|
||||
return status_code if isinstance(status_code, int) else None
|
||||
|
||||
|
||||
def is_timeout_error(exc: BaseException) -> bool:
|
||||
"""Return whether an exception is an httpx2 or legacy-httpx timeout."""
|
||||
return isinstance(exc, httpx2.TimeoutException) or _is_legacy_httpx_exception(
|
||||
exc, "TimeoutException"
|
||||
)
|
||||
|
||||
|
||||
def is_request_error(exc: BaseException) -> bool:
|
||||
"""Return whether an exception is an httpx2 or legacy-httpx request error."""
|
||||
return isinstance(exc, httpx2.RequestError) or _is_legacy_httpx_exception(
|
||||
exc, "RequestError"
|
||||
)
|
||||
|
||||
|
||||
def iter_exc(group: BaseExceptionGroup):
|
||||
for exc in group.exceptions:
|
||||
if isinstance(exc, BaseExceptionGroup):
|
||||
|
|
|
|||
|
|
@ -77,3 +77,34 @@ async def test_legacy_client_preserves_http_error_details() -> None:
|
|||
await mcp_client.call_tool("list_items", {})
|
||||
|
||||
assert "items not found" in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("error_kind", "message"),
|
||||
[
|
||||
("timeout", "HTTP request timed out (ReadTimeout)"),
|
||||
("connect", "Request error (ConnectError)"),
|
||||
],
|
||||
)
|
||||
async def test_legacy_client_preserves_transport_error_details(
|
||||
error_kind: str,
|
||||
message: str,
|
||||
) -> None:
|
||||
def handler(request: "httpx.Request") -> "httpx.Response":
|
||||
if error_kind == "timeout":
|
||||
raise httpx.ReadTimeout("transport failed", request=request)
|
||||
raise httpx.ConnectError("transport failed", request=request)
|
||||
|
||||
transport = httpx.MockTransport(handler)
|
||||
async with httpx.AsyncClient(
|
||||
transport=transport,
|
||||
base_url="https://api.example.com",
|
||||
) as client:
|
||||
with pytest.warns(FastMCPDeprecationWarning):
|
||||
server = FastMCP.from_openapi(SPEC, client=client)
|
||||
|
||||
async with Client(server) as mcp_client:
|
||||
with pytest.raises(ToolError) as exc_info:
|
||||
await mcp_client.call_tool("list_items", {})
|
||||
|
||||
assert message in str(exc_info.value)
|
||||
|
|
|
|||
33
tests/server/test_legacy_httpx_errors.py
Normal file
33
tests/server/test_legacy_httpx_errors.py
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
"""Compatibility tests for legacy-httpx exceptions raised by user code."""
|
||||
|
||||
import pytest
|
||||
|
||||
from fastmcp import FastMCP
|
||||
from fastmcp.exceptions import ResourceError, ToolError
|
||||
|
||||
httpx = pytest.importorskip("httpx", reason="legacy httpx not installed")
|
||||
|
||||
|
||||
async def test_legacy_httpx_rate_limit_remains_actionable() -> None:
|
||||
server = FastMCP("Legacy httpx errors", mask_error_details=True)
|
||||
|
||||
@server.tool
|
||||
def rate_limited() -> None:
|
||||
request = httpx.Request("GET", "https://example.com")
|
||||
response = httpx.Response(429, request=request)
|
||||
raise httpx.HTTPStatusError("rate limited", request=request, response=response)
|
||||
|
||||
with pytest.raises(ToolError, match="Rate limited by upstream API"):
|
||||
await server.call_tool("rate_limited", {})
|
||||
|
||||
|
||||
async def test_legacy_httpx_resource_timeout_remains_actionable() -> None:
|
||||
server = FastMCP("Legacy httpx errors", mask_error_details=True)
|
||||
|
||||
@server.resource("resource://timed-out")
|
||||
def timed_out() -> str:
|
||||
request = httpx.Request("GET", "https://example.com")
|
||||
raise httpx.ReadTimeout("timed out", request=request)
|
||||
|
||||
with pytest.raises(ResourceError, match="Upstream request timed out"):
|
||||
await server.read_resource("resource://timed-out")
|
||||
Loading…
Add table
Add a link
Reference in a new issue