From b75c9169c630ab0450d16aa74898da6b953d0e78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=82osz=20Sobczyk?= Date: Thu, 6 Aug 2026 04:07:51 -0700 Subject: [PATCH] fix(a2a): drop unparseable A2A metadata instead of aborting conversion `ResponseConverter.parseMetadata` rethrew every deserialization failure as `IllegalArgumentException`, which propagated out of `taskToEvent`, `messageToEvent` and `handleTaskUpdate`. Because `adk_grounding_metadata`, `adk_usage_metadata`, `adk_custom_metadata` and `adk_error_code` are all peer-controlled, a single unparseable value from a remote agent could break the caller's turn. Log at WARN and drop the offending field instead, so auxiliary telemetry cannot take down the whole conversion. On the Java side these keys are read-only: nothing in ADK Java writes them, so a parse failure is always peer data, never our own serialization. This matches the drop semantics in ADK Kotlin (`LegacyA2aConverters.kt` logs WARN and returns null) and ADK Python (`to_adk_event._extract_genai_metadata` logs and returns None); Java was the outlier. The warning here omits the parser message, which quotes the peer's bytes, so it follows Python rather than Kotlin, which still attaches the exception. PiperOrigin-RevId: 960211499 --- .../adk/a2a/converters/ResponseConverter.java | 82 ++++++++++----- .../a2a/converters/ResponseConverterTest.java | 99 +++++++++++++++++-- 2 files changed, 147 insertions(+), 34 deletions(-) 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