UAT: Silent exception suppression in _notify_facade() violates exception propagation rules #3864

New issue

Open

opened 2026-04-06 07:01:41 +00:00 by freemo · 0 comments

freemo commented

2026-04-06 07:01:41 +00:00

Owner

Metadata

Branch: fix/notify-facade-silent-exception-suppression
Commit Message: fix(plan): replace bare except pass in _notify_facade() with structured debug logging
Milestone: (backlog — see note below)
Parent Epic: #362

Bug Report

Feature Area: Error Handling and Resilience
Severity: Medium
Found by: UAT automated testing

What Was Tested

The _notify_facade() function in src/cleveragents/cli/commands/plan.py (lines 75-101) was analyzed for compliance with the CONTRIBUTING.md exception propagation rules.

Expected Behavior (from CONTRIBUTING.md)

Errors must never be suppressed. Exceptions should be allowed to propagate up to the top-level execution handler.
Catch-all exception handlers (catch (Exception e)) should not be used unless they re-raise the exception.
Exceptions should only be caught when there is a meaningful way to handle them.

Actual Behavior

_notify_facade() uses a bare except Exception: pass that silently swallows all exceptions without logging, re-raising, or any meaningful handling:

# src/cleveragents/cli/commands/plan.py lines 87-101
def _notify_facade(operation: str, params: dict[str, Any]) -> None:
    """Best-effort A2A facade notification for protocol bookkeeping.
    ...
    """
    try:
        ...
        facade = get_facade()
        request = A2aRequest(method=operation, params=params)
        facade.dispatch(request)
    except Exception:
        pass  # <-- VIOLATION: bare pass with no logging or re-raise

Why This Is a Problem

No logging: When the facade fails, there is zero diagnostic information. If the A2A protocol is broken, developers have no way to detect it.
Violates CONTRIBUTING.md: The rule states exceptions should only be caught when there is a meaningful recovery action. A bare pass is not a meaningful recovery.
Masks real errors: If get_facade() raises due to a configuration error or facade.dispatch() raises due to a protocol violation, these are silently discarded.

Recommended Fix

Replace the bare pass with at minimum a debug-level log:

    except Exception:
        logger.debug(
            "A2A facade notification failed (best-effort, non-fatal)",
            operation=operation,
            exc_info=True,
        )

This preserves the fire-and-forget semantics while providing diagnostic information and complying with the "catch with purpose" rule.

Code Location

File: src/cleveragents/cli/commands/plan.py
Lines: 87-101
Function: _notify_facade()

Subtasks

Locate _notify_facade() in src/cleveragents/cli/commands/plan.py and confirm the bare except Exception: pass pattern
Replace the bare pass with a logger.debug(...) call that logs the exception with exc_info=True, operation, and a descriptive message
Verify the logger instance is available in the module (add structlog.get_logger() if missing)
Write a Behave unit test scenario in features/ that asserts the debug log is emitted when facade.dispatch() raises
Run nox -e lint and nox -e typecheck to confirm no regressions
Run nox -e unit_tests and nox -e coverage_report to confirm coverage ≥ 97%

Definition of Done

_notify_facade() no longer contains a bare except Exception: pass
All caught exceptions in _notify_facade() are logged at DEBUG level with full exc_info
A Behave scenario covers the exception-logging path
All nox stages pass
Coverage >= 97%

Backlog note: This issue was discovered during autonomous operation
on milestone v3.3.0. 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/notify-facade-silent-exception-suppression` - **Commit Message**: `fix(plan): replace bare except pass in _notify_facade() with structured debug logging` - **Milestone**: *(backlog — see note below)* - **Parent Epic**: #362 ## Bug Report **Feature Area:** Error Handling and Resilience **Severity:** Medium **Found by:** UAT automated testing ### What Was Tested The `_notify_facade()` function in `src/cleveragents/cli/commands/plan.py` (lines 75-101) was analyzed for compliance with the CONTRIBUTING.md exception propagation rules. ### Expected Behavior (from CONTRIBUTING.md) > Errors must **never** be suppressed. Exceptions should be allowed to propagate up to the top-level execution handler. > Catch-all exception handlers (`catch (Exception e)`) should not be used unless they re-raise the exception. > Exceptions should only be caught when there is a meaningful way to handle them. ### Actual Behavior `_notify_facade()` uses a bare `except Exception: pass` that silently swallows all exceptions without logging, re-raising, or any meaningful handling: ```python # src/cleveragents/cli/commands/plan.py lines 87-101 def _notify_facade(operation: str, params: dict[str, Any]) -> None: """Best-effort A2A facade notification for protocol bookkeeping. ... """ try: ... facade = get_facade() request = A2aRequest(method=operation, params=params) facade.dispatch(request) except Exception: pass # <-- VIOLATION: bare pass with no logging or re-raise ``` ### Why This Is a Problem 1. **No logging**: When the facade fails, there is zero diagnostic information. If the A2A protocol is broken, developers have no way to detect it. 2. **Violates CONTRIBUTING.md**: The rule states exceptions should only be caught when there is a meaningful recovery action. A bare `pass` is not a meaningful recovery. 3. **Masks real errors**: If `get_facade()` raises due to a configuration error or `facade.dispatch()` raises due to a protocol violation, these are silently discarded. ### Recommended Fix Replace the bare `pass` with at minimum a debug-level log: ```python except Exception: logger.debug( "A2A facade notification failed (best-effort, non-fatal)", operation=operation, exc_info=True, ) ``` This preserves the fire-and-forget semantics while providing diagnostic information and complying with the "catch with purpose" rule. ### Code Location - File: `src/cleveragents/cli/commands/plan.py` - Lines: 87-101 - Function: `_notify_facade()` ## Subtasks - [ ] Locate `_notify_facade()` in `src/cleveragents/cli/commands/plan.py` and confirm the bare `except Exception: pass` pattern - [ ] Replace the bare `pass` with a `logger.debug(...)` call that logs the exception with `exc_info=True`, `operation`, and a descriptive message - [ ] Verify the logger instance is available in the module (add `structlog.get_logger()` if missing) - [ ] Write a Behave unit test scenario in `features/` that asserts the debug log is emitted when `facade.dispatch()` raises - [ ] Run `nox -e lint` and `nox -e typecheck` to confirm no regressions - [ ] Run `nox -e unit_tests` and `nox -e coverage_report` to confirm coverage ≥ 97% ## Definition of Done - [ ] `_notify_facade()` no longer contains a bare `except Exception: pass` - [ ] All caught exceptions in `_notify_facade()` are logged at `DEBUG` level with full `exc_info` - [ ] A Behave scenario covers the exception-logging path - [ ] All nox stages pass - [ ] Coverage >= 97% > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.3.0. 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 a new dependency

2026-04-06 07:02:01 +00:00

#362 Epic: Security & Safety Hardening

freemo added the

Points

label

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

freemo referenced this issue

2026-04-06 07:04:45 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 07:10:37 +00:00