-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: #2061 Sanitize invalid JSON in tool call arguments #2062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for sending this PR. Validating the JSON data format itself is fine (OpenAI SDK's code comment recommends doing so too), but I am unsure if falling back to an empty JSON data works well in the scenario, and it may be even more confusing for developers to debug. Sending an invalid JSON data to the model and receiving the error clearly telling the arguments are broken may be okay. |
|
@seratch Your concern is valid, but this issue causes sessions to break entirely. Also, when this issue occurs, the currently raised errors are just as confusing for debugging; it is not clear that the issue is in a certain tool call's arguments. Examples: Perhaps this SDK could raise more informative error, or proceed with the sanitization but with a warning level instead of debug. I'm also open to hearing your suggestions. |
|
@habema The From a broader perspective, I think the root issue is related to the current session design. Maybe we could provide a small helper function to clear out problematic session items, but deciding when to use it should probably be left to the developer, since we cannot know exactly which server-side API errors require clearing. It’s more like a retry or recovery feature. Just my two cents. |
|
@ihower Thank you. Yes, the As for the session's design, I think this can be very opinionated. In my opinion, I think the session should reflect the exact interactions that happen without alteration. If any sanitization is needed, this can be done inside the On one side, this takes away the responsibility of handling such problems from this SDK's core internals, and delegates to the session, which can be implemented in a custom manner (and at this point I believe any production use-case will need its own custom session implementation). On the other side, and as @seratch mentioned earlier:
After all the broken JSON is what the agent sent. Since I managed to solve this issue by implementing a function very similar |
Resolves #2061
Fixes issue where invalid JSON in tool call arguments (stored in session history) would break API calls when using Anthropic models via LiteLLM.
Solution:
_sanitize_tool_call_arguments()method that validates JSON before sending to APIs"{}"as safe default (prevents API errors while maintaining compatibility)Not too sure if the logging should be more verbose, I'll leave that to you.