UAT: `ActorRegistry.add()` catches all exceptions in duplicate-check guard instead of `NotFoundError` only — silently swallows unexpected errors #3738

New issue

Open

opened 2026-04-05 22:22:52 +00:00 by freemo · 0 comments

freemo commented

2026-04-05 22:22:52 +00:00

Owner

Metadata

Branch: fix/actor-registry-add-specific-exception-catch
Commit Message: fix(actor): catch NotFoundError specifically in ActorRegistry.add() duplicate-check guard
Milestone: None (Backlog)
Parent Epic: #230

Background and Context

CONTRIBUTING.md states:

"Only catch exceptions when they can be meaningfully handled (e.g., for retry logic or resource cleanup). Do not catch exceptions just to log and re-raise."
"Follow fail-fast principles, using assertions and avoiding silent failures."

The ActorRegistry.add() method in src/cleveragents/actor/registry.py uses a broad except Exception clause in its duplicate-check guard. This silently swallows any unexpected exceptions (database errors, network errors, etc.) that occur during the get_actor() call.

Current Behavior

ActorRegistry.add() (approximately lines 155–175 of registry.py):

def add(self, yaml_text: str, *, update: bool = False, ...) -> Actor:
    ...
    # Check for existing actor when not updating
    if not update:
        try:
            self._actor_service.get_actor(name)
            raise ValidationError(
                f"Actor '{name}' already exists. Pass update=True to overwrite."
            )
        except Exception as exc:
            if "already exists" in str(exc):
                raise
            # NotFoundError is expected for new actors

