Bug: Overly complex exception handling in ServiceRetryWiring._apply_config_overrides can swallow errors #8116

New issue

Open

opened 2026-04-13 03:35:07 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-13 03:35:07 +00:00

Owner

Metadata

Commit message: fix: simplify exception handling in ServiceRetryWiring._apply_config_overrides
Branch name: fix/service-retry-wiring-exception-handling
Module: src/cleveragents/application/services/service_retry_wiring.py
Lines: 245-254
SHA: d5446081e3

Background and Context

The _apply_config_overrides method in ServiceRetryWiring is responsible for parsing per-service retry overrides from a JSON configuration string. It contains nested try...except blocks to handle potential parsing errors.

Current behavior

The exception handling logic at lines 245-254 is unnecessarily complex and potentially dangerous.

        except (json.JSONDecodeError, TypeError, RecursionError):
            try:
                # P2-17: Truncate raw JSON to prevent secret leakage.
                logger.warning(
                    "Invalid retry_service_overrides JSON — ignoring",
                    raw=raw[:80] + "..." if len(raw) > 80 else raw,
                )
            except TypeError:
                with contextlib.suppress(Exception):
                    logger.warning("Invalid retry_service_overrides JSON — ignoring")

The outer except catches TypeError, but the inner except also catches TypeError. The innermost with contextlib.suppress(Exception) is particularly problematic as it will swallow any exception that occurs during the logging call, not just a TypeError, potentially hiding bugs in the logging configuration or other unexpected issues.

Expected Behavior

The exception handling should be simplified to a single try...except block that logs the error without swallowing other potential exceptions. The logic can be streamlined to be more readable and maintainable.

Acceptance Criteria

The nested try...except blocks in _apply_config_overrides are refactored into a single, clearer except block.
The contextlib.suppress(Exception) is removed to avoid swallowing unrelated exceptions.
The code correctly handles and logs JSON parsing errors.

Subtasks

Refactor the exception handling in _apply_config_overrides.
Add unit tests to verify the new exception handling logic.

Definition of Done

The refactored code is merged into the master branch.
The associated unit tests are passing.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata - **Commit message:** `fix: simplify exception handling in ServiceRetryWiring._apply_config_overrides` - **Branch name:** `fix/service-retry-wiring-exception-handling` - **Module:** `src/cleveragents/application/services/service_retry_wiring.py` - **Lines:** 245-254 - **SHA:** d5446081e3b9f0fc7736afb21e0fd110ce7cae15 ## Background and Context The `_apply_config_overrides` method in `ServiceRetryWiring` is responsible for parsing per-service retry overrides from a JSON configuration string. It contains nested `try...except` blocks to handle potential parsing errors. ## Current behavior The exception handling logic at lines 245-254 is unnecessarily complex and potentially dangerous. ```python except (json.JSONDecodeError, TypeError, RecursionError): try: # P2-17: Truncate raw JSON to prevent secret leakage. logger.warning( "Invalid retry_service_overrides JSON — ignoring", raw=raw[:80] + "..." if len(raw) > 80 else raw, ) except TypeError: with contextlib.suppress(Exception): logger.warning("Invalid retry_service_overrides JSON — ignoring") ``` The outer `except` catches `TypeError`, but the inner `except` also catches `TypeError`. The innermost `with contextlib.suppress(Exception)` is particularly problematic as it will swallow *any* exception that occurs during the logging call, not just a `TypeError`, potentially hiding bugs in the logging configuration or other unexpected issues. ## Expected Behavior The exception handling should be simplified to a single `try...except` block that logs the error without swallowing other potential exceptions. The logic can be streamlined to be more readable and maintainable. ## Acceptance Criteria - The nested `try...except` blocks in `_apply_config_overrides` are refactored into a single, clearer `except` block. - The `contextlib.suppress(Exception)` is removed to avoid swallowing unrelated exceptions. - The code correctly handles and logs JSON parsing errors. ## Subtasks - [ ] Refactor the exception handling in `_apply_config_overrides`. - [ ] Add unit tests to verify the new exception handling logic. ## Definition of Done - The refactored code is merged into the `master` branch. - The associated unit tests are passing. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 added this to the v3.2.0 milestone

2026-04-13 03:35:12 +00:00

HAL9000 added the

labels

2026-04-13 03:35:58 +00:00

HAL9000 commented

2026-04-13 04:07:08 +00:00

Author

Owner

✅ Verified — Exception swallowing makes debugging significantly harder and can hide real bugs. Should Have fix for v3.2.0. Verified.

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

✅ **Verified** — Exception swallowing makes debugging significantly harder and can hide real bugs. **Should Have** fix for v3.2.0. Verified. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-13 04:17:08 +00:00

[AUTO-PROJ-OWN] Status: Project Owner Report (Cycle 1) #8181