fix: guard streaming tool call JSON.parse to prevent generator crashes#644
Conversation
…rator crashes Malformed streaming tool call arguments from OpenAI or Anthropic APIs would throw an uncaught SyntaxError, terminating the async generator and losing all accumulated state. Now falls back to empty args with a warning log.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: d35873a The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the stability and resilience of the LLM streaming functionality by implementing robust error handling for malformed JSON in tool call arguments. By preventing generator crashes caused by invalid JSON, it ensures that streaming operations can continue uninterrupted, preserving valuable state and improving the overall reliability of the system. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively prevents potential crashes caused by malformed JSON in streaming tool call arguments by introducing a safeParseToolArgs helper function. The implementation is solid and includes tests to validate the fix. I've included a suggestion to enhance maintainability by centralizing the new helper function.
| function safeParseToolArgs( | ||
| json: string | undefined, | ||
| logger: Logger, | ||
| ): Record<string, unknown> { | ||
| try { | ||
| return JSON.parse(json || "{}"); | ||
| } catch (error) { | ||
| logger.warn("Failed to parse tool call arguments, using empty args", { | ||
| rawArgs: json, | ||
| error: String(error), | ||
| }); | ||
| return {}; | ||
| } | ||
| } |
There was a problem hiding this comment.
This safeParseToolArgs function is also defined in packages/adk/src/models/openai-llm.ts. To avoid code duplication and improve maintainability, consider extracting this function into a shared utility file (e.g., in a new packages/adk/src/models/utils.ts or a similar existing one) and importing it where needed.
Addresses review feedback to deduplicate the helper. Also adds changeset.
|
@claude review |
|
Claude finished @MarvelNwachukwu's task —— View job Code ReviewI've reviewed the PR that fixes streaming JSON parse crashes in OpenAI and Anthropic LLMs. Here's my comprehensive analysis: Todo List:
✅ Overall Assessment: Strong Fix with Good ImplementationThis PR effectively addresses a critical bug where malformed JSON in streaming tool call arguments could crash async generators. The implementation is solid and well-tested. 🎯 What This Fixes
✅ Implementation Strengths1. Smart Shared Utility ApproachThe PR correctly extracted 2. Robust Error Handlingexport function safeParseToolArgs(
json: string | undefined,
logger: Logger,
): Record<string, unknown> {
try {
return JSON.parse(json || "{}");
} catch (error) {
logger.warn("Failed to parse tool call arguments, using empty args", {
rawArgs: json,
error: String(error),
});
return {};
}
}
3. Comprehensive Integration
✅ Test Coverage ExcellenceAnthropic Test (
|
Timonwa
left a comment
Timonwa
left a comment
There was a problem hiding this comment.
Nice work @MarvelNwachukwu! Since safeParseToolArgs is now in a shared llm-utils.ts module, should it also be applied to anthropic-llm.ts and other LLM providers, or is OpenAI the only one that can send malformed JSON during streaming?
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
Thanks @Timonwa! Good question — yes, |
|
cool. |
2 participants
Pull Request
Description
Bare
JSON.parsecalls on streaming tool call arguments inOpenAiLlmandAnthropicLlmcould throw an uncaughtSyntaxErrorwhen the API returns malformed JSON (e.g. truncated streams, network issues). This crashes the entire async generator, losing all accumulated state — text, other tool calls, and usage metadata. This PR wraps each call in asafeParseToolArgshelper that catches parse errors, logs a warning, and falls back to empty args so the response can still be yielded.Related Issue
Fixes #650
Type of Change
How Has This Been Tested?
anthropic-llm.test.ts: streaming with truncated tool call JSON completes without crashing, returns empty argsopenai-llm.test.ts:openAiMessageToLlmResponsewith malformed tool call JSON returns empty argspnpm test -- anthropic-llm,pnpm test -- openai-llm)pnpm build --filter=@iqai/adksucceeds with no type errorsChecklist