Broad Exception Handling in `async_cleanup.py` #8242

New issue

Open

opened 2026-04-13 06:01:43 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-13 06:01:43 +00:00

Owner

Metadata

File: src/cleveragents/core/async_cleanup.py
Line: 122
Commit message: fix(core): separate CancelledError handling in async_cleanup close_all
Branch name: fix/core-async-cleanup-cancelled-error-handling

Background and Context

In src/cleveragents/core/async_cleanup.py, the close_all method uses a broad except (Exception, asyncio.CancelledError) block. While this ensures that the loop continues even if one resource fails to close, it conflates two fundamentally different situations:

Real errors — a resource failing to close due to an unexpected exception.
Cancellation — a cooperative signal (asyncio.CancelledError) that the coroutine should stop, which is not an error condition.

Logging a CancelledError at the same level as a genuine exception produces noisy logs and can obscure real issues. Additionally, in Python 3.8+, asyncio.CancelledError is a subclass of BaseException (not Exception), so catching it inside a broad except Exception block is already semantically incorrect — it should be caught explicitly and handled differently.

Impacted component: core

Expected Behavior

asyncio.CancelledError raised during resource cleanup is caught in its own except asyncio.CancelledError block.
Cancellation is either re-raised (to propagate cooperative cancellation correctly) or logged at DEBUG/WARNING level rather than ERROR.
Genuine exceptions continue to be caught, logged at ERROR level, and allow the cleanup loop to continue.
Logs are clean and actionable — real errors are not buried under cancellation noise.

Acceptance Criteria

close_all in async_cleanup.py has a dedicated except asyncio.CancelledError block separate from the general except Exception block.
asyncio.CancelledError is handled gracefully (re-raised or logged at a non-error level) and does not appear in logs as an exception.
Genuine exceptions during resource cleanup are still caught, logged at ERROR level, and do not abort the cleanup loop.
Existing unit tests for close_all continue to pass.
New or updated tests cover the CancelledError path explicitly.
No regressions in nox / CI pipeline.

Subtasks

Review close_all at line 122 in src/cleveragents/core/async_cleanup.py and understand the full exception-handling flow.
Add a dedicated except asyncio.CancelledError block; decide whether to re-raise or log at DEBUG/WARNING.
Ensure the general except Exception block remains to keep the cleanup loop alive on real errors.
Update or add unit tests to cover the CancelledError branch.
Run nox locally to confirm all tests pass and coverage is maintained.
Update inline comments/docstrings to explain the rationale for the split exception handling.

Definition of Done

This issue should be closed when:

The close_all method separates asyncio.CancelledError from general Exception handling.
CancelledError no longer appears in logs as an error-level event.
All existing tests pass and the new CancelledError path is covered by tests.
The change is merged to the main branch via a reviewed PR.

Reproduction Steps: N/A — identified via static code review.

Proposed Fix: Add a separate except asyncio.CancelledError: block before the general except Exception: block. Handle cancellation gracefully — either re-raise it (preferred for cooperative cancellation) or log it at DEBUG level. Example:

try:
    await resource.close()
except asyncio.CancelledError:
    # Cancellation is not an error; re-raise to propagate cooperative cancellation
    raise
