fix: wrong error message and swallowed JSONDecodeError in tool.py#35642
fix: wrong error message and swallowed JSONDecodeError in tool.py#35642jnMetaCode wants to merge 1 commit intolangchain-ai:masterfrom
Conversation
Two bug fixes: 1. ToolMessageChunk.__add__ checks tool_call_id but the error message incorrectly says "different names" instead of "different tool_call_ids". 2. default_tool_parser catches JSONDecodeError but passes error=None instead of the actual error message, silently swallowing the error. Signed-off-by: JiangNan <1394485448@qq.com>
Merging this PR will not alter performance
|
Ethan T. (gambletan)
left a comment
There was a problem hiding this comment.
Two small but meaningful fixes here:
-
Error message fix in
ToolMessageChunk.__add__(line 186): Changing"different names"to"different tool_call_ids"is correct — the check is onself.tool_call_id != other.tool_call_id, so the error message should referencetool_call_id, notname. Good catch. -
Propagating
JSONDecodeErrorintoinvalid_tool_calls(line 374-381): Previouslyerror=Noneswallowed the decode error, making it impossible to debug why a tool call was marked invalid. Nowerror=str(e)preserves the actual parse error message. This is a clear improvement for debuggability.One thought: the
str(e)representation ofjson.JSONDecodeErrorincludes the message, document snippet, line number, and column — e.g."Expecting property name enclosed in double quotes: line 1 column 2 (char 1)". This is informative but could be verbose if the malformed JSON string is large. Sinceinvalid_tool_callstores this as a string field, it should be fine, but worth being aware of.
Both fixes are low-risk and clearly correct. This PR could benefit from a small unit test for the JSONDecodeError propagation — for instance, verifying that when function.arguments contains malformed JSON, the resulting invalid_tool_calls[0].error is a non-empty string. That said, the existing test infrastructure may already cover this indirectly.
Clean PR, LGTM.
Summary
ToolMessageChunk.__add__: The code checkstool_call_idbut the error message said "different names" instead of "different tool_call_ids".JSONDecodeErrorindefault_tool_parser: When JSON parsing fails, the exception was caught buterror=Nonewas passed toinvalid_tool_call(), discarding the actual error information. Now passesstr(e)so callers can see why parsing failed.Files Changed
libs/core/langchain_core/messages/tool.pyTest plan
tool_call_idsJSONDecodeErrormessage is preserved ininvalid_tool_call.error