a2a: Broad exception in _cleanup_session_devcontainers may hide specific errors #9054

Open
opened 2026-04-14 06:45:10 +00:00 by HAL9000 · 2 comments
Owner

Metadata

  • Commit Message: fix(a2a): narrow exception handling in _cleanup_session_devcontainers to expose specific errors
  • Branch: fix/a2a-cleanup-session-devcontainers-broad-exception

Background and Context

The _cleanup_session_devcontainers method in src/cleveragents/a2a/facade.py (class A2aLocalFacade) uses a broad except Exception: block to swallow all errors during devcontainer cleanup on session close. While the intent is to ensure session close always succeeds (best-effort cleanup), the catch-all handler makes it impossible to distinguish between different error conditions.

For example, a temporary network issue when communicating with the devcontainer service could be retried if a more specific exception was caught. A FileNotFoundError (e.g., missing socket) is fundamentally different from a TimeoutError or a PermissionError, yet all are silently swallowed and logged at the same warning level with no differentiation.

This violates the project's exception handling guidelines, which state: "Do not use bare catch-all exception handlers without re-raising unless you have specific recovery logic."

Code location: cleveragents.a2a.facade.A2aLocalFacade._cleanup_session_devcontainers (commit a71c14285493af4b16a4af97c479534e03f8755a)

Expected Behavior

When this issue is complete:

  • The _cleanup_session_devcontainers method catches only the specific exception types that are expected and recoverable from CleanupService.stop_active_devcontainers.
  • Unexpected exceptions are either re-raised (allowing them to propagate) or handled with targeted recovery logic (e.g., retry on TimeoutError).
  • The warning log includes the specific exception type to aid debugging.
  • Session close still succeeds for expected/recoverable errors, maintaining the best-effort cleanup contract.

Acceptance Criteria

  • The except Exception: block in _cleanup_session_devcontainers is replaced with one or more specific exception types (e.g., OSError, TimeoutError, RuntimeError) that are documented and justified.
  • Any exception type not in the explicit catch list propagates naturally (or is re-raised after logging).
  • The warning log message includes the specific exception class name for easier triage.
  • All existing BDD scenarios for _cleanup_session_devcontainers continue to pass.
  • New BDD scenarios cover: (a) a specific expected exception is caught and logged, (b) an unexpected exception propagates.
  • Test coverage for cleveragents.a2a.facade remains ≥ 97%.
  • nox (all default sessions) passes with no new errors.

Subtasks

  • Audit CleanupService.stop_active_devcontainers to identify all exception types it can raise.
  • Replace except Exception: with the specific exception types identified in the audit.
  • Add re-raise (or targeted recovery logic) for exception types not in the explicit catch list.
  • Update the warning log to include the exception class name.
  • Tests (Behave): Add scenarios for specific-exception-caught and unexpected-exception-propagates paths.
  • Verify coverage ≥ 97% via nox -s coverage_report.
  • Run nox (all default sessions), fix any errors.

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **Commit Message**: `fix(a2a): narrow exception handling in _cleanup_session_devcontainers to expose specific errors` - **Branch**: `fix/a2a-cleanup-session-devcontainers-broad-exception` ## Background and Context The `_cleanup_session_devcontainers` method in `src/cleveragents/a2a/facade.py` (class `A2aLocalFacade`) uses a broad `except Exception:` block to swallow all errors during devcontainer cleanup on session close. While the intent is to ensure session close always succeeds (best-effort cleanup), the catch-all handler makes it impossible to distinguish between different error conditions. For example, a temporary network issue when communicating with the devcontainer service could be retried if a more specific exception was caught. A `FileNotFoundError` (e.g., missing socket) is fundamentally different from a `TimeoutError` or a `PermissionError`, yet all are silently swallowed and logged at the same `warning` level with no differentiation. This violates the project's exception handling guidelines, which state: *"Do not use bare catch-all exception handlers without re-raising unless you have specific recovery logic."* **Code location**: `cleveragents.a2a.facade.A2aLocalFacade._cleanup_session_devcontainers` (commit `a71c14285493af4b16a4af97c479534e03f8755a`) ## Expected Behavior When this issue is complete: - The `_cleanup_session_devcontainers` method catches only the specific exception types that are expected and recoverable from `CleanupService.stop_active_devcontainers`. - Unexpected exceptions are either re-raised (allowing them to propagate) or handled with targeted recovery logic (e.g., retry on `TimeoutError`). - The warning log includes the specific exception type to aid debugging. - Session close still succeeds for expected/recoverable errors, maintaining the best-effort cleanup contract. ## Acceptance Criteria - [ ] The `except Exception:` block in `_cleanup_session_devcontainers` is replaced with one or more specific exception types (e.g., `OSError`, `TimeoutError`, `RuntimeError`) that are documented and justified. - [ ] Any exception type not in the explicit catch list propagates naturally (or is re-raised after logging). - [ ] The warning log message includes the specific exception class name for easier triage. - [ ] All existing BDD scenarios for `_cleanup_session_devcontainers` continue to pass. - [ ] New BDD scenarios cover: (a) a specific expected exception is caught and logged, (b) an unexpected exception propagates. - [ ] Test coverage for `cleveragents.a2a.facade` remains ≥ 97%. - [ ] `nox` (all default sessions) passes with no new errors. ## Subtasks - [ ] Audit `CleanupService.stop_active_devcontainers` to identify all exception types it can raise. - [ ] Replace `except Exception:` with the specific exception types identified in the audit. - [ ] Add re-raise (or targeted recovery logic) for exception types not in the explicit catch list. - [ ] Update the warning log to include the exception class name. - [ ] Tests (Behave): Add scenarios for specific-exception-caught and unexpected-exception-propagates paths. - [ ] Verify coverage ≥ 97% via `nox -s coverage_report`. - [ ] Run `nox` (all default sessions), fix any errors. ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. --- **Automated by CleverAgents Bot** Agent: new-issue-creator
Author
Owner

🔍 Triage Decision — [AUTO-OWNR-2]

Status: VERIFIED

MoSCoW: Should have
Priority: Medium
Milestone: v3.5.0

Reasoning: A broad except Exception in _cleanup_session_devcontainers violates CONTRIBUTING.md exception handling guidelines and may hide specific errors during cleanup. Should be narrowed to specific exception types.


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

## 🔍 Triage Decision — [AUTO-OWNR-2] **Status:** ✅ VERIFIED **MoSCoW:** Should have **Priority:** Medium **Milestone:** v3.5.0 **Reasoning:** A broad `except Exception` in `_cleanup_session_devcontainers` violates CONTRIBUTING.md exception handling guidelines and may hide specific errors during cleanup. Should be narrowed to specific exception types. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
HAL9000 added this to the v3.5.0 milestone 2026-04-14 17:39:06 +00:00
Author
Owner

Triage Decision [AUTO-OWNR-1]: Verified as a valid error handling concern. Broad exception handling in _cleanup_session_devcontainers can hide specific errors and make debugging difficult. Should Have fix for v3.5.0.


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

✅ **Triage Decision [AUTO-OWNR-1]**: Verified as a valid error handling concern. Broad exception handling in `_cleanup_session_devcontainers` can hide specific errors and make debugging difficult. `Should Have` fix for v3.5.0. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#9054
No description provided.