AurimasNav opened a new pull request, #43374:
URL: https://github.com/apache/superset/pull/43374
> _Drafted with AI assistance._
Fixes #43358, which @sadpandajoe invited a PR for.
### The problem
`StructuredContentStripperMiddleware.on_call_tool` catches every exception
and returns a `ToolResult` carrying the error text, but never sets `is_error`.
It defaults to `False`, so the call serializes as `isError: false` — a
permission denial or an unhandled crash is indistinguishable from a successful
call to any client that inspects the flag rather than parsing message text.
### The fix
Set `is_error=True` on that last-resort result. The catch-all itself stays
exactly as it is — letting exceptions reach the MCP SDK produces
`CallToolResult(isError=True)` responses that some transports can't encode,
which is the whole reason the handler exists. `is_error` rides along in the
serialized result as a plain boolean, so this restores protocol conformance
without reintroducing the unencodable response.
This is option 1 from the issue discussion, which @dosu also identified as
the lowest-risk path. One correction to the snippet suggested there: the field
is `is_error`, not `isError` — the latter raises `TypeError`.
The PR also preserves the flag through the `structured_content` strip. That
path rebuilds the `ToolResult` and dropped `is_error`, so a tool reporting
failure alongside structured output came back looking successful.
### Verification
Against **fastmcp 3.4.7**, the pinned version:
```
signature: (self, content=None, structured_content=None, meta=None,
is_error: bool = False)
wire JSON: {"content": [{"type": "text", "text": "Error: Permission
denied"}], "isError": true}
default is_error = False
```
The default staying `False` is what keeps successful calls unaffected.
Four tests cover it: a `ToolError` denial and an unexpected exception are
both flagged, a successful result is not, and the flag survives
structured-content stripping.
On the encoding concern raised in the original workaround comment — a
`ToolResult` with `is_error=True` serializes as a plain dict with a boolean
field, not as the exception object that triggered the bridge failure. I don't
have access to the Claude.ai bridge to test that leg directly, so if a
maintainer can confirm against it before merging, that would close the last
open question.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
