fix(mcp): don't trip circuit breaker on business errors (isError) - #47898
fix(mcp): don't trip circuit breaker on business errors (isError)#47898blut-agent wants to merge 1 commit into
Conversation
When an MCP tool returns isError=true with a business error message
(e.g. validation failure), the handler wraps it as {"error": "..."}.
The circuit-breaker check at line ~2867 was treating this the same as
a transport failure and bumping the error counter, which caused the
circuit breaker to trip after 3 consecutive business errors (NousResearch#47851).
Business errors mean the server is healthy — the model should retry
with corrected arguments. Only transport/auth failures (caught as
exceptions in the outer except block) should increment the counter.
Fix: reset the error counter for JSON responses containing an "error"
key instead of bumping it. Transport failures still bump via the outer
except Exception block.
Refs: NousResearch#47851
|
Duplicate of #40951 — both patch the identical |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Small, well-scoped fix. The change correctly distinguishes between MCP protocol business errors (isError flag in JSON response) and transport failures - only the latter should trip the circuit breaker. The comments clearly explain the rationale and reference the related issue.
Looks Good
- Single-file change in tools/mcp_tool.py
- Clear comments explaining the fix rationale
- No security or performance concerns
- No test changes needed (this is a runtime behavior fix for a rare edge case)
Reviewed by Hermes Agent
Problem
When an MCP tool returns
isError: truewith a business error message (e.g. validation failure, missing required field), the handler wraps it as{"error": "..."}. The circuit-breaker check at line ~2867 was treating this the same as a transport failure and calling_bump_server_error(), which caused the circuit breaker to trip after 3 consecutive business errors.This blocks ALL tools on the server for ~60 seconds with a misleading message like:
"MCP server 'waw' is unreachable after 3 consecutive failures"even though the server is perfectly healthy — the errors are just validation/business errors.
Refs: #47851
Fix
Reset the error counter for JSON responses containing an
"error"key instead of bumping it. Business errors mean the server is healthy — the model should retry with corrected arguments.Transport/auth failures are still properly handled by the outer
except Exceptionblock (line ~2876), which still calls_bump_server_error().Changes
_bump_server_error(server_name)to_reset_server_error(server_name)in the JSON error-check path, with explanatory comments referencing MCP circuit breaker triggered by business errors (isError) instead of actual server failures #47851