Add error classes by kabir · Pull Request #22 · a2aproject/a2a-java

kabir · 2025-05-07T20:04:43Z

Fixes #1

kabir · 2025-05-07T20:05:27Z

@@ -0,0 +1,4 @@
+package io.a2a.spec;
+
+public class A2AClientError extends Exception {


Not 100% sure here, the Python implementation is:

class A2AClientError(Exception): pass

Maybe we could add a constructor here that just takes a string message?

pass means "no implementation" in python. It's like an interface in Java, just for typing

well bad explanation...since it extends Exception class, in python it inherits also its constructor. IOW pass just pass anything to the super class. So this code should work

def a_method(): raise A2AClientError("Connection failed") try: amehod() except A2AClientError as e: print(f"Exception message: {e}")

and it should print something like: Exception message: Connection failed

kabir · 2025-05-07T20:06:28Z

@@ -0,0 +1,4 @@
+package io.a2a.spec;
+
+public class A2AClientJSONError extends A2AClientError {


Not totally sure here either, here is the Python

class A2AClientJSONError(A2AClientError): def __init__(self, message: str): self.message = message super().__init__(f'JSON Error: {message}')

Looks like they're just prefixing the message string with "JSON Error". We can always do that later if we think it's useful. I see now that A2AClientJSONError isn't in a2a.json so what we do here is mostly up to us.

kabir · 2025-05-07T20:07:12Z

@@ -0,0 +1,5 @@
+package io.a2a.spec;
+
+public class MissingAPIKeyError extends Exception {


Same doubt as for the A2AClientError

fjuma · 2025-05-07T20:28:51Z

 * Represents a JSONRPC error.
 */
-public record JSONRPCError(int code, String message, Object data) {
+public class JSONRPCError {


Just to check, any reason to convert this to a class? This is immutable and we're using Jackson to convert to/from JSON. It looks like the MCP Java SDK used records for errors like this.

Ah, I see now, we need to use a class because the other errors need to extend this one, right?

@fjuma Yeah, I wasn't able to inherit from Records. It says they are final

fjuma · 2025-05-07T20:44:21Z

+
+public class UnsupportedOperationError extends JSONRPCError {
+
+    public UnsupportedOperationError(int code, String message, Object data) {


For classes like this that extend JSONRPCError, I believe we'll need the @JsonCreator annotation on the constructor and @JsonProperty("blah") annotations on each parameter to be able to successfully create the object from JSON. Let me know if I'm missing something though.

I need to read up a bit on this :-)

fjuma · 2025-05-07T20:47:46Z

+public class UnsupportedOperationError extends JSONRPCError {
+
+    public UnsupportedOperationError(int code, String message, Object data) {
+        super(code, message, data);


Looks like the spec specifies default values for code and message:

https://github.com/google/A2A/blob/main/specification/json/a2a.json#L1450
https://github.com/google/A2A/blob/main/samples/python/common/types.py#L303-L304

Looks like the other error types also have defaults that we should use as well.

fjuma · 2025-05-07T21:00:28Z

+        Assert.checkNotNullParam("message", message);
+        this.code = code;
+        this.message = message;
+        this.data = data;


Is data needed here? I don't see it in the python implementation for this one.

kabir · 2025-05-08T11:57:19Z

Hi @fjuma PR updated.

For the root JSONRPCError class I wasn't sure if this should have the annotations, or if it is always one of the subclasses. I originally made it abstract, but looking at spec json, a lot of the Response classes simply have the error as JSONRPCError. So, I think the errors must be getting deserialized on the client, and thus ended up putting the annotations on this class anyway.

I see in the test it is simply deserialized as JSONRPCError. So I'm now wondering a bit if the sub-classes actually need the annotations, but don't know yet whether they are needed or not. Perhaps they are used for the serialization.

fjuma · 2025-05-08T13:34:20Z

+            @JsonProperty("message") String message,
+            @JsonProperty("data") Object data) {
+        super(
+                defaultIfNull(code, -32702),


This should be -32602

fjuma · 2025-05-08T13:36:57Z

+            @JsonProperty("message") String message,
+            @JsonProperty("data") Object data) {
+        super(
+                defaultIfNull(code, -32704),


This should be -32004

fjuma · 2025-05-08T13:49:05Z

Looks good, just a couple small comments on the error codes.

I see in the test it is simply deserialized as JSONRPCError. So I'm now wondering a bit if the sub-classes actually need the annotations, but don't know yet whether they are needed or not. Perhaps they are used for the serialization.

For Part and its sub-classes, I was able to use @JsonTypeInfo and @JsonSubTypes to make sure the correct class gets instantiated:

https://github.com/fjuma/a2a-java-sdk/blob/main/src/main/java/io/a2a/spec/Part.java#L12-L21

For JSONRPCError and its sub-classes, there isn't a type field so this approach wouldn't work there to get the correct class to be instantiated. Maybe we'd need a custom deserializer for something like that? We could always do that as a follow-up issue.

fjuma · 2025-05-08T14:12:49Z

@kabir I created #23 to track looking into deserialization for classes that extend JSONRPCError to see if the correct class can be instantiated based on the error code.

I'll go ahead and merge this PR and submit a small fix for the two error codes. Thanks!

Add error classes

33939af

kabir commented May 7, 2025

View reviewed changes

fjuma reviewed May 7, 2025

View reviewed changes

kabir added 4 commits May 8, 2025 08:46

Fix constructors and remove superfluous messages

8c76a5e

Add Jackson annotations

90151df

Add annotations to the JSONRPCError class

1d65bd8

Test the error deserialization in the client test

cffae02

kabir force-pushed the errors-1 branch from e57e905 to cffae02 Compare May 8, 2025 11:55

fjuma reviewed May 8, 2025

View reviewed changes

fjuma merged commit 370b2d3 into a2aproject:main May 8, 2025

		@@ -0,0 +1,4 @@
		package io.a2a.spec;

		public class A2AClientError extends Exception {

		@@ -0,0 +1,4 @@
		package io.a2a.spec;

		public class A2AClientJSONError extends A2AClientError {

		@@ -0,0 +1,5 @@
		package io.a2a.spec;

		public class MissingAPIKeyError extends Exception {


		public class UnsupportedOperationError extends JSONRPCError {

		public UnsupportedOperationError(int code, String message, Object data) {

Conversation

kabir commented May 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjuma May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maeste May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjuma May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjuma May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kabir commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fjuma commented May 8, 2025

Uh oh!

fjuma commented May 8, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fjuma May 7, 2025 •

edited

Loading

maeste May 8, 2025 •

edited

Loading

fjuma May 7, 2025 •

edited

Loading

fjuma May 7, 2025 •

edited

Loading