Repository navigation
fix: preserve upstream MCP tool errors - #1213
Conversation
|
👋 Welcome, @tridev-tej, and thanks for opening your first PR on AnythingMCP! A few quick pointers:
Someone from the core team will look at this within ~48h. If you don't hear back, please ping us in Discussions / Q&A. ⭐ While you wait — if you find AnythingMCP useful, a star helps others discover it. |
|
All contributors have signed the CLA ✍️ ✅ |
helpcodeai
left a comment
There was a problem hiding this comment.
Thanks @tridev-tej, and welcome! The behaviour is right and the tests cover what #1203 asks for.
The problem is the size. Most of the diff splits executeTool into a dozen helpers, and that's the path every tool call in AnythingMCP goes through, so I can't review a refactor of it alongside a bug fix. Could you cut it down to the fix only? Keep executeTool as it is, and right after executeWithEngine, if the connector is MCP and the body is an MCP result with isError: true, log it as ERROR, skip the cache and the KG ingest, and return the error. That plus your tests should be a few dozen lines. If you think the refactor is worth it on its own, open an issue first and we can talk about it there.
One more detail: return a text version of the upstream content (like the success path does) instead of passing the upstream result object through, so _meta, structuredContent or other content types from a remote server don't reach the client unfiltered.
And before we can merge anything, please sign the CLA: post a comment with exactly "I have read the CLA Document and I hereby sign the CLA" and nothing else.
|
Thanks @tridev-tej, this is exactly the size I was hoping for, and returning only the text parts is the right call. I've started CI on the fork. Small optional thing: if a server sends an error with no text content at all, The only thing blocking the merge is the CLA. Post a new comment with just this line and nothing else, and I'll merge once CI is green: I have read the CLA Document and I hereby sign the CLA |
|
I have read the CLA Document and I hereby sign the CLA |
|
hi @helpcodeai, CLA signed and CI is green. Could you merge this? |
helpcodeai
left a comment
There was a problem hiding this comment.
Thanks @tridev-tej, CLA signed and CI green. Merging.
Summary
MCP tool failures were logged and cached as successes.
Changes
isError.Type
Testing
Checklist
Related Issues
Fixes #1203