Skip to content

fix(mcp server): propagate detailed error messages to mcp client instead of generic message and migrate to OnyxError#9880

Open
wenxi-onyx wants to merge 1 commit intomainfrom
whuang/fix-onyx-mcp-error-messages
Open

fix(mcp server): propagate detailed error messages to mcp client instead of generic message and migrate to OnyxError#9880
wenxi-onyx wants to merge 1 commit intomainfrom
whuang/fix-onyx-mcp-error-messages

Conversation

@wenxi-onyx
Copy link
Copy Markdown
Member

@wenxi-onyx wenxi-onyx commented Apr 3, 2026

Description

How Has This Been Tested?

Additional Options

  • [Optional] Please cherry-pick this PR to the latest release version.
  • [Optional] Override Linear Check

Summary by cubic

Propagates detailed backend error messages to the MCP client and standardizes web search API errors using OnyxError. This improves user feedback and makes failures easier to debug.

  • Bug Fixes

    • MCP tools (search_indexed_documents, search_web, open_urls) now return a populated error field with backend detail instead of a generic HTTP error.
    • Added helper to parse httpx response JSON and fall back to status text when needed.
  • Refactors

    • Replaced HTTPException with OnyxError + OnyxErrorCode in web search API for consistent error handling.
    • Clearer messages for missing/invalid provider configuration and provider failures (INVALID_INPUT, BAD_GATEWAY).

Written for commit dee39b8. Summary will update on new commits.

…ead of generic message and migrate to OnyxError
@wenxi-onyx wenxi-onyx requested a review from a team as a code owner April 3, 2026 00:12
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR makes two focused improvements to the MCP server's error-handling story: it migrates HTTPException usages in web_search/api.py to the project-standard OnyxError / OnyxErrorCode pattern, and it updates the MCP tool layer in search.py to parse the structured {"error_code": "...", "detail": "..."} body from failed backend responses and surface the human-readable detail string to the MCP client instead of a generic status-code message.

Key changes:

  • backend/onyx/server/features/web_search/api.py: All HTTPException raises replaced with OnyxError; INVALID_INPUT is used for missing/misconfigured providers, BAD_GATEWAY for upstream failures.
  • backend/onyx/mcp_server/tools/search.py: response.raise_for_status() replaced with if not response.is_success: + a new _extract_error_detail helper; all three MCP tools (search_indexed_documents, search_web, open_urls) now return a structured error dict that the LLM can read and relay to the end user.
  • The except OnyxError: raise / except Exception as exc: guard pattern is correctly preserved in both helper functions in api.py.

Confidence Score: 5/5

This PR is safe to merge; all changes are straightforward error-handling improvements with no logic changes to core search functionality.

Both files contain clean, targeted changes: the web_search API correctly adopts OnyxError throughout with semantically appropriate error codes, and the MCP layer now gracefully surfaces detailed error messages to LLM clients. No new endpoints, no schema changes, and existing test surface is unaffected. No P0 or P1 findings were identified.

No files require special attention.

Important Files Changed

Filename Overview
backend/onyx/mcp_server/tools/search.py Adds _extract_error_detail helper and replaces raise_for_status() with structured error-dict returns across all three MCP tool handlers; error messages are now surfaced to MCP clients instead of propagating as raw httpx exceptions.
backend/onyx/server/features/web_search/api.py Removes HTTPException in favour of OnyxError throughout; uses INVALID_INPUT for configuration errors and BAD_GATEWAY for upstream provider failures, preserving existing HTTP status semantics while adopting the project-wide error pattern.

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant MCPTool as search.py (MCP Tool)
    participant WebSearchAPI as web_search/api.py
    participant Provider as Web Search Provider

    Client->>MCPTool: call search_web(query)
    MCPTool->>WebSearchAPI: POST /web-search/search-lite

    alt Provider not configured / bad API key
        WebSearchAPI-->>MCPTool: 4xx {"error_code":"INVALID_INPUT","detail":"...actionable message..."}
        MCPTool->>MCPTool: _extract_error_detail(response)
        MCPTool-->>Client: {"error": "No web search provider configured.", "results": []}
    else Provider throws upstream error
        WebSearchAPI->>Provider: search(query)
        Provider-->>WebSearchAPI: Exception
        WebSearchAPI-->>MCPTool: 502 {"error_code":"BAD_GATEWAY","detail":"Web search provider failed to execute query."}
        MCPTool->>MCPTool: _extract_error_detail(response)
        MCPTool-->>Client: {"error": "Web search provider failed to execute query.", "results": []}
    else Success
        WebSearchAPI->>Provider: search(query)
        Provider-->>WebSearchAPI: results
        WebSearchAPI-->>MCPTool: 200 {"results": [...]}
        MCPTool-->>Client: {"results": [...], "query": "..."}
    end
Loading

Reviews (1): Last reviewed commit: "fix(mcp server): propagate detailed erro..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 2 files

Confidence score: 4/5

  • This PR looks safe to merge overall, with risk concentrated in observability rather than core functionality.
  • The highest-severity issue is in backend/onyx/mcp_server/tools/search.py: non-success backend responses return fallback payloads without logging, which can hide operational failures and slow incident diagnosis.
  • Exception handling in backend/onyx/mcp_server/tools/search.py also swallows backend error-response parsing failures without logs, reducing traceability when error formats change or break.
  • Pay close attention to backend/onyx/mcp_server/tools/search.py - add logging in both fallback and parse-exception paths so backend failures remain debuggable.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/onyx/mcp_server/tools/search.py">

<violation number="1" location="backend/onyx/mcp_server/tools/search.py:30">
P2: Do not silently swallow exceptions while parsing backend error responses; log the parsing failure before falling back.

(Based on your team's feedback about logging exceptions in API error-handling paths.) [FEEDBACK_USED]</violation>

<violation number="2" location="backend/onyx/mcp_server/tools/search.py:178">
P2: Log backend non-success responses before returning fallback payloads so operational failures remain traceable.

(Based on your team's feedback about always logging underlying errors in API/fetch failure paths.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@@ -3,6 +3,8 @@
from datetime import datetime
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Apr 3, 2026

Choose a reason for hiding this comment

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

P2: Log backend non-success responses before returning fallback payloads so operational failures remain traceable.

(Based on your team's feedback about always logging underlying errors in API/fetch failure paths.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/onyx/mcp_server/tools/search.py, line 178:

<comment>Log backend non-success responses before returning fallback payloads so operational failures remain traceable.

(Based on your team's feedback about always logging underlying errors in API/fetch failure paths.) </comment>

<file context>
@@ -158,7 +175,14 @@ async def search_indexed_documents(
             headers=auth_headers,
         )
-        response.raise_for_status()
+        if not response.is_success:
+            error_detail = _extract_error_detail(response)
+            return {
</file context>
Fix with Cubic

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

🖼️ Visual Regression Report

Project Changed Added Removed Unchanged Report
admin 0 0 0 165 ✅ No changes
exclusive 0 0 0 8 ✅ No changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants