feat: Remove legacy JSON-RPC errors#360
feat: Remove legacy JSON-RPC errors#360bartek-gralewicz merged 11 commits intoepic/1.0_breaking_changesfrom
Conversation
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 refactors the error handling mechanism within the 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
|
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring to decouple errors from the JSON-RPC transport layer. By removing the legacy A2AError and introducing transport-agnostic error classes, the codebase is now more robust and better prepared for multiple transport protocols. The changes have been applied consistently across all relevant files, including server-side handlers (REST, gRPC, JSON-RPC), client-side transports, and the core request handler logic. The tests have also been diligently updated to reflect these changes. Overall, this is an excellent improvement to the project's architecture and maintainability. I have one minor suggestion to improve code consistency.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request successfully refactors the error handling mechanism by removing the legacy JSON-RPC specific A2AError class and introducing a set of transport-agnostic error classes (RequestMalformedError, GenericError, TaskNotFoundError, etc.). This change significantly improves the modularity and flexibility of the error system, allowing for better support of multiple transports as stated in the PR description. The updates across client and server transport handlers, as well as converters and tests, demonstrate a thorough implementation of this new error model. The removal of as <Type> assertions in grpc_transport.ts also enhances type safety.
Description
Update to errors usage in
a2a-js. Previously, errors were based around JSON-RPC transport but since now, multiple transports are supported, errors thrown by the server should be transport agnostic.List of changes
src/server/error.ts.as <Type>usage ingrpc_transport.tsas suggested here: feat: Remove Proto Conversions #356 (comment)Fixes (partially) #179 🦕