Bug: The except Exception clause catches ALL exceptions. If get_actor() raises a database error, a connection error, or any other unexpected exception, the code silently swallows it (since "already exists" won't be in the error message) and proceeds as if the actor doesn't exist. This can lead to:

Duplicate actors being created when the database is temporarily unavailable
Silent data corruption
Unexpected behavior that is very difficult to debug

Expected Behavior

The code should specifically catch NotFoundError (from cleveragents.core.exceptions), which is the only expected exception when an actor does not exist. All other exceptions should propagate:

from cleveragents.core.exceptions import NotFoundError, ValidationError

if not update:
    try:
        self._actor_service.get_actor(name)
        raise ValidationError(
            f"Actor '{name}' already exists. Pass update=True to overwrite."
        )
    except NotFoundError:
        pass  # Expected: actor doesn't exist yet, proceed with creation

Impact

Database errors, connection errors, and other unexpected exceptions during the duplicate check are silently swallowed.
The actor creation proceeds even when the system is in an error state, potentially creating duplicate or corrupted data.
Violates CONTRIBUTING.md's fail-fast principle and prohibition on broad exception suppression.
Makes debugging very difficult — unexpected errors disappear without a trace.

Code Location

src/cleveragents/actor/registry.py — add() method, duplicate-check guard (approximately lines 155–175)

Steps to Reproduce

Mock ActorService.get_actor() to raise a RuntimeError (simulating a database failure).
Call ActorRegistry.add(yaml_text) with update=False.
Expected: RuntimeError propagates out of add().
Actual: RuntimeError is silently swallowed and actor creation proceeds.

Subtasks

Write a TDD failing Behave scenario (@tdd_expected_fail) asserting that unexpected exceptions from get_actor() propagate out of add()
Replace except Exception as exc: with except NotFoundError: in the duplicate-check guard
Remove the if "already exists" in str(exc): raise string-matching logic (fragile and unnecessary with specific exception catching)
Remove @tdd_expected_fail tag once fix is in place
Verify nox -e typecheck passes
Verify nox -e unit_tests passes with coverage >= 97%
Run nox (all default sessions), fix any errors

Definition of Done

ActorRegistry.add() catches only NotFoundError in the duplicate-check guard
All other exceptions from get_actor() propagate to the caller
No string-matching on exception messages ("already exists" in str(exc))
Behave BDD scenarios cover both the duplicate-actor case and the unexpected-exception case
All nox stages pass
Coverage >= 97%

Backlog note: This issue was discovered during autonomous UAT testing of the Actor System feature area. It does not block milestone completion and has been placed in the backlog for human review and future milestone assignment.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/actor-registry-add-specific-exception-catch` - **Commit Message**: `fix(actor): catch NotFoundError specifically in ActorRegistry.add() duplicate-check guard` - **Milestone**: None (Backlog) - **Parent Epic**: #230 ## Background and Context `CONTRIBUTING.md` states: > "Only catch exceptions when they can be meaningfully handled (e.g., for retry logic or resource cleanup). Do not catch exceptions just to log and re-raise." > "Follow fail-fast principles, using assertions and avoiding silent failures." The `ActorRegistry.add()` method in `src/cleveragents/actor/registry.py` uses a broad `except Exception` clause in its duplicate-check guard. This silently swallows any unexpected exceptions (database errors, network errors, etc.) that occur during the `get_actor()` call. ## Current Behavior `ActorRegistry.add()` (approximately lines 155–175 of `registry.py`): ```python def add(self, yaml_text: str, *, update: bool = False, ...) -> Actor: ... # Check for existing actor when not updating if not update: try: self._actor_service.get_actor(name) raise ValidationError( f"Actor '{name}' already exists. Pass update=True to overwrite." ) except Exception as exc: if "already exists" in str(exc): raise # NotFoundError is expected for new actors ``` **Bug**: The `except Exception` clause catches ALL exceptions. If `get_actor()` raises a database error, a connection error, or any other unexpected exception, the code silently swallows it (since `"already exists"` won't be in the error message) and proceeds as if the actor doesn't exist. This can lead to: 1. Duplicate actors being created when the database is temporarily unavailable 2. Silent data corruption 3. Unexpected behavior that is very difficult to debug ## Expected Behavior The code should specifically catch `NotFoundError` (from `cleveragents.core.exceptions`), which is the only expected exception when an actor does not exist. All other exceptions should propagate: ```python from cleveragents.core.exceptions import NotFoundError, ValidationError if not update: try: self._actor_service.get_actor(name) raise ValidationError( f"Actor '{name}' already exists. Pass update=True to overwrite." ) except NotFoundError: pass # Expected: actor doesn't exist yet, proceed with creation ``` ## Impact - Database errors, connection errors, and other unexpected exceptions during the duplicate check are silently swallowed. - The actor creation proceeds even when the system is in an error state, potentially creating duplicate or corrupted data. - Violates `CONTRIBUTING.md`'s fail-fast principle and prohibition on broad exception suppression. - Makes debugging very difficult — unexpected errors disappear without a trace. ## Code Location `src/cleveragents/actor/registry.py` — `add()` method, duplicate-check guard (approximately lines 155–175) ## Steps to Reproduce 1. Mock `ActorService.get_actor()` to raise a `RuntimeError` (simulating a database failure). 2. Call `ActorRegistry.add(yaml_text)` with `update=False`. 3. **Expected**: `RuntimeError` propagates out of `add()`. 4. **Actual**: `RuntimeError` is silently swallowed and actor creation proceeds. ## Subtasks - [ ] Write a TDD failing Behave scenario (`@tdd_expected_fail`) asserting that unexpected exceptions from `get_actor()` propagate out of `add()` - [ ] Replace `except Exception as exc:` with `except NotFoundError:` in the duplicate-check guard - [ ] Remove the `if "already exists" in str(exc): raise` string-matching logic (fragile and unnecessary with specific exception catching) - [ ] Remove `@tdd_expected_fail` tag once fix is in place - [ ] Verify `nox -e typecheck` passes - [ ] Verify `nox -e unit_tests` passes with coverage >= 97% - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done - [ ] `ActorRegistry.add()` catches only `NotFoundError` in the duplicate-check guard - [ ] All other exceptions from `get_actor()` propagate to the caller - [ ] No string-matching on exception messages (`"already exists" in str(exc)`) - [ ] Behave BDD scenarios cover both the duplicate-actor case and the unexpected-exception case - [ ] All nox stages pass - [ ] Coverage >= 97% > **Backlog note:** This issue was discovered during autonomous UAT testing of the Actor System feature area. It does not block milestone completion and has been placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added the

labels

2026-04-05 22:22:58 +00:00

freemo added a new dependency

2026-04-05 22:23:04 +00:00

#230 Stage A1: Plan Data Model