From 22df0ccc01bc4d53192cd21680ab4654fc72a61b Mon Sep 17 00:00:00 2001 From: Giulio Leone Date: Thu, 9 Apr 2026 02:24:06 +0200 Subject: [PATCH] fix: guard against None converter results in RemoteA2aAgent handlers Both _handle_a2a_response() and _handle_a2a_response_v2() dereference converter results (accessing event.content, event.custom_metadata) without guarding against None. Converters can legitimately return None for messages with no convertible parts, metadata-only events, or empty status updates. Add None guards after each converter call in both handlers: - Legacy handler: 3 guards (task-no-update, status-update-message, artifact-update paths) + 1 guard (A2AMessage path) - V2 handler: 1 guard (A2AMessage path; tuple path was already guarded) The fix returns None (skip event) which is consistent with the existing pattern in the v2 tuple branch and is properly handled by the caller in _run_async_impl via 'if not event: continue'. Fixes google/adk-python#5187 --- src/google/adk/agents/remote_a2a_agent.py | 10 + .../unittests/agents/test_remote_a2a_agent.py | 197 ++++++++++++++++++ 2 files changed, 207 insertions(+) diff --git a/src/google/adk/agents/remote_a2a_agent.py b/src/google/adk/agents/remote_a2a_agent.py index cbfbd61290..bf321c5f74 100644 --- a/src/google/adk/agents/remote_a2a_agent.py +++ b/src/google/adk/agents/remote_a2a_agent.py @@ -452,6 +452,8 @@ async def _handle_a2a_response( event = convert_a2a_task_to_event( task, self.name, ctx, self._a2a_part_converter ) + if not event: + return None # for streaming task, we update the event with the task status. # We update the event as Thought updates. if ( @@ -476,6 +478,8 @@ async def _handle_a2a_response( event = convert_a2a_message_to_event( update.status.message, self.name, ctx, self._a2a_part_converter ) + if not event: + return None if event.content is not None and update.status.state in ( TaskState.submitted, TaskState.working, @@ -495,6 +499,8 @@ async def _handle_a2a_response( event = convert_a2a_task_to_event( task, self.name, ctx, self._a2a_part_converter ) + if not event: + return None else: # This is a streaming update without a message (e.g. status change) # or a partial artifact update. We don't emit an event for these @@ -513,6 +519,8 @@ async def _handle_a2a_response( event = convert_a2a_message_to_event( a2a_response, self.name, ctx, self._a2a_part_converter ) + if not event: + return None event.custom_metadata = event.custom_metadata or {} if a2a_response.context_id: @@ -583,6 +591,8 @@ async def _handle_a2a_response_v2( event = self._config.a2a_message_converter( a2a_response, self.name, ctx, self._config.a2a_part_converter ) + if not event: + return None event.custom_metadata = event.custom_metadata or {} if a2a_response.context_id: diff --git a/tests/unittests/agents/test_remote_a2a_agent.py b/tests/unittests/agents/test_remote_a2a_agent.py index 4707f33799..6c1bd3eae6 100644 --- a/tests/unittests/agents/test_remote_a2a_agent.py +++ b/tests/unittests/agents/test_remote_a2a_agent.py @@ -1922,6 +1922,203 @@ async def test_handle_a2a_response_impl_handles_client_error(self): assert result.branch == self.mock_context.branch +class TestRemoteA2aAgentNoneConverterResults: + """Regression tests for None converter results in both legacy and v2 handlers. + + Converters can legitimately return None for messages/tasks with no convertible + parts, metadata-only events, or empty status updates. The handlers must not + crash with AttributeError when this happens. + """ + + def setup_method(self): + """Setup test fixtures.""" + from google.adk.a2a.agent.config import A2aRemoteAgentConfig + + self.agent_card = create_test_agent_card() + + # Legacy handler agent + self.mock_a2a_part_converter = Mock() + self.legacy_agent = RemoteA2aAgent( + name="test_agent", + agent_card=self.agent_card, + a2a_part_converter=self.mock_a2a_part_converter, + ) + + # V2 handler agent + self.mock_config = Mock(spec=A2aRemoteAgentConfig) + self.mock_config.a2a_part_converter = Mock() + self.mock_config.a2a_task_converter = Mock() + self.mock_config.a2a_status_update_converter = Mock() + self.mock_config.a2a_artifact_update_converter = Mock() + self.mock_config.a2a_message_converter = Mock() + self.mock_config.request_interceptors = None + self.v2_agent = RemoteA2aAgent( + name="test_agent", + agent_card=self.agent_card, + config=self.mock_config, + ) + + # Shared mock context + self.mock_session = Mock(spec=Session) + self.mock_session.id = "session-123" + self.mock_session.events = [] + + self.mock_context = Mock(spec=InvocationContext) + self.mock_context.session = self.mock_session + self.mock_context.invocation_id = "invocation-123" + self.mock_context.branch = "main" + + # --- V2 handler regression tests --- + + @pytest.mark.asyncio + async def test_v2_message_converter_returns_none(self): + """V2 handler must not crash when message converter returns None.""" + mock_msg = Mock(spec=A2AMessage) + mock_msg.metadata = {} + mock_msg.context_id = None + + self.mock_config.a2a_message_converter.return_value = None + + result = await self.v2_agent._handle_a2a_response_v2( + mock_msg, self.mock_context + ) + + assert result is None + self.mock_config.a2a_message_converter.assert_called_once() + + @pytest.mark.asyncio + async def test_v2_message_converter_returns_none_with_context_id(self): + """V2 handler returns None even when message has a context_id.""" + mock_msg = Mock(spec=A2AMessage) + mock_msg.metadata = {} + mock_msg.context_id = "ctx-should-not-be-accessed" + + self.mock_config.a2a_message_converter.return_value = None + + result = await self.v2_agent._handle_a2a_response_v2( + mock_msg, self.mock_context + ) + + assert result is None + + @pytest.mark.asyncio + async def test_v2_task_converter_returns_none(self): + """V2 handler must not crash when task converter returns None.""" + mock_task = Mock(spec=A2ATask) + mock_task.id = "task-123" + mock_task.context_id = "ctx-123" + + self.mock_config.a2a_task_converter.return_value = None + + result = await self.v2_agent._handle_a2a_response_v2( + (mock_task, None), self.mock_context + ) + + assert result is None + + @pytest.mark.asyncio + async def test_v2_status_update_converter_returns_none(self): + """V2 handler must not crash when status update converter returns None.""" + mock_task = Mock(spec=A2ATask) + mock_task.id = "task-123" + mock_task.context_id = None + + mock_update = Mock(spec=TaskStatusUpdateEvent) + + self.mock_config.a2a_status_update_converter.return_value = None + + result = await self.v2_agent._handle_a2a_response_v2( + (mock_task, mock_update), self.mock_context + ) + + assert result is None + + # --- Legacy handler regression tests --- + + @pytest.mark.asyncio + async def test_legacy_message_converter_returns_none(self): + """Legacy handler must not crash when message converter returns None.""" + mock_msg = Mock(spec=A2AMessage) + mock_msg.context_id = "context-123" + + with patch( + "google.adk.agents.remote_a2a_agent.convert_a2a_message_to_event" + ) as mock_convert: + mock_convert.return_value = None + + result = await self.legacy_agent._handle_a2a_response( + mock_msg, self.mock_context + ) + + assert result is None + mock_convert.assert_called_once() + + @pytest.mark.asyncio + async def test_legacy_task_converter_returns_none_no_update(self): + """Legacy handler must not crash when task converter returns None (no update).""" + mock_task = Mock(spec=A2ATask) + mock_task.id = "task-123" + mock_task.context_id = None + mock_task.status = Mock() + mock_task.status.state = TaskState.completed + + with patch( + "google.adk.agents.remote_a2a_agent.convert_a2a_task_to_event" + ) as mock_convert: + mock_convert.return_value = None + + result = await self.legacy_agent._handle_a2a_response( + (mock_task, None), self.mock_context + ) + + assert result is None + + @pytest.mark.asyncio + async def test_legacy_message_converter_returns_none_status_update(self): + """Legacy handler must not crash when message converter returns None for status update.""" + mock_task = Mock(spec=A2ATask) + mock_task.id = "task-123" + mock_task.context_id = "ctx-123" + + mock_update = Mock(spec=TaskStatusUpdateEvent) + mock_update.status = Mock() + mock_update.status.message = Mock() + mock_update.status.state = TaskState.working + + with patch( + "google.adk.agents.remote_a2a_agent.convert_a2a_message_to_event" + ) as mock_convert: + mock_convert.return_value = None + + result = await self.legacy_agent._handle_a2a_response( + (mock_task, mock_update), self.mock_context + ) + + assert result is None + + @pytest.mark.asyncio + async def test_legacy_task_converter_returns_none_artifact_update(self): + """Legacy handler must not crash when task converter returns None for artifact update.""" + mock_task = Mock(spec=A2ATask) + mock_task.id = "task-123" + mock_task.context_id = None + + mock_update = Mock(spec=TaskArtifactUpdateEvent) + mock_update.append = False + mock_update.last_chunk = True + + with patch( + "google.adk.agents.remote_a2a_agent.convert_a2a_task_to_event" + ) as mock_convert: + mock_convert.return_value = None + + result = await self.legacy_agent._handle_a2a_response( + (mock_task, mock_update), self.mock_context + ) + + assert result is None + + class TestRemoteA2aAgentExecution: """Test agent execution functionality."""