mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-22 13:34:17 +02:00
Fix unhandled exceptions in OpenAPI POST tool calls (#3133)
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
This commit is contained in:
Jeremiah Lowin
GitHub
parent
40e80d60e5
commit
a715176499
4 changed files with 217 additions and 11 deletions
|
|
@ -27,7 +27,7 @@ run(self, arguments: dict[str, Any]) -> ToolResult
|
|||
Execute the HTTP request using RequestDirector.
|
||||
|
||||
|
||||
### `OpenAPIResource` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L222" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
### `OpenAPIResource` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L227" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
|
||||
Resource implementation for OpenAPI endpoints.
|
||||
|
|
@ -35,7 +35,7 @@ Resource implementation for OpenAPI endpoints.
|
|||
|
||||
**Methods:**
|
||||
|
||||
#### `read` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L252" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
#### `read` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L257" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
```python
|
||||
read(self) -> ResourceResult
|
||||
|
|
@ -44,7 +44,7 @@ read(self) -> ResourceResult
|
|||
Fetch the resource data by making an HTTP request.
|
||||
|
||||
|
||||
### `OpenAPIResourceTemplate` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L336" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
### `OpenAPIResourceTemplate` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L341" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
|
||||
Resource template implementation for OpenAPI endpoints.
|
||||
|
|
@ -52,7 +52,7 @@ Resource template implementation for OpenAPI endpoints.
|
|||
|
||||
**Methods:**
|
||||
|
||||
#### `create_resource` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L368" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
#### `create_resource` <sup><a href="https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/server/providers/openapi/components.py#L373" target="_blank"><Icon icon="github" style="width: 14px; height: 14px;" /></a></sup>
|
||||
|
||||
```python
|
||||
create_resource(self, uri: str, params: dict[str, Any], context: Context | None = None) -> Resource
|
||||
|
|
|
|||
|
|
@ -159,23 +159,28 @@ class OpenAPITool(Tool):
|
|||
|
||||
async def run(self, arguments: dict[str, Any]) -> ToolResult:
|
||||
"""Execute the HTTP request using RequestDirector."""
|
||||
# Build the request — errors here are programming/schema issues,
|
||||
# not HTTP failures, so we catch them separately.
|
||||
try:
|
||||
base_url = str(self._client.base_url) or "http://localhost"
|
||||
|
||||
# Build the request using RequestDirector
|
||||
request = self._director.build(self._route, arguments, base_url)
|
||||
|
||||
# Add client headers (lowest precedence)
|
||||
if self._client.headers:
|
||||
for key, value in self._client.headers.items():
|
||||
if key not in request.headers:
|
||||
request.headers[key] = value
|
||||
|
||||
# Add MCP transport headers (highest precedence)
|
||||
mcp_headers = get_http_headers()
|
||||
if mcp_headers:
|
||||
request.headers.update(mcp_headers)
|
||||
except Exception as e:
|
||||
raise ValueError(
|
||||
f"Error building request for {self._route.method.upper()} "
|
||||
f"{self._route.path}: {type(e).__name__}: {e}"
|
||||
) from e
|
||||
|
||||
# Send the request and process the response.
|
||||
try:
|
||||
logger.debug(f"run - sending request; headers: {request.headers}")
|
||||
|
||||
response = await self._client.send(request)
|
||||
|
|
|
|||
|
|
@ -166,12 +166,18 @@ class RequestDirector:
|
|||
body = None
|
||||
if body_props:
|
||||
# If we have body properties, construct the body object
|
||||
if route.request_body and route.request_body.content_schema:
|
||||
# Check if the request body expects an object with properties
|
||||
if (
|
||||
route.request_body
|
||||
and route.request_body.content_schema
|
||||
and len(route.request_body.content_schema) > 0
|
||||
):
|
||||
content_type = next(iter(route.request_body.content_schema))
|
||||
body_schema = route.request_body.content_schema[content_type]
|
||||
|
||||
if body_schema.get("type") == "object":
|
||||
if (
|
||||
isinstance(body_schema, dict)
|
||||
and body_schema.get("type") == "object"
|
||||
):
|
||||
body = body_props
|
||||
elif len(body_props) == 1:
|
||||
# If body schema is not an object and we have exactly one property,
|
||||
|
|
|
|||
|
|
@ -763,3 +763,198 @@ class TestOpenAPIComprehensive:
|
|||
error_message = str(exc_info.value)
|
||||
assert "timed out" in error_message
|
||||
assert "ReadTimeout" in error_message
|
||||
|
||||
|
||||
class TestOpenAPIPostEdgeCases:
|
||||
"""Tests for POST request edge cases that could cause unhandled errors."""
|
||||
|
||||
@pytest.fixture
|
||||
def post_spec_with_empty_content_schema(self):
|
||||
"""OpenAPI spec where a POST endpoint has an empty content_schema."""
|
||||
return {
|
||||
"openapi": "3.0.0",
|
||||
"info": {"title": "Test API", "version": "1.0.0"},
|
||||
"servers": [{"url": "https://api.example.com"}],
|
||||
"paths": {
|
||||
"/items": {
|
||||
"post": {
|
||||
"operationId": "create_item",
|
||||
"summary": "Create an item",
|
||||
"requestBody": {
|
||||
"required": True,
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"value": {"type": "integer"},
|
||||
},
|
||||
"required": ["name"],
|
||||
}
|
||||
}
|
||||
},
|
||||
},
|
||||
"responses": {
|
||||
"201": {
|
||||
"description": "Created",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"id": {"type": "integer"},
|
||||
"name": {"type": "string"},
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
"/items/{item_id}": {
|
||||
"post": {
|
||||
"operationId": "update_item",
|
||||
"summary": "Update an item",
|
||||
"parameters": [
|
||||
{
|
||||
"name": "item_id",
|
||||
"in": "path",
|
||||
"required": True,
|
||||
"schema": {"type": "integer"},
|
||||
}
|
||||
],
|
||||
"requestBody": {
|
||||
"required": True,
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"value": {"type": "integer"},
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
},
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "Updated",
|
||||
"content": {
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"id": {"type": "integer"},
|
||||
"name": {"type": "string"},
|
||||
},
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
}
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
async def test_post_with_body_params(self, post_spec_with_empty_content_schema):
|
||||
"""POST with body parameters should build the request correctly."""
|
||||
mock_client = Mock(spec=httpx.AsyncClient)
|
||||
mock_client.base_url = "https://api.example.com"
|
||||
mock_client.headers = None
|
||||
|
||||
mock_response = Mock(spec=Response)
|
||||
mock_response.status_code = 201
|
||||
mock_response.json.return_value = {"id": 1, "name": "Test"}
|
||||
mock_response.raise_for_status = Mock()
|
||||
mock_client.send = AsyncMock(return_value=mock_response)
|
||||
|
||||
server = create_openapi_server(
|
||||
openapi_spec=post_spec_with_empty_content_schema,
|
||||
client=mock_client,
|
||||
)
|
||||
|
||||
async with Client(server) as mcp_client:
|
||||
result = await mcp_client.call_tool(
|
||||
"create_item", {"name": "Test", "value": 42}
|
||||
)
|
||||
|
||||
mock_client.send.assert_called_once()
|
||||
request = mock_client.send.call_args[0][0]
|
||||
assert request.method == "POST"
|
||||
body_data = json.loads(request.content)
|
||||
assert body_data["name"] == "Test"
|
||||
assert body_data["value"] == 42
|
||||
assert result is not None
|
||||
|
||||
async def test_post_with_path_params_and_body(
|
||||
self, post_spec_with_empty_content_schema
|
||||
):
|
||||
"""POST with both path parameters and body should route args correctly."""
|
||||
mock_client = Mock(spec=httpx.AsyncClient)
|
||||
mock_client.base_url = "https://api.example.com"
|
||||
mock_client.headers = None
|
||||
|
||||
mock_response = Mock(spec=Response)
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = {"id": 5, "name": "Updated"}
|
||||
mock_response.raise_for_status = Mock()
|
||||
mock_client.send = AsyncMock(return_value=mock_response)
|
||||
|
||||
server = create_openapi_server(
|
||||
openapi_spec=post_spec_with_empty_content_schema,
|
||||
client=mock_client,
|
||||
)
|
||||
|
||||
async with Client(server) as mcp_client:
|
||||
result = await mcp_client.call_tool(
|
||||
"update_item",
|
||||
{"item_id": 5, "name": "Updated", "value": 99},
|
||||
)
|
||||
|
||||
mock_client.send.assert_called_once()
|
||||
request = mock_client.send.call_args[0][0]
|
||||
assert request.method == "POST"
|
||||
assert "/items/5" in str(request.url)
|
||||
body_data = json.loads(request.content)
|
||||
assert body_data["name"] == "Updated"
|
||||
assert body_data["value"] == 99
|
||||
assert "item_id" not in body_data
|
||||
assert result is not None
|
||||
|
||||
async def test_unexpected_error_in_request_building_gives_useful_message(self):
|
||||
"""Unexpected exceptions during request building should produce useful errors."""
|
||||
from fastmcp.server.providers.openapi.components import OpenAPITool
|
||||
from fastmcp.utilities.openapi.director import RequestDirector
|
||||
from fastmcp.utilities.openapi.models import HTTPRoute
|
||||
|
||||
mock_client = Mock(spec=httpx.AsyncClient)
|
||||
mock_client.base_url = "https://api.example.com"
|
||||
mock_client.headers = None
|
||||
|
||||
route = HTTPRoute(
|
||||
path="/test",
|
||||
method="POST",
|
||||
operation_id="test_op",
|
||||
parameters=[],
|
||||
responses={},
|
||||
response_schemas={},
|
||||
)
|
||||
|
||||
mock_director = Mock(spec=RequestDirector)
|
||||
mock_director.build.side_effect = KeyError("missing_param")
|
||||
|
||||
tool = OpenAPITool(
|
||||
client=mock_client,
|
||||
route=route,
|
||||
director=mock_director,
|
||||
name="test_tool",
|
||||
description="test",
|
||||
parameters={},
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match="Error building request for POST /test"):
|
||||
await tool.run({"some_arg": "value"})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue