-
Notifications
You must be signed in to change notification settings - Fork 116
fix: throw on JSON-RPC response id mismatch #318
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
fix: throw on JSON-RPC response id mismatch #318
Conversation
Per the JSON-RPC 2.0 specification, a response id must match the request id. A mismatch indicates a broken protocol invariant. Previously, mismatches were logged via console.error/console.warn but execution continued, potentially causing silent data corruption. Now throws an error in both the unary (_sendRpcRequest) and streaming (_processSseEventData) code paths. Fixes a2aproject#176 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @corvid-agent, 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 enhances the robustness of the JSON-RPC transport by strictly adhering to the JSON-RPC 2.0 specification regarding response ID matching. Instead of merely logging warnings or errors, the system now throws exceptions when a response ID does not correspond to the original request ID. This critical change prevents potential silent data corruption and ensures that protocol invariant violations are immediately surfaced, leading to more predictable and reliable communication. Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🧪 Code Coverage
Generated by coverage-comment.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively addresses the critical issue of JSON-RPC response ID mismatches by transitioning from logging errors to throwing explicit errors. This change aligns with the JSON-RPC 2.0 specification, treating protocol invariant violations as critical failures, which is a significant improvement for client robustness. The corresponding update to the SSE event processing catch block ensures that these new, specific errors are correctly propagated, preserving valuable context for debugging and handling.
…Data Move validation logic (ID mismatch, error response, missing result) outside the try/catch block so errors no longer need to be caught and re-thrown based on message string matching. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ishymko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Summary
Replaces
console.error/console.warncalls with thrown errors when a JSON-RPC response id does not match the request id. Per the JSON-RPC 2.0 specification, this is a protocol invariant violation — continuing execution with a mismatched response can lead to silent data corruption.Changes
_sendRpcRequest:console.error→throw new Erroron id mismatch_processSseEventData:console.warn→throw new Erroron id mismatch_processSseEventDatacatch block: Added the new error message to the pass-through condition so it is not swallowed and re-wrapped as a parse errorContext
From the code review on PR #169: "Go SDK doesn't perform any validation, but I'd probably be throwing in case of a mismatch" — @yarolegovich
Fixes #176
🤖 Generated with Claude Code