cleveragents/cleveragents-core

Fork 3

UAT: SkillService silently swallows ALL database exceptions — violates spec error-handling contract #3722

New issue

Open

opened 2026-04-05 22:18:32 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 22:18:32 +00:00

Owner

Metadata

Branch: fix/skill-service-db-exception-propagation
Commit Message: fix(skill-service): propagate database exceptions instead of swallowing them
Milestone: v3.6.0
Parent Epic: #3374

Background and Context

The SkillService in src/cleveragents/application/services/skill_service.py silently swallows all exceptions in four database-related methods, violating the project's error-handling specification.

Per CONTRIBUTING.md and docs/specification.md, exceptions must not be suppressed — errors must propagate to the top-level execution where they can be properly logged and handled. Silent failures (like returning null on error or continuing after a failed write) are explicitly prohibited in favour of raising exceptions.

Current Behavior

Four methods catch bare Exception and only log a warning, silently continuing:

_load_from_db() (line ~100):

except Exception:
    logger.warning("Failed to load skills from database", exc_info=True)

_persist_skill() (line ~130):

except Exception:
    logger.warning("Failed to persist skill %s to database", skill.name, exc_info=True)

_delete_skill_from_db() (line ~150):

except Exception:
    logger.warning("Failed to delete skill %s from database", name, exc_info=True)

_commit() (line ~165):

except Exception:
    logger.warning("Failed to commit session", exc_info=True)

Expected Behavior

Per the project error-handling contract:

"Exceptions should only be caught when they can be handled meaningfully (e.g., for retry logic or resource cleanup). Catching exceptions only to log them is not allowed."

All four methods must propagate database exceptions to callers rather than swallowing them.

Impact

Database write failures are silently ignored — skills appear registered in memory but are not persisted
Database read failures on startup silently produce an empty skill registry
Commit failures are silently ignored — data loss goes undetected
Violates the project's error-handling contract

Steps to Reproduce

Configure SkillService with a SkillRepository that raises on create()
Call add_skill() with a valid config
Observe: no exception is raised, but the skill is not persisted to DB

Code Location

src/cleveragents/application/services/skill_service.py — methods _load_from_db, _persist_skill, _delete_skill_from_db, _commit

Subtasks

Replace bare except Exception in _load_from_db() with meaningful error propagation
Replace bare except Exception in _persist_skill() with meaningful error propagation
Replace bare except Exception in _delete_skill_from_db() with meaningful error propagation
Replace bare except Exception in _commit() with meaningful error propagation
Add Behave tests verifying DB exceptions propagate correctly

Definition of Done

This issue is complete when:

All four methods propagate DB exceptions instead of swallowing them
Unit tests verify exception propagation
nox -e typecheck passes
nox -e unit_tests passes
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
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/skill-service-db-exception-propagation` - **Commit Message**: `fix(skill-service): propagate database exceptions instead of swallowing them` - **Milestone**: v3.6.0 - **Parent Epic**: #3374 ## Background and Context The `SkillService` in `src/cleveragents/application/services/skill_service.py` silently swallows all exceptions in four database-related methods, violating the project's error-handling specification. Per `CONTRIBUTING.md` and `docs/specification.md`, exceptions must not be suppressed — errors must propagate to the top-level execution where they can be properly logged and handled. Silent failures (like returning null on error or continuing after a failed write) are explicitly prohibited in favour of raising exceptions. ## Current Behavior Four methods catch bare `Exception` and only log a warning, silently continuing: **`_load_from_db()` (line ~100)**: ```python except Exception: logger.warning("Failed to load skills from database", exc_info=True) ``` **`_persist_skill()` (line ~130)**: ```python except Exception: logger.warning("Failed to persist skill %s to database", skill.name, exc_info=True) ``` **`_delete_skill_from_db()` (line ~150)**: ```python except Exception: logger.warning("Failed to delete skill %s from database", name, exc_info=True) ``` **`_commit()` (line ~165)**: ```python except Exception: logger.warning("Failed to commit session", exc_info=True) ``` ## Expected Behavior Per the project error-handling contract: > "Exceptions should only be caught when they can be handled meaningfully (e.g., for retry logic or resource cleanup). Catching exceptions only to log them is not allowed." All four methods must propagate database exceptions to callers rather than swallowing them. ## Impact - Database write failures are silently ignored — skills appear registered in memory but are not persisted - Database read failures on startup silently produce an empty skill registry - Commit failures are silently ignored — data loss goes undetected - Violates the project's error-handling contract ## Steps to Reproduce 1. Configure `SkillService` with a `SkillRepository` that raises on `create()` 2. Call `add_skill()` with a valid config 3. Observe: no exception is raised, but the skill is not persisted to DB ## Code Location `src/cleveragents/application/services/skill_service.py` — methods `_load_from_db`, `_persist_skill`, `_delete_skill_from_db`, `_commit` ## Subtasks - [ ] Replace bare `except Exception` in `_load_from_db()` with meaningful error propagation - [ ] Replace bare `except Exception` in `_persist_skill()` with meaningful error propagation - [ ] Replace bare `except Exception` in `_delete_skill_from_db()` with meaningful error propagation - [ ] Replace bare `except Exception` in `_commit()` with meaningful error propagation - [ ] Add Behave tests verifying DB exceptions propagate correctly ## Definition of Done This issue is complete when: - [ ] All four methods propagate DB exceptions instead of swallowing them - [ ] Unit tests verify exception propagation - [ ] `nox -e typecheck` passes - [ ] `nox -e unit_tests` passes - [ ] 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 - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added this to the v3.6.0 milestone

2026-04-05 22:18:36 +00:00

freemo added a new dependency

2026-04-05 22:18:52 +00:00

#3374 Epic: E2E Workflow Specification Tests & Code Review Tool Examples

freemo referenced this issue

2026-04-05 22:20:24 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:20:38 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:22:14 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:22:42 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:25:59 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:33:37 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo commented

2026-04-06 18:59:01 +00:00

Author

Owner

Milestone Triage Decision: Moved to Backlog

This error handling improvement has been moved out of v3.6.0 during aggressive milestone triage. Exception handling improvements are maintenance tasks, not advanced concepts.

Reasoning:

v3.6.0 focus: Advanced concepts that extend beyond core MVP
This issue: SkillService exception swallowing - error handling improvement
Impact: Code quality improvement, not advanced conceptual capability

Should be addressed in a dedicated maintenance/quality improvement effort alongside similar error handling issues.

**Milestone Triage Decision: Moved to Backlog** This error handling improvement has been moved out of v3.6.0 during aggressive milestone triage. Exception handling improvements are maintenance tasks, not advanced concepts. **Reasoning:** - v3.6.0 focus: Advanced concepts that extend beyond core MVP - This issue: SkillService exception swallowing - error handling improvement - Impact: Code quality improvement, not advanced conceptual capability Should be addressed in a dedicated maintenance/quality improvement effort alongside similar error handling issues.

freemo removed this from the v3.6.0 milestone

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