except Exception:
    logger.exception("Failed to close resource %r", resource)

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata - **File:** `src/cleveragents/core/async_cleanup.py` - **Line:** 122 - **Commit message:** `fix(core): separate CancelledError handling in async_cleanup close_all` - **Branch name:** `fix/core-async-cleanup-cancelled-error-handling` ## Background and Context In `src/cleveragents/core/async_cleanup.py`, the `close_all` method uses a broad `except (Exception, asyncio.CancelledError)` block. While this ensures that the loop continues even if one resource fails to close, it conflates two fundamentally different situations: 1. **Real errors** — a resource failing to close due to an unexpected exception. 2. **Cancellation** — a cooperative signal (`asyncio.CancelledError`) that the coroutine should stop, which is not an error condition. Logging a `CancelledError` at the same level as a genuine exception produces noisy logs and can obscure real issues. Additionally, in Python 3.8+, `asyncio.CancelledError` is a subclass of `BaseException` (not `Exception`), so catching it inside a broad `except Exception` block is already semantically incorrect — it should be caught explicitly and handled differently. **Impacted component:** `core` ## Expected Behavior - `asyncio.CancelledError` raised during resource cleanup is caught in its own `except asyncio.CancelledError` block. - Cancellation is either re-raised (to propagate cooperative cancellation correctly) or logged at `DEBUG`/`WARNING` level rather than `ERROR`. - Genuine exceptions continue to be caught, logged at `ERROR` level, and allow the cleanup loop to continue. - Logs are clean and actionable — real errors are not buried under cancellation noise. ## Acceptance Criteria - [ ] `close_all` in `async_cleanup.py` has a dedicated `except asyncio.CancelledError` block separate from the general `except Exception` block. - [ ] `asyncio.CancelledError` is handled gracefully (re-raised or logged at a non-error level) and does not appear in logs as an exception. - [ ] Genuine exceptions during resource cleanup are still caught, logged at `ERROR` level, and do not abort the cleanup loop. - [ ] Existing unit tests for `close_all` continue to pass. - [ ] New or updated tests cover the `CancelledError` path explicitly. - [ ] No regressions in `nox` / CI pipeline. ## Subtasks - [ ] Review `close_all` at line 122 in `src/cleveragents/core/async_cleanup.py` and understand the full exception-handling flow. - [ ] Add a dedicated `except asyncio.CancelledError` block; decide whether to re-raise or log at `DEBUG`/`WARNING`. - [ ] Ensure the general `except Exception` block remains to keep the cleanup loop alive on real errors. - [ ] Update or add unit tests to cover the `CancelledError` branch. - [ ] Run `nox` locally to confirm all tests pass and coverage is maintained. - [ ] Update inline comments/docstrings to explain the rationale for the split exception handling. ## Definition of Done This issue should be closed when: 1. The `close_all` method separates `asyncio.CancelledError` from general `Exception` handling. 2. `CancelledError` no longer appears in logs as an error-level event. 3. All existing tests pass and the new `CancelledError` path is covered by tests. 4. The change is merged to the main branch via a reviewed PR. **Reproduction Steps:** N/A — identified via static code review. **Proposed Fix:** Add a separate `except asyncio.CancelledError:` block before the general `except Exception:` block. Handle cancellation gracefully — either re-raise it (preferred for cooperative cancellation) or log it at `DEBUG` level. Example: ```python try: await resource.close() except asyncio.CancelledError: # Cancellation is not an error; re-raise to propagate cooperative cancellation raise except Exception: logger.exception("Failed to close resource %r", resource) ``` --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added this to the v3.2.0 milestone

2026-04-13 06:03:33 +00:00

HAL9000 commented

2026-04-13 06:18:47 +00:00

Author

Owner

[AUTO-EPIC] Epic Linkage

This issue is a child of Epic #8043 — M3 UAT Bug Resolution — Spec Alignment Backlog (v3.2.0).

Broad exception handling in async_cleanup.py is a code quality/correctness issue that falls under the M3 UAT bug resolution scope. It should be resolved before v3.2.0 can be accepted.

Dependency direction: This issue (#8242) BLOCKS Epic #8043. Epic #8043 DEPENDS ON this issue being resolved.

Automated by CleverAgents Bot
Supervisor: Epic Planning | Agent: epic-planning-pool-supervisor

## [AUTO-EPIC] Epic Linkage This issue is a child of **Epic #8043** — M3 UAT Bug Resolution — Spec Alignment Backlog (v3.2.0). Broad exception handling in `async_cleanup.py` is a code quality/correctness issue that falls under the M3 UAT bug resolution scope. It should be resolved before v3.2.0 can be accepted. **Dependency direction**: This issue (#8242) BLOCKS Epic #8043. Epic #8043 DEPENDS ON this issue being resolved. --- **Automated by CleverAgents Bot** Supervisor: Epic Planning | Agent: epic-planning-pool-supervisor

HAL9000 referenced this issue

2026-04-13 06:22:22 +00:00

[AUTO-EPIC] Status: Epic Planning Supervisor — Cycle 7 #8250

HAL9000 referenced this issue

2026-04-13 08:54:14 +00:00

[AUTO-BUG-POOL] Status: Bug Hunt Pool Status Update (Cycle 1) #8327