BUG-HUNT: [error-handling] Broad exception in execute #1876

New issue

Open

opened 2026-04-03 00:03:55 +00:00 by freemo · 1 comment

freemo commented

2026-04-03 00:03:55 +00:00

Owner

Metadata

Branch: fix/error-handling-tool-runner-execute
Commit Message: fix(tool): replace broad Exception catch in ToolRunner.execute with specific exception types and structured logging
Milestone: v3.2.0
Parent Epic: #1669

Bug Report: [error-handling] — Broad exception in execute

Severity Assessment

Impact: Potential to swallow and hide bugs, making debugging difficult.
Likelihood: Low, but possible if other unexpected errors occur.
Priority: Low

Location

File: src/cleveragents/tool/runner.py
Function/Class: execute
Lines: 294-305

Description

The execute method in ToolRunner uses a broad except Exception as exc: block. This can catch and obscure unexpected errors, making it difficult to debug issues that are not ToolError exceptions.

Evidence

        except Exception as exc:
            # Intentionally broad: the runner's contract is to
            # normalise *any* handler failure into a ToolResult
            # with ``success=False`` so callers never see raw
            # exceptions from container execution.
            return ToolResult(
                success=False,
                output={},
                error=f"Container execution error: {type(exc).__name__}: {exc}",
                duration_ms=0.0,
            )

Expected Behavior

The exception handling should be more specific, catching only the expected exceptions. If the intention is to catch all exceptions, it should be logged and re-raised or handled in a way that doesn't hide the original error.

Suggested Fix

Catch more specific exceptions if possible. If not, log the exception and re-raise it or handle it in a way that preserves the original stack trace.

Subtasks

Write a TDD issue-capture Behave test that proves the bug exists (unexpected exceptions are silently normalised into ToolResult(success=False) without any log trace)
Add logger.exception (or exc_info=True) call inside the broad except block so the full stack trace is always captured in structured logs
Identify the concrete exception types that execute legitimately expects (e.g. ToolError, TimeoutError, container-specific errors) and split the handler accordingly
Replace the single broad except Exception with targeted except clauses for each expected type, re-raising anything truly unexpected
Define a domain-specific ContainerExecutionError (or equivalent) in the tool module if one does not already exist, and raise it instead of returning a silent failure
Update all callers of execute to handle the new exception type appropriately
Ensure all new code is fully statically typed and passes nox -e typecheck
Verify nox -e unit_tests passes with the new Behave scenario
Verify nox -e coverage_report reports coverage >= 97%

Definition of Done

A Behave scenario exists that captures the bug (fails before fix, passes after)
ToolRunner.execute no longer silently normalises unexpected exceptions without logging
Full exception traceback is logged via structured logger on any unexpected failure
Only expected, well-understood exception types are caught without re-raising
Truly unexpected exceptions propagate (or are wrapped in a domain-specific exception) so callers are aware
No # type: ignore suppressions introduced
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/error-handling-tool-runner-execute` - **Commit Message**: `fix(tool): replace broad Exception catch in ToolRunner.execute with specific exception types and structured logging` - **Milestone**: v3.2.0 - **Parent Epic**: #1669 ## Bug Report: [error-handling] — Broad exception in execute ### Severity Assessment - **Impact**: Potential to swallow and hide bugs, making debugging difficult. - **Likelihood**: Low, but possible if other unexpected errors occur. - **Priority**: Low ### Location - **File**: `src/cleveragents/tool/runner.py` - **Function/Class**: `execute` - **Lines**: `294-305` ### Description The `execute` method in `ToolRunner` uses a broad `except Exception as exc:` block. This can catch and obscure unexpected errors, making it difficult to debug issues that are not `ToolError` exceptions. ### Evidence ```python except Exception as exc: # Intentionally broad: the runner's contract is to # normalise *any* handler failure into a ToolResult # with ``success=False`` so callers never see raw # exceptions from container execution. return ToolResult( success=False, output={}, error=f"Container execution error: {type(exc).__name__}: {exc}", duration_ms=0.0, ) ``` ### Expected Behavior The exception handling should be more specific, catching only the expected exceptions. If the intention is to catch all exceptions, it should be logged and re-raised or handled in a way that doesn't hide the original error. ### Suggested Fix Catch more specific exceptions if possible. If not, log the exception and re-raise it or handle it in a way that preserves the original stack trace. ### Category error-handling ## Subtasks - [ ] Write a TDD issue-capture Behave test that proves the bug exists (unexpected exceptions are silently normalised into `ToolResult(success=False)` without any log trace) - [ ] Add `logger.exception` (or `exc_info=True`) call inside the broad `except` block so the full stack trace is always captured in structured logs - [ ] Identify the concrete exception types that `execute` legitimately expects (e.g. `ToolError`, `TimeoutError`, container-specific errors) and split the handler accordingly - [ ] Replace the single broad `except Exception` with targeted `except` clauses for each expected type, re-raising anything truly unexpected - [ ] Define a domain-specific `ContainerExecutionError` (or equivalent) in the tool module if one does not already exist, and raise it instead of returning a silent failure - [ ] Update all callers of `execute` to handle the new exception type appropriately - [ ] Ensure all new code is fully statically typed and passes `nox -e typecheck` - [ ] Verify `nox -e unit_tests` passes with the new Behave scenario - [ ] Verify `nox -e coverage_report` reports coverage >= 97% ## Definition of Done - [ ] A Behave scenario exists that captures the bug (fails before fix, passes after) - [ ] `ToolRunner.execute` no longer silently normalises unexpected exceptions without logging - [ ] Full exception traceback is logged via structured logger on any unexpected failure - [ ] Only expected, well-understood exception types are caught without re-raising - [ ] Truly unexpected exceptions propagate (or are wrapped in a domain-specific exception) so callers are aware - [ ] No `# type: ignore` suppressions introduced - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.2.0 milestone

2026-04-03 00:04:31 +00:00

freemo added a new dependency

2026-04-03 00:09:29 +00:00

#1669 Bug Hunting Session

freemo commented

2026-04-03 00:13:01 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
MoSCoW: MoSCoW/Should Have — bug or error handling improvement.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: MoSCoW/Should Have — bug or error handling improvement. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner