diff --git a/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java b/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java index eb670bb79..c8d20dbdd 100644 --- a/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java +++ b/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java @@ -20,7 +20,7 @@ import static com.google.common.collect.Streams.zip; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.adk.agents.InvocationContext; import com.google.adk.events.Event; @@ -59,6 +59,8 @@ public final class ResponseConverter { private static final ObjectMapper objectMapper = new ObjectMapper(); private static final Logger logger = LoggerFactory.getLogger(ResponseConverter.class); + private static final JavaType CUSTOM_METADATA_LIST_TYPE = + objectMapper.getTypeFactory().constructCollectionType(List.class, CustomMetadata.class); private static final ImmutableSet PENDING_STATES = ImmutableSet.of(TaskState.WORKING, TaskState.SUBMITTED); @@ -69,6 +71,8 @@ private ResponseConverter() {} * empty optional if the event should be ignored (e.g. if the event is not a final update for * TaskArtifactUpdateEvent or if the message is empty for TaskStatusUpdateEvent). * + *

Unparseable ADK metadata is logged and dropped; the rest of the event is still converted. + * * @throws IllegalArgumentException if the event type is not supported. */ public static Optional clientEventToEvent( @@ -187,7 +191,11 @@ public static Event messageToFailedEvent(Message message, InvocationContext invo return builder.build(); } - /** Converts an A2A message back to ADK events. */ + /** + * Converts an A2A message back to ADK events. + * + *

Unparseable ADK metadata is logged and dropped; the rest of the event is still converted. + */ public static Event messageToEvent(Message message, InvocationContext invocationContext) { return updateEventMetadata( remoteAgentEventBuilder(invocationContext) @@ -217,6 +225,8 @@ public static Event messageToEvent( * Converts an A2A {@link Task} to an ADK {@link Event}. If the artifacts are present, the last * artifact is used. If not, the status message is used. If not, the last history message is used. * If none of these are present, an empty event is returned. + * + *

Unparseable ADK metadata is logged and dropped; the rest of the event is still converted. */ public static Event taskToEvent(Task task, InvocationContext invocationContext) { ImmutableList.Builder genaiParts = ImmutableList.builder(); @@ -298,13 +308,13 @@ private static Event updateEventMetadata( clientMetadata = ImmutableMap.of(); } Event.Builder eventBuilder = event.toBuilder(); - Object groundingMetadata = clientMetadata.get(A2AMetadataKey.GROUNDING_METADATA.getType()); - // if groundingMetadata is null, parseMetadata will return null as well. - eventBuilder.groundingMetadata(parseMetadata(groundingMetadata, GroundingMetadata.class)); - Object usageMetadata = clientMetadata.get(A2AMetadataKey.USAGE_METADATA.getType()); - // if usageMetadata is null, parseMetadata will return null as well. + eventBuilder.groundingMetadata( + parseMetadata(clientMetadata, A2AMetadataKey.GROUNDING_METADATA, GroundingMetadata.class)); eventBuilder.usageMetadata( - parseMetadata(usageMetadata, GenerateContentResponseUsageMetadata.class)); + parseMetadata( + clientMetadata, + A2AMetadataKey.USAGE_METADATA, + GenerateContentResponseUsageMetadata.class)); ImmutableList.Builder customMetadataList = ImmutableList.builder(); customMetadataList @@ -318,32 +328,35 @@ private static Event updateEventMetadata( .key(AdkMetadataKey.CONTEXT_ID.getType()) .stringValue(contextId) .build()); - Object customMetadata = clientMetadata.get(A2AMetadataKey.CUSTOM_METADATA.getType()); - if (customMetadata != null) { - customMetadataList.addAll( - parseMetadata(customMetadata, new TypeReference>() {})); + List parsedCustomMetadata = + parseMetadata(clientMetadata, A2AMetadataKey.CUSTOM_METADATA, CUSTOM_METADATA_LIST_TYPE); + if (parsedCustomMetadata != null) { + customMetadataList.addAll(parsedCustomMetadata); } eventBuilder.customMetadata(customMetadataList.build()); - Object errorCode = clientMetadata.get(A2AMetadataKey.ERROR_CODE.getType()); - eventBuilder.errorCode(parseMetadata(errorCode, FinishReason.class)); + eventBuilder.errorCode( + parseMetadata(clientMetadata, A2AMetadataKey.ERROR_CODE, FinishReason.class)); return eventBuilder.build(); } - private static @Nullable T parseMetadata(@Nullable Object metadata, Class type) { - try { - if (metadata instanceof String jsonString) { - return objectMapper.readValue(jsonString, type); - } else { - return objectMapper.convertValue(metadata, type); - } - } catch (IllegalArgumentException | JsonProcessingException e) { - throw new IllegalArgumentException("Failed to parse metadata of type " + type, e); - } + /** + * Reads {@code key} out of the peer-supplied {@code clientMetadata} and deserializes it. + * + *

Returns null when the key is absent, and also when its value cannot be parsed: metadata is + * peer-controlled, so a malformed value is logged and dropped rather than failing the whole + * conversion. + */ + private static @Nullable T parseMetadata( + Map clientMetadata, A2AMetadataKey key, Class type) { + return parseMetadata(clientMetadata, key, objectMapper.getTypeFactory().constructType(type)); } - private static @Nullable T parseMetadata(@Nullable Object metadata, TypeReference type) { + /** Overload of {@link #parseMetadata(Map, A2AMetadataKey, Class)} for generic target types. */ + private static @Nullable T parseMetadata( + Map clientMetadata, A2AMetadataKey key, JavaType type) { + Object metadata = clientMetadata.get(key.getType()); try { if (metadata instanceof String jsonString) { return objectMapper.readValue(jsonString, type); @@ -351,10 +364,27 @@ private static Event updateEventMetadata( return objectMapper.convertValue(metadata, type); } } catch (IllegalArgumentException | JsonProcessingException e) { - throw new IllegalArgumentException("Failed to parse metadata of type " + type.getType(), e); + logDroppedMetadata(key, e); + return null; } } + /** + * Reports a dropped metadata value. + * + *

The parser's message quotes the peer's bytes, so the warning carries only the key and the + * exception type. A peer that streams malformed metadata would otherwise be able to write + * arbitrary content and a stack trace into the log on every event. The full exception is + * available at debug level. + */ + private static void logDroppedMetadata(A2AMetadataKey key, Exception e) { + logger.warn( + "Dropping unparseable A2A metadata for key {} ({})", + key.getType(), + e.getClass().getSimpleName()); + logger.debug("Unparseable A2A metadata for key {}", key.getType(), e); + } + private static Event emptyEvent(InvocationContext invocationContext) { Event.Builder builder = Event.builder() diff --git a/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java b/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java index 899af5c34..9b854b616 100644 --- a/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java +++ b/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java @@ -19,7 +19,6 @@ import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.joining; -import static org.junit.Assert.assertThrows; import com.google.adk.agents.BaseAgent; import com.google.adk.agents.InvocationContext; @@ -214,6 +213,92 @@ public void taskToEvent_withCustomMetadata_returnsEvent() { .inOrder(); } + @Test + public void taskToEvent_withMalformedMetadata_dropsFieldsAndConverts() { + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(new TextPart("Status message"))) + .build(); + TaskStatus status = new TaskStatus(TaskState.WORKING, statusMessage, null); + Task task = + testTask() + .status(status) + .artifacts(null) + .metadata( + ImmutableMap.of( + A2AMetadataKey.GROUNDING_METADATA.getType(), "not-valid-json", + A2AMetadataKey.USAGE_METADATA.getType(), "not-valid-json", + A2AMetadataKey.CUSTOM_METADATA.getType(), "not-valid-json", + A2AMetadataKey.ERROR_CODE.getType(), "not-valid-json")) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.content().get().parts().get().get(0).text()).hasValue("Status message"); + assertThat(event.groundingMetadata()).isEmpty(); + assertThat(event.usageMetadata()).isEmpty(); + assertThat(event.errorCode()).isEmpty(); + assertThat(event.customMetadata().get()) + .containsExactly( + CustomMetadata.builder().key("a2a:task_id").stringValue("task-1").build(), + CustomMetadata.builder().key("a2a:context_id").stringValue("context-1").build()); + } + + @Test + public void taskToEvent_withUnrecognizedMetadataField_dropsField() { + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(new TextPart("Status message"))) + .build(); + TaskStatus status = new TaskStatus(TaskState.WORKING, statusMessage, null); + Task task = + testTask() + .status(status) + .artifacts(null) + .metadata( + ImmutableMap.of( + // A nested object takes the convertValue branch rather than readValue. The + // genai builders reject unknown fields, so snake_case fails to convert. + A2AMetadataKey.GROUNDING_METADATA.getType(), + ImmutableMap.of("web_search_queries", ImmutableList.of("test-query")))) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.groundingMetadata()).isEmpty(); + assertThat(event.content().get().parts().get().get(0).text()).hasValue("Status message"); + } + + @Test + public void taskToEvent_withOneMalformedMetadataField_keepsTheValidFields() { + GroundingMetadata groundingMetadata = + GroundingMetadata.builder().webSearchQueries("test-query").build(); + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(new TextPart("Status message"))) + .build(); + TaskStatus status = new TaskStatus(TaskState.WORKING, statusMessage, null); + Task task = + testTask() + .status(status) + .artifacts(null) + .metadata( + ImmutableMap.of( + A2AMetadataKey.GROUNDING_METADATA.getType(), + groundingMetadata.toJson(), + A2AMetadataKey.USAGE_METADATA.getType(), + "not-valid-json")) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.groundingMetadata()).hasValue(groundingMetadata); + assertThat(event.usageMetadata()).isEmpty(); + } + @Test public void messageToEvent_withMissingTaskId_returnsEvent() { Message a2aMessage = @@ -543,7 +628,7 @@ public void clientEventToEvent_withFailedTaskStatusUpdateEvent_returnsErrorEvent } @Test - public void taskToEvent_withInvalidMetadata_throwsException() { + public void taskToEvent_withInvalidMetadata_dropsFieldInsteadOfThrowing() { Message statusMessage = new Message.Builder() .role(Message.Role.AGENT) @@ -558,12 +643,10 @@ public void taskToEvent_withInvalidMetadata_throwsException() { ImmutableMap.of(A2AMetadataKey.GROUNDING_METADATA.getType(), "{ invalid json ]")) .build(); - IllegalArgumentException exception = - assertThrows( - IllegalArgumentException.class, - () -> ResponseConverter.taskToEvent(task, invocationContext)); - assertThat(exception).hasMessageThat().contains("Failed to parse metadata"); - assertThat(exception).hasMessageThat().contains("GroundingMetadata"); + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.groundingMetadata()).isEmpty(); + assertThat(event.content().get().parts().get().get(0).text()).hasValue("Status message"); } @Test