From 662ec082cf22076dc34937b750383445e4265750 Mon Sep 17 00:00:00 2001 From: Kian Jones <11655409+kianjones9@users.noreply.github.com> Date: Thu, 5 Feb 2026 18:45:51 -0800 Subject: [PATCH] fix(core): handle MCP errors and API key whitespace (#9306) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: strip whitespace from API keys in LLM client headers Fixes httpx.LocalProtocolError when API keys contain leading/trailing whitespace. Strips whitespace from API keys before using them in HTTP headers across: - OpenAI client (openai.py) - Mistral client (mistral.py) - Anthropic client (anthropic_client.py) - Anthropic schema provider (schemas/providers/anthropic.py) - Google AI client (google_ai_client.py) - Proxy helpers (proxy_helpers.py) 🐾 Generated with [Letta Code](https://letta.com) Co-Authored-By: Letta * fix: handle McpError gracefully in MCP client execute_tool Return error as failed result instead of re-raising to avoid Datadog alerts for expected user-facing errors like missing tool arguments. * fix: strip whitespace from API keys before passing to httpx client Fixes httpx.LocalProtocolError by stripping leading/trailing whitespace from API keys before passing them to OpenAI/AsyncOpenAI clients. The OpenAI client library constructs Authorization headers internally, and invalid header values (like keys with leading spaces) cause protocol errors. Applied fix to: - azure_client.py (AzureOpenAI/AsyncAzureOpenAI) - deepseek_client.py (OpenAI/AsyncOpenAI) - openai_client.py (OpenAI/AsyncOpenAI via kwargs) - xai_client.py (OpenAI/AsyncOpenAI) 🐾 Generated with [Letta Code](https://letta.com) Co-Authored-By: Letta * fix: handle JSONDecodeError in OpenAI client requests Catches json.JSONDecodeError from OpenAI SDK when API returns invalid JSON (typically HTML error pages from 500-series errors) and converts to LLMServerError with helpful details. 🐾 Generated with [Letta Code](https://letta.com) Co-Authored-By: Letta * fix(core): strip API key whitespace at schema level on write/create Add field_validator to ProviderCreate, ProviderUpdate, and ProviderCheck schemas to strip whitespace from api_key and access_key fields before persistence. This ensures keys are clean at the point of entry, preventing whitespace from being encrypted and stored in the database. Co-authored-by: Kian Jones * refactor: remove api_key.strip() calls across all LLM clients Remove redundant .strip() calls on api_key parameters since pydantic models now handle whitespace trimming at the validation layer. This centralizes the validation logic and follows DRY principles. - Updated 13 files across multiple LLM client implementations - Removed 34 occurrences of api_key.strip() - Includes: OpenAI, Anthropic, Azure, Google AI, Groq, XAI, DeepSeek, ZAI, Together, Mistral - Also updated proxy helpers and provider schemas 👾 Generated with [Letta Code](https://letta.com) Co-Authored-By: Letta * refactor: remove redundant ternary operators from api_key parameters Remove `if api_key else None` ternaries since pydantic validation ensures api_key is either a valid string or None. The ternary was defensive programming that's now unnecessary with proper model-level validation. - Simplified 23 occurrences across 7 files - Cleaner, more concise client initialization code - No behavioral change since pydantic already handles this 👾 Generated with [Letta Code](https://letta.com) Co-Authored-By: Letta --------- Co-authored-by: Letta Co-authored-by: letta-code <248085862+letta-code@users.noreply.github.com> Co-authored-by: Kian Jones --- letta/llm_api/openai_client.py | 40 +++++++++++++++++++++---------- letta/schemas/providers/base.py | 15 ++++++++++++ letta/services/mcp/base_client.py | 3 +++ 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/letta/llm_api/openai_client.py b/letta/llm_api/openai_client.py index 991e8d84..48201362 100644 --- a/letta/llm_api/openai_client.py +++ b/letta/llm_api/openai_client.py @@ -620,12 +620,20 @@ class OpenAIClient(LLMClientBase): client = OpenAI(**self._prepare_client_kwargs(llm_config)) # Route based on payload shape: Responses uses 'input', Chat Completions uses 'messages' - if "input" in request_data and "messages" not in request_data: - resp = client.responses.create(**request_data) - return resp.model_dump() - else: - response: ChatCompletion = client.chat.completions.create(**request_data) - return response.model_dump() + try: + if "input" in request_data and "messages" not in request_data: + resp = client.responses.create(**request_data) + return resp.model_dump() + else: + response: ChatCompletion = client.chat.completions.create(**request_data) + return response.model_dump() + except json.JSONDecodeError as e: + logger.error(f"[OpenAI] Failed to parse API response as JSON: {e}") + raise LLMServerError( + message=f"OpenAI API returned invalid JSON response (likely an HTML error page): {str(e)}", + code=ErrorCode.INTERNAL_SERVER_ERROR, + details={"json_error": str(e), "error_position": f"line {e.lineno} column {e.colno}"}, + ) @trace_method async def request_async(self, request_data: dict, llm_config: LLMConfig) -> dict: @@ -638,12 +646,20 @@ class OpenAIClient(LLMClientBase): kwargs = await self._prepare_client_kwargs_async(llm_config) client = AsyncOpenAI(**kwargs) # Route based on payload shape: Responses uses 'input', Chat Completions uses 'messages' - if "input" in request_data and "messages" not in request_data: - resp = await client.responses.create(**request_data) - return resp.model_dump() - else: - response: ChatCompletion = await client.chat.completions.create(**request_data) - return response.model_dump() + try: + if "input" in request_data and "messages" not in request_data: + resp = await client.responses.create(**request_data) + return resp.model_dump() + else: + response: ChatCompletion = await client.chat.completions.create(**request_data) + return response.model_dump() + except json.JSONDecodeError as e: + logger.error(f"[OpenAI] Failed to parse API response as JSON: {e}") + raise LLMServerError( + message=f"OpenAI API returned invalid JSON response (likely an HTML error page): {str(e)}", + code=ErrorCode.INTERNAL_SERVER_ERROR, + details={"json_error": str(e), "error_position": f"line {e.lineno} column {e.colno}"}, + ) def is_reasoning_model(self, llm_config: LLMConfig) -> bool: return is_openai_reasoning_model(llm_config.model) diff --git a/letta/schemas/providers/base.py b/letta/schemas/providers/base.py index 73e4a239..bad77164 100644 --- a/letta/schemas/providers/base.py +++ b/letta/schemas/providers/base.py @@ -263,6 +263,11 @@ class ProviderCreate(ProviderBase): base_url: str | None = Field(None, description="Base URL used for requests to the provider.") api_version: str | None = Field(None, description="API version used for requests to the provider.") + @field_validator("api_key", "access_key", mode="before") + @classmethod + def strip_whitespace(cls, v: str | None) -> str | None: + return v.strip() if isinstance(v, str) else v + class ProviderUpdate(ProviderBase): api_key: str = Field(..., description="API key or secret key used for requests to the provider.") @@ -271,6 +276,11 @@ class ProviderUpdate(ProviderBase): base_url: str | None = Field(None, description="Base URL used for requests to the provider.") api_version: str | None = Field(None, description="API version used for requests to the provider.") + @field_validator("api_key", "access_key", mode="before") + @classmethod + def strip_whitespace(cls, v: str | None) -> str | None: + return v.strip() if isinstance(v, str) else v + class ProviderCheck(BaseModel): provider_type: ProviderType = Field(..., description="The type of the provider.") @@ -279,3 +289,8 @@ class ProviderCheck(BaseModel): region: str | None = Field(None, description="Region used for requests to the provider.") base_url: str | None = Field(None, description="Base URL used for requests to the provider.") api_version: str | None = Field(None, description="API version used for requests to the provider.") + + @field_validator("api_key", "access_key", mode="before") + @classmethod + def strip_whitespace(cls, v: str | None) -> str | None: + return v.strip() if isinstance(v, str) else v diff --git a/letta/services/mcp/base_client.py b/letta/services/mcp/base_client.py index d297e83f..6b4a0536 100644 --- a/letta/services/mcp/base_client.py +++ b/letta/services/mcp/base_client.py @@ -87,6 +87,9 @@ class AsyncBaseMCPClient: # Log at debug level to avoid triggering production alerts for expected failures if e.__class__.__name__ in ("McpError", "ToolError"): logger.debug(f"MCP tool '{tool_name}' execution failed: {str(e)}") + # Return error message with failure status instead of raising to avoid Datadog alerts + return str(e), False + # Re-raise unexpected errors raise parsed_content = []