diff --git a/features/actor_compute_impact_error_handling.feature b/features/actor_compute_impact_error_handling.feature new file mode 100644 index 000000000..b8fe97078 --- /dev/null +++ b/features/actor_compute_impact_error_handling.feature @@ -0,0 +1,49 @@ +Feature: _compute_actor_impact logs exceptions instead of silently swallowing them + As an operator debugging actor removal issues + I want exceptions in _compute_actor_impact to be logged at WARNING level + So that I can diagnose database or service failures without silent data loss + + Background: + Given an actor CLI runner + + @mock_only + Scenario: Session query failure is logged at WARNING level and returns zero count + Given the session service raises an exception when listing sessions + When I call _compute_actor_impact for actor "local/my-actor" + Then the session count should be 0 + And a WARNING log message should contain "Failed to query session count" + And a WARNING log message should contain "local/my-actor" + + @mock_only + Scenario: Active plan query failure is logged at WARNING level and returns zero count + Given the unit of work raises an exception when listing plans + When I call _compute_actor_impact for actor "local/my-actor" + Then the active plan count should be 0 + And a WARNING log message should contain "Failed to query active plan count" + And a WARNING log message should contain "local/my-actor" + + @mock_only + Scenario: Action query failure is logged at WARNING level and returns zero count + Given the plan lifecycle service raises an exception when listing actions + When I call _compute_actor_impact for actor "local/my-actor" + Then the action count should be 0 + And a WARNING log message should contain "Failed to query action count" + And a WARNING log message should contain "local/my-actor" + + @mock_only + Scenario: All three queries succeed and return correct counts + Given the session service returns 2 sessions for actor "local/my-actor" + And the unit of work returns 1 active plan for actor "local/my-actor" + And the plan lifecycle service returns 3 actions for actor "local/my-actor" + When I call _compute_actor_impact for actor "local/my-actor" + Then the session count should be 2 + And the active plan count should be 1 + And the action count should be 3 + And no WARNING log messages should be emitted + + @mock_only + Scenario: Exception log message includes exception type and message for diagnostics + Given the session service raises a RuntimeError with message "DB connection refused" + When I call _compute_actor_impact for actor "local/my-actor" + Then a WARNING log message should contain "RuntimeError" + And a WARNING log message should contain "DB connection refused" diff --git a/features/steps/actor_compute_impact_error_handling_steps.py b/features/steps/actor_compute_impact_error_handling_steps.py new file mode 100644 index 000000000..05ca2e4f6 --- /dev/null +++ b/features/steps/actor_compute_impact_error_handling_steps.py @@ -0,0 +1,211 @@ +# pyrightconfig: reportRedeclaration=false +"""Step definitions for _compute_actor_impact error handling feature.""" + +from __future__ import annotations + +import logging +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when + + +@given("the session service raises an exception when listing sessions") +def step_session_service_raises(context: Any) -> None: + """Configure session service to raise an exception.""" + context.session_raises = RuntimeError("DB unavailable") + context.plan_raises = None + context.action_raises = None + context.session_count_override = None + context.plan_count_override = None + context.action_count_override = None + + +@given("the unit of work raises an exception when listing plans") +def step_uow_raises(context: Any) -> None: + """Configure unit of work to raise an exception.""" + context.session_raises = None + context.plan_raises = RuntimeError("DB unavailable") + context.action_raises = None + context.session_count_override = 0 + context.plan_count_override = None + context.action_count_override = None + + +@given("the plan lifecycle service raises an exception when listing actions") +def step_lifecycle_service_raises(context: Any) -> None: + """Configure plan lifecycle service to raise an exception.""" + context.session_raises = None + context.plan_raises = None + context.action_raises = RuntimeError("DB unavailable") + context.session_count_override = 0 + context.plan_count_override = 0 + context.action_count_override = None + + +@given('the session service returns {count:d} sessions for actor "{actor_name}"') +def step_session_service_returns(context: Any, count: int, actor_name: str) -> None: + """Configure session service to return a specific count.""" + context.session_raises = None + context.plan_raises = None + context.action_raises = None + context.session_count_override = count + context.plan_count_override = None + context.action_count_override = None + + +@given('the unit of work returns {count:d} active plan for actor "{actor_name}"') +def step_uow_returns_plan(context: Any, count: int, actor_name: str) -> None: + """Configure unit of work to return a specific active plan count.""" + context.plan_count_override = count + + +@given('the plan lifecycle service returns {count:d} actions for actor "{actor_name}"') +def step_lifecycle_returns_actions(context: Any, count: int, actor_name: str) -> None: + """Configure plan lifecycle service to return a specific action count.""" + context.action_count_override = count + + +@given('the session service raises a RuntimeError with message "{message}"') +def step_session_raises_with_message(context: Any, message: str) -> None: + """Configure session service to raise a RuntimeError with a specific message.""" + context.session_raises = RuntimeError(message) + context.plan_raises = None + context.action_raises = None + context.session_count_override = None + context.plan_count_override = None + context.action_count_override = None + + +@when('I call _compute_actor_impact for actor "{actor_name}"') +def step_call_compute_actor_impact(context: Any, actor_name: str) -> None: + """Call _compute_actor_impact with mocked services and capture log output.""" + from cleveragents.cli.commands.actor import _compute_actor_impact + + session_raises = getattr(context, "session_raises", None) + plan_raises = getattr(context, "plan_raises", None) + action_raises = getattr(context, "action_raises", None) + session_count_override = getattr(context, "session_count_override", 0) + plan_count_override = getattr(context, "plan_count_override", 0) + action_count_override = getattr(context, "action_count_override", 0) + + # Build mock container + mock_container = MagicMock() + + # Configure session service + if session_raises is not None: + mock_container.session_service.return_value.list.side_effect = session_raises + else: + count = session_count_override or 0 + mock_sessions = [MagicMock(actor_name=actor_name) for _ in range(count)] + mock_container.session_service.return_value.list.return_value = mock_sessions + + # Configure unit of work / plans + if plan_raises is not None: + mock_uow = MagicMock() + mock_uow.transaction.side_effect = plan_raises + mock_container.unit_of_work.return_value = mock_uow + else: + count = plan_count_override or 0 + mock_plans = [] + for _ in range(count): + p = MagicMock() + p.processing_state = "queued" + p.strategy_actor = actor_name + p.execution_actor = None + mock_plans.append(p) + mock_ctx = MagicMock() + mock_ctx.lifecycle_plans.list_plans.return_value = mock_plans + mock_uow = MagicMock() + mock_uow.transaction.return_value.__enter__ = MagicMock(return_value=mock_ctx) + mock_uow.transaction.return_value.__exit__ = MagicMock(return_value=False) + mock_container.unit_of_work.return_value = mock_uow + + # Configure plan lifecycle service / actions + if action_raises is not None: + mock_container.plan_lifecycle_service.return_value.list_actions.side_effect = ( + action_raises + ) + else: + count = action_count_override or 0 + mock_actions = [] + for _ in range(count): + a = MagicMock() + a.strategy_actor = actor_name + a.execution_actor = None + mock_actions.append(a) + mock_container.plan_lifecycle_service.return_value.list_actions.return_value = ( + mock_actions + ) + + # Capture log records + log_records: list[logging.LogRecord] = [] + + class CapturingHandler(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + log_records.append(record) + + handler = CapturingHandler() + actor_logger = logging.getLogger("cleveragents.cli.commands.actor") + actor_logger.addHandler(handler) + original_level = actor_logger.level + actor_logger.setLevel(logging.WARNING) + + try: + with patch( + "cleveragents.cli.commands.actor.get_container", + return_value=mock_container, + ): + context.impact_result = _compute_actor_impact(actor_name) + finally: + actor_logger.removeHandler(handler) + actor_logger.setLevel(original_level) + + context.log_records = log_records + + +@then("the session count should be {count:d}") +def step_session_count(context: Any, count: int) -> None: + """Assert the session count in the result.""" + assert context.impact_result is not None, "impact_result was not set" + actual = context.impact_result[0] + assert actual == count, f"Expected session count {count}, got {actual}" + + +@then("the active plan count should be {count:d}") +def step_active_plan_count(context: Any, count: int) -> None: + """Assert the active plan count in the result.""" + assert context.impact_result is not None, "impact_result was not set" + actual = context.impact_result[1] + assert actual == count, f"Expected active plan count {count}, got {actual}" + + +@then("the action count should be {count:d}") +def step_action_count(context: Any, count: int) -> None: + """Assert the action count in the result.""" + assert context.impact_result is not None, "impact_result was not set" + actual = context.impact_result[2] + assert actual == count, f"Expected action count {count}, got {actual}" + + +@then('a WARNING log message should contain "{text}"') +def step_warning_log_contains(context: Any, text: str) -> None: + """Assert that at least one WARNING log message contains the given text.""" + warning_messages = [ + r.getMessage() for r in context.log_records if r.levelno >= logging.WARNING + ] + assert any(text in msg for msg in warning_messages), ( + f"Expected a WARNING log message containing {text!r}, " + f"but got: {warning_messages}" + ) + + +@then("no WARNING log messages should be emitted") +def step_no_warning_logs(context: Any) -> None: + """Assert that no WARNING log messages were emitted.""" + warning_messages = [ + r.getMessage() for r in context.log_records if r.levelno >= logging.WARNING + ] + assert not warning_messages, ( + f"Expected no WARNING log messages, but got: {warning_messages}" + ) diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 1fe10c78e..0c491af0d 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import json +import logging from pathlib import Path from typing import Annotated, Any, cast @@ -48,6 +49,7 @@ app = typer.Typer( ) ) console = Console() +_logger = logging.getLogger(__name__) # Reusable --format option description _FORMAT_HELP = "Output format: json, yaml, plain, table, or rich (default: rich)" @@ -246,9 +248,14 @@ def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: RuntimeError, CleverAgentsError, OperationalError, - ValidationError, # pragma: no cover - defensive; DB may be unavailable - ): - pass + ValidationError, + ) as exc: + _logger.warning( + "Failed to query session count for actor %r: %s: %s", + actor_name, + type(exc).__name__, + exc, + ) # --- Active lifecycle plans referencing this actor --- active_plan_count = 0 @@ -268,9 +275,14 @@ def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: RuntimeError, CleverAgentsError, OperationalError, - ValidationError, # pragma: no cover - defensive; DB may be unavailable - ): - pass + ValidationError, + ) as exc: + _logger.warning( + "Failed to query active plan count for actor %r: %s: %s", + actor_name, + type(exc).__name__, + exc, + ) # --- Actions configured to use this actor --- action_count = 0 @@ -295,9 +307,14 @@ def _compute_actor_impact(actor_name: str) -> tuple[int, int, int]: RuntimeError, CleverAgentsError, OperationalError, - ValidationError, # pragma: no cover - defensive; DB may be unavailable - ): - pass + ValidationError, + ) as exc: + _logger.warning( + "Failed to query action count for actor %r: %s: %s", + actor_name, + type(exc).__name__, + exc, + ) return session_count, active_plan_count, action_count