fix(connectors): return tool errors as results so the model can self-correct (#427) - #519
Open
hari (Mr-Neutr0n) wants to merge 1 commit into
Open
Conversation
…correct (langchain-ai#427) The connector tools threw on error instead of returning the message as the tool result. A thrown tool error aborts the agent run, and even when the run survives it, the model never sees the text - which matters because these messages are written FOR the model. callMcpTool answers a wrong tool name with MCP tool notion-get-page-content was not returned by tools/list for notion. Run openwiki_list_mcp_tools first and use an exact discovered name. which is exactly the hint needed to retry correctly. Verified before/after on that path: invoke() rejected, and now returns "Tool error: notion MCP connector is not enabled." Errors are surfaced, not swallowed: the result is prefixed `Tool error:` so a failure is never mistaken for data. CONTRACT CHANGE worth a maintainer's eye: three tests in test/raw-connector-tools.test.ts asserted that symlink rejections *throw* (`.rejects.toThrow(/symbolic links/u)`). They now assert the same refusal on the result channel instead. The security property is unchanged - the symlink is still refused before any read, and the reason still reaches the caller - but if you would rather security refusals keep hard-failing while ordinary tool errors are returned, say so and I will exempt the raw-file tools from the wrapper. Scope note: LangChain rejects schema violations before `func` runs, so those remain rejections. This covers errors thrown inside the tool body, which is what the issue is about. Tests: test/connector-tool-errors.test.ts - 6 cases (thrown Error, rejected promise, non-Error rejection, success passthrough untouched, the issue's MCP tool-name case end to end, and a successful connector call).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #427.
The connector tools threw on error instead of returning the message as the tool result. A thrown tool error aborts the agent run, and even when the run survives it, the model never sees the text — which matters because these messages are written for the model.
callMcpToolanswers a wrong tool name with:That is precisely the hint needed to retry correctly, and the model never received it.
Verified before/after on that exact path
Errors are surfaced, not swallowed — the result is prefixed
Tool error:so a failure is never mistaken for data.Three tests in
test/raw-connector-tools.test.tsasserted that symlink rejections throw:They now assert the same refusal on the result channel instead. The security property is unchanged — the symlink is still refused before any read, and the reason still reaches the caller — but this is a real change to what those tests pin, and they are security tests, so I did not want to quietly rewrite them and move on.
If you would rather security refusals keep hard-failing while ordinary tool errors are returned, say so and I will exempt the raw-file tools from the wrapper. That is a two-line change.
Scope
LangChain rejects schema violations before
funcruns, so those remain rejections — a different layer. This covers errors thrown inside the tool body, which is what the issue is about.Tests
test/connector-tool-errors.test.ts— 6 cases: thrownError, rejected promise, non-Errorrejection, success passthrough untouched, the issue's MCP tool-name case end to end, and a successful connector call still returning parseable JSON.Full suite 775 passed, typecheck and
lint:checkclean.Reviewed and tested locally; drafted with AI assistance.