fix: bound hosted binary tool responses - #112
Conversation
Greptile SummaryThe PR bounds hosted binary Appwrite responses at 25 MiB while preserving the generated SDK path for local stdio deployments. The compressed-error fix prevents decompression before bounding, but currently bypasses Appwrite error translation for compressed HTTP failures.
Confidence Score: 4/5The PR should not merge until compressed upstream HTTP errors retain the existing Appwrite error contract. The reordered encoding guard protects memory but converts compressed 4xx and 5xx responses into generic ValueErrors before their status, type, body, and telemetry classification can be preserved. Files Needing Attention: src/mcp_server_appwrite/server.py Important Files Changed
Prompt To Fix All With AI### Issue 1
src/mcp_server_appwrite/server.py:887-891
**Compressed errors lose Appwrite context**
If an upstream server or intermediary returns a compressed HTTP 4xx or 5xx response despite the identity request, this guard raises `ValueError` before `_raise_bounded_response_error` can translate the status. The caller consequently loses the upstream status, type, and body, while telemetry loses the `appwrite_4xx` or `appwrite_5xx` classification and associated 5xx monitoring.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (2): Last reviewed commit: "fix: guard compressed upstream error bod..." | Re-trigger Greptile |
| if content_encoding.lower().strip() not in {"", "identity"}: | ||
| raise ValueError( | ||
| "Hosted MCP cannot safely return a compressed binary response. " | ||
| "Use an Appwrite SDK or REST API for this content." | ||
| ) |
There was a problem hiding this comment.
Compressed errors lose Appwrite context
If an upstream server or intermediary returns a compressed HTTP 4xx or 5xx response despite the identity request, this guard raises ValueError before _raise_bounded_response_error can translate the status. The caller consequently loses the upstream status, type, and body, while telemetry loses the appwrite_4xx or appwrite_5xx classification and associated 5xx monitoring.
Knowledge Base Used: Observability and error handling
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/server.py
Line: 887-891
Comment:
**Compressed errors lose Appwrite context**
If an upstream server or intermediary returns a compressed HTTP 4xx or 5xx response despite the identity request, this guard raises `ValueError` before `_raise_bounded_response_error` can translate the status. The caller consequently loses the upstream status, type, and body, while telemetry loses the `appwrite_4xx` or `appwrite_5xx` classification and associated 5xx monitoring.
**Knowledge Base Used:** [Observability and error handling](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/appwrite/mcp/-/docs/observability-and-error-handling.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary
Content-Lengthor observed streamed bytes with a stablehosted_response_too_largeMCP tool errorContext
Production pods were OOM-killed while
appwrite_call_toolwas inflight. Binary SDK methods currently read the full body into memory, after which MCP base64 and JSON serialization create additional copies. A single large response can therefore exceed the 1 GiB pod limit.This keeps binary methods available and backward-compatible for responses up to 25 MiB. Larger responses fail predictably and direct clients to an Appwrite SDK or REST API.
Validation
uv run --group dev ruff check src testsuv run --group dev black --check src testsuv run --group dev pyrightuv run python -m unittest discover -s tests/unit -v(252 tests)