BUG-HUNT: [error-handling] ActorRegistry.add() swallows all non-"already exists" exceptions from get_actor() — DB/network errors silently ignored #6390

New issue

Open

opened 2026-04-09 21:00:06 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-09 21:00:06 +00:00

Owner

Bug Report: [error-handling] Overly broad exception handler in ActorRegistry.add() masks real errors

Severity Assessment

Impact: Database errors, SQLAlchemy OperationalError, connection failures, and other unexpected exceptions raised by get_actor() are silently swallowed. The method then continues as if no actor exists and proceeds to upsert, potentially creating a new actor when the operation should have failed. Callers have no way to detect that an error occurred during the existence check.
Likelihood: Low in normal operation but triggered whenever the database is unavailable or returns an unexpected error during a new-actor add.
Priority: Medium

Location

File: src/cleveragents/actor/registry.py
Function: ActorRegistry.add
Lines: 226–235

Description

The add() method in ActorRegistry checks whether an actor already exists by calling self._actor_service.get_actor(name) inside a bare except Exception handler. Only exceptions whose string representation contains "already exists" are re-raised. All other exceptions — including database errors, OperationalError, NotFoundError subclasses without that string, connection errors, and unexpected service failures — are silently swallowed by the # NotFoundError is expected for new actors comment branch.

This means:

If get_actor() raises a real error (e.g., the database is locked or corrupted), add() silently continues to upsert_actor(), which will likely raise its own error — but the root cause has been lost.
Any custom NotFoundError class that does NOT include "already exists" in its message is correctly handled (falls through), but any other exception is also silently discarded.

Evidence

# registry.py, lines 226-235
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
        # ↑ But also: DB errors, OperationalError, ANY other exception
        # is silently swallowed here! ← BUG

Consider what happens when get_actor() raises sqlalchemy.exc.OperationalError("database is locked"):

"already exists" is NOT in str(exc), so the check fails.
The except block exits silently.
Execution continues to self._actor_service.upsert_actor(...) at line 241.
That call may also fail (with a different error), or worse, succeed — creating the actor in a partially-consistent state.

Expected Behavior

Only NotFoundError (the specific "actor not found" exception from the actor service) should be caught and swallowed. All other exceptions should propagate to the caller. The pattern should be:

from cleveragents.core.exceptions import NotFoundError

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 does not exist yet, continue with add

Actual Behavior

Any exception from get_actor() is swallowed unless its message contains "already exists". The add() method then proceeds with the upsert, potentially masking the root cause of a failure.

Suggested Fix

Import and catch the specific NotFoundError (or equivalent) from cleveragents.core.exceptions, replacing the broad except Exception:

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  # Actor does not exist — safe to create
    # All other exceptions propagate naturally

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: [error-handling] Overly broad exception handler in ActorRegistry.add() masks real errors ### Severity Assessment - **Impact**: Database errors, SQLAlchemy `OperationalError`, connection failures, and other unexpected exceptions raised by `get_actor()` are silently swallowed. The method then continues as if no actor exists and proceeds to upsert, potentially creating a new actor when the operation should have failed. Callers have no way to detect that an error occurred during the existence check. - **Likelihood**: Low in normal operation but triggered whenever the database is unavailable or returns an unexpected error during a new-actor add. - **Priority**: Medium ### Location - **File**: `src/cleveragents/actor/registry.py` - **Function**: `ActorRegistry.add` - **Lines**: 226–235 ### Description The `add()` method in `ActorRegistry` checks whether an actor already exists by calling `self._actor_service.get_actor(name)` inside a bare `except Exception` handler. Only exceptions whose string representation contains `"already exists"` are re-raised. All other exceptions — including database errors, `OperationalError`, `NotFoundError` subclasses without that string, connection errors, and unexpected service failures — are silently swallowed by the `# NotFoundError is expected for new actors` comment branch. This means: 1. If `get_actor()` raises a real error (e.g., the database is locked or corrupted), `add()` silently continues to `upsert_actor()`, which will likely raise its own error — but the root cause has been lost. 2. Any custom `NotFoundError` class that does NOT include `"already exists"` in its message is correctly handled (falls through), but any other exception is also silently discarded. ### Evidence ```python # registry.py, lines 226-235 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 # ↑ But also: DB errors, OperationalError, ANY other exception # is silently swallowed here! ← BUG ``` Consider what happens when `get_actor()` raises `sqlalchemy.exc.OperationalError("database is locked")`: - `"already exists"` is NOT in `str(exc)`, so the check fails. - The `except` block exits silently. - Execution continues to `self._actor_service.upsert_actor(...)` at line 241. - That call may also fail (with a different error), or worse, succeed — creating the actor in a partially-consistent state. ### Expected Behavior Only `NotFoundError` (the specific "actor not found" exception from the actor service) should be caught and swallowed. All other exceptions should propagate to the caller. The pattern should be: ```python from cleveragents.core.exceptions import NotFoundError 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 does not exist yet, continue with add ``` ### Actual Behavior Any exception from `get_actor()` is swallowed unless its message contains `"already exists"`. The `add()` method then proceeds with the upsert, potentially masking the root cause of a failure. ### Suggested Fix Import and catch the specific `NotFoundError` (or equivalent) from `cleveragents.core.exceptions`, replacing the broad `except Exception`: ```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 # Actor does not exist — safe to create # All other exceptions propagate naturally ``` ### Category error-handling ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added the

labels

2026-04-09 21:07:30 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 21:09:21 +00:00

HAL9000 referenced this issue