-
Notifications
You must be signed in to change notification settings - Fork 143
fix: Updating error payloads after the transcoding changes correction. #783
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,6 @@ | |||||
| import static io.a2a.common.MediaType.APPLICATION_JSON; | ||||||
| import static io.a2a.server.util.async.AsyncUtils.createTubeConfig; | ||||||
|
|
||||||
| import io.a2a.spec.A2AErrorCodes; | ||||||
|
|
||||||
| import java.time.Instant; | ||||||
| import java.time.format.DateTimeParseException; | ||||||
|
|
@@ -40,6 +39,7 @@ | |||||
| import io.a2a.server.version.A2AVersionValidator; | ||||||
| import io.a2a.server.util.async.Internal; | ||||||
| import io.a2a.spec.A2AError; | ||||||
| import io.a2a.spec.A2AErrorCodes; | ||||||
| import io.a2a.spec.AgentCard; | ||||||
| import io.a2a.spec.CancelTaskParams; | ||||||
| import io.a2a.spec.ContentTypeNotSupportedError; | ||||||
|
|
@@ -122,13 +122,13 @@ | |||||
| public class RestHandler { | ||||||
|
|
||||||
| private static final Logger log = Logger.getLogger(RestHandler.class.getName()); | ||||||
| private static final String TASK_STATE_PREFIX = "TASK_STATE_"; | ||||||
|
|
||||||
| // Fields set by constructor injection cannot be final. We need a noargs constructor for | ||||||
| // Jakarta compatibility, and it seems that making fields set by constructor injection | ||||||
| // final, is not proxyable in all runtimes | ||||||
| private AgentCard agentCard; | ||||||
| private @Nullable Instance<AgentCard> extendedAgentCard; | ||||||
| private @Nullable | ||||||
| Instance<AgentCard> extendedAgentCard; | ||||||
| private AgentCardCacheMetadata cacheMetadata; | ||||||
| private RequestHandler requestHandler; | ||||||
| private Executor executor; | ||||||
|
|
@@ -377,7 +377,7 @@ public HTTPRestResponse createTaskPushNotificationConfiguration(ServerCallContex | |||||
| if (!taskIdFromBody.isEmpty() && !taskIdFromBody.equals(taskId)) { | ||||||
| throw new InvalidParamsError("Task ID in request body (" + taskIdFromBody + ") does not match task ID in URL path (" + taskId + ")."); | ||||||
| } | ||||||
|
|
||||||
| builder.setTenant(tenant); | ||||||
| builder.setTaskId(taskId); | ||||||
| TaskPushNotificationConfig result = requestHandler.onCreateTaskPushNotificationConfig(ProtoUtils.FromProto.createTaskPushNotificationConfig(builder), context); | ||||||
|
|
@@ -766,29 +766,38 @@ private static int mapErrorToHttpStatus(A2AError error) { | |||||
| if (error instanceof InvalidParamsError) { | ||||||
| return 422; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with the rest of the method and to ensure that HTTP status codes are centralized in the
Suggested change
References
|
||||||
| } | ||||||
| if (error instanceof MethodNotFoundError || error instanceof TaskNotFoundError) { | ||||||
| return 404; | ||||||
| if (error instanceof MethodNotFoundError) { | ||||||
| return A2AErrorCodes.METHOD_NOT_FOUND.httpCode(); | ||||||
| } | ||||||
| if (error instanceof TaskNotFoundError) { | ||||||
| return A2AErrorCodes.TASK_NOT_FOUND.httpCode(); | ||||||
| } | ||||||
| if (error instanceof TaskNotCancelableError) { | ||||||
| return 409; | ||||||
| return A2AErrorCodes.TASK_NOT_CANCELABLE.httpCode(); | ||||||
| } | ||||||
| if (error instanceof UnsupportedOperationError) { | ||||||
| return 501; | ||||||
| return A2AErrorCodes.UNSUPPORTED_OPERATION.httpCode(); | ||||||
| } | ||||||
| if (error instanceof ContentTypeNotSupportedError) { | ||||||
| return 415; | ||||||
| return A2AErrorCodes.CONTENT_TYPE_NOT_SUPPORTED.httpCode(); | ||||||
| } | ||||||
| if (error instanceof InvalidAgentResponseError) { | ||||||
| return 502; | ||||||
| return A2AErrorCodes.INVALID_AGENT_RESPONSE.httpCode(); | ||||||
| } | ||||||
| if (error instanceof ExtendedAgentCardNotConfiguredError) { | ||||||
| return A2AErrorCodes.EXTENDED_AGENT_CARD_NOT_CONFIGURED.httpCode(); | ||||||
| } | ||||||
| if (error instanceof ExtensionSupportRequiredError) { | ||||||
| return A2AErrorCodes.EXTENSION_SUPPORT_REQUIRED.httpCode(); | ||||||
| } | ||||||
| if (error instanceof ExtendedAgentCardNotConfiguredError | ||||||
| || error instanceof ExtensionSupportRequiredError | ||||||
| || error instanceof VersionNotSupportedError | ||||||
| || error instanceof PushNotificationNotSupportedError) { | ||||||
| return 400; | ||||||
| if (error instanceof VersionNotSupportedError) { | ||||||
| return A2AErrorCodes.VERSION_NOT_SUPPORTED.httpCode(); | ||||||
| } | ||||||
| if (error instanceof PushNotificationNotSupportedError) { | ||||||
| return A2AErrorCodes.PUSH_NOTIFICATION_NOT_SUPPORTED.httpCode(); | ||||||
| } | ||||||
| if (error instanceof InternalError) { | ||||||
| return 500; | ||||||
| return A2AErrorCodes.INTERNAL.httpCode(); | ||||||
| } | ||||||
| return 500; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is better to use the centralized
Suggested change
References
|
||||||
| } | ||||||
|
|
@@ -827,7 +836,7 @@ public HTTPRestResponse getExtendedAgentCard(ServerCallContext context, String t | |||||
| } catch (A2AError e) { | ||||||
| return createErrorResponse(e); | ||||||
| } catch (Throwable t) { | ||||||
| return createErrorResponse(500, new InternalError(t.getMessage())); | ||||||
| return createErrorResponse(A2AErrorCodes.INTERNAL.httpCode(), new InternalError(t.getMessage())); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -1036,12 +1045,16 @@ public String toString() { | |||||
| return "HTTPRestErrorResponse{error=" + error + '}'; | ||||||
| } | ||||||
|
|
||||||
| private record ErrorBody(int code, String status, String message, List<ErrorDetail> details) {} | ||||||
| private record ErrorBody(int code, String status, String message, List<ErrorDetail> details) { | ||||||
|
|
||||||
| } | ||||||
|
|
||||||
| private record ErrorDetail( | ||||||
| @com.google.gson.annotations.SerializedName("@type") String type, | ||||||
| String reason, | ||||||
| String domain, | ||||||
| Map<String, Object> metadata) {} | ||||||
| Map<String, Object> metadata) { | ||||||
|
|
||||||
| } | ||||||
|
Comment on lines
+1048
to
+1058
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These records were previously defined as concise one-liners. The addition of empty lines inside the record bodies is unnecessary and deviates from the project's style for simple data carriers. private record ErrorBody(int code, String status, String message, List<ErrorDetail> details) {}
private record ErrorDetail(
@com.google.gson.annotations.SerializedName("@type") String type,
String reason,
String domain,
Map<String, Object> metadata) {} |
||||||
| } | ||||||
| } | ||||||
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.
The
@Nullableannotation should be on the same line as the field type to maintain consistent formatting and readability.