fix(retry): raise a proper exception type (S5632) #71

Open

Claude wants to merge 1 commit from Claude/backup-script:fix/issue-30-S5632-retry-non-exception-raise into master

pull from: Claude/backup-script:fix/issue-30-S5632-retry-non-exception-raise

merge into: bc1bb:master

bc1bb:master

Conversation 0 Commits 1 Files changed 1 +8 -1

Claude commented 2026-05-18 01:31:59 +02:00

First-time contributor

Copy link

Closes #30

Problem

SonarQube rule S5632 flags raise last_exc at core/retry.py:127 as
raising a non-BaseException value. last_exc is typed
BaseException | None and is never reassigned anywhere in the current
implementation — at the only point it can be raised, it is always None,
which is not a valid raise target.

Fix

Replace raise last_exc with raise RuntimeError(...) from last_exc. The
explicit RuntimeError is guaranteed to be a real exception type, and the
from last_exc clause preserves any chained cause if a future patch ever
populates the variable. The if last_exc is not None: guard becomes
type-safe for the from clause (which requires BaseException | None).

Verification

• pytest -q tests/unit/ — all 211 tests pass, including
  tests/unit/test_retry_policy.py which covers the retry loop on both
  retried and non-retried exception types.
• mypy --strict src/gardien/core/ — clean (10 source files, no issues).

Note

Overlaps with #29 — the surrounding if last_exc is not None: identity
check is the S5727 companion bug. This PR only changes the raise target;
it deliberately leaves the identity check in place so that #29 can be
reviewed independently. Either PR landing first will leave the other
mergeable with a trivial conflict resolution.

Closes #30 ### Problem SonarQube rule **S5632** flags `raise last_exc` at `core/retry.py:127` as raising a non-`BaseException` value. `last_exc` is typed `BaseException | None` and is never reassigned anywhere in the current implementation — at the only point it can be raised, it is always `None`, which is not a valid raise target. ### Fix Replace `raise last_exc` with `raise RuntimeError(...) from last_exc`. The explicit `RuntimeError` is guaranteed to be a real exception type, and the `from last_exc` clause preserves any chained cause if a future patch ever populates the variable. The `if last_exc is not None:` guard becomes type-safe for the `from` clause (which requires `BaseException | None`). ### Verification - `pytest -q tests/unit/` — all 211 tests pass, including `tests/unit/test_retry_policy.py` which covers the retry loop on both retried and non-retried exception types. - `mypy --strict src/gardien/core/` — clean (10 source files, no issues). ### Note Overlaps with #29 — the surrounding `if last_exc is not None:` identity check is the S5727 companion bug. This PR only changes the raise target; it deliberately leaves the identity check in place so that #29 can be reviewed independently. Either PR landing first will leave the other mergeable with a trivial conflict resolution.

Claude added 1 commit 2026-05-18 01:31:59 +02:00

fix(retry): raise a proper exception type in run_with_retry (S5632) ... 2c289820ff

`raise last_exc` in run_with_retry's defensive fallback would have raised
`None` (last_exc is typed `BaseException | None` and is never reassigned in
the current implementation). Replace with an explicit RuntimeError chained
via `from last_exc` so the cause is preserved if a future patch populates
it.

Overlaps with #29 — the surrounding identity check is the dead-code
companion bug; this PR only changes the raise target.

Refs #30

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u fix/issue-30-S5632-retry-non-exception-raise:Claude-fix/issue-30-S5632-retry-non-exception-raise

git switch Claude-fix/issue-30-S5632-retry-non-exception-raise

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master

git merge --no-ff Claude-fix/issue-30-S5632-retry-non-exception-raise

git switch Claude-fix/issue-30-S5632-retry-non-exception-raise

git rebase master

git switch master

git merge --ff-only Claude-fix/issue-30-S5632-retry-non-exception-raise

git switch Claude-fix/issue-30-S5632-retry-non-exception-raise

git rebase master

git switch master

git merge --no-ff Claude-fix/issue-30-S5632-retry-non-exception-raise

git switch master

git merge --squash Claude-fix/issue-30-S5632-retry-non-exception-raise

git switch master

git merge --ff-only Claude-fix/issue-30-S5632-retry-non-exception-raise

git switch master

git merge Claude-fix/issue-30-S5632-retry-non-exception-raise

git push origin master

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

bc1bb/backup-script!71

No description provided.