bc1bb/backup-script
1
0
Fork 1
Code Issues 43 Pull requests 41 Projects Releases Packages Wiki Activity Actions

fix(executor): narrow final-drain except to Exception (S5754) #75

Open
Claude wants to merge 1 commit from Claude/backup-script:fix/issue-23-S5754-executor-bare-except into master
pull from: Claude/backup-script:fix/issue-23-S5754-executor-bare-except
merge into: bc1bb:master
Conversation 0 Commits 1 Files changed 1 +7 -2
Claude commented 2026-05-18 01:34:00 +02:00
First-time contributor
Copy link

Closes #23

Problem

SonarQube rule S5754 flags the except BaseException as e: at
core/executor.py:75 (inside the finally of Executor.run()) as
swallowing every exception class — including SystemExit and
KeyboardInterrupt — without reraising. That defeats the SignalManager
escalation path: if a second SIGTERM landed during signal_mgr.drain(),
the resulting SystemExit would be caught here, logged as
executor.final_drain_failed, and then the run would carry on as if
nothing had happened.

Every other except in this file (_run_step_safe, _rollback_prior_steps,
_ensure_destination_healthy) already narrows to Exception for exactly
this reason, with an inline comment.

Fix

Narrow the offending except from BaseException to Exception, matching
the convention used three other places in the same module. Add the same
rationale comment so the next reader sees the consistent pattern.
Behaviour for ordinary drain bugs is unchanged: log a warning, record
finished_at, return the report. Behaviour for signal-driven aborts is
restored: the SystemExit propagates out of run() and reaches the
signal-handler path.

Verification

  • pytest -q tests/unit/ — all 211 tests pass (no test asserted the
    swallow-everything behaviour because none should have).
  • mypy --strict src/gardien/core/ — clean (10 source files, no issues).
Closes #23 ### Problem SonarQube rule **S5754** flags the `except BaseException as e:` at `core/executor.py:75` (inside the `finally` of `Executor.run()`) as swallowing every exception class — including `SystemExit` and `KeyboardInterrupt` — without reraising. That defeats the SignalManager escalation path: if a second SIGTERM landed during `signal_mgr.drain()`, the resulting `SystemExit` would be caught here, logged as `executor.final_drain_failed`, and then the run would carry on as if nothing had happened. Every other `except` in this file (`_run_step_safe`, `_rollback_prior_steps`, `_ensure_destination_healthy`) already narrows to `Exception` for exactly this reason, with an inline comment. ### Fix Narrow the offending `except` from `BaseException` to `Exception`, matching the convention used three other places in the same module. Add the same rationale comment so the next reader sees the consistent pattern. Behaviour for ordinary drain bugs is unchanged: log a warning, record `finished_at`, return the report. Behaviour for signal-driven aborts is *restored*: the SystemExit propagates out of `run()` and reaches the signal-handler path. ### Verification - `pytest -q tests/unit/` — all 211 tests pass (no test asserted the swallow-everything behaviour because none should have). - `mypy --strict src/gardien/core/` — clean (10 source files, no issues). 
The `finally` block in `Executor.run()` caught `BaseException` and swallowed
it, which would prevent SystemExit / KeyboardInterrupt from propagating to
the signal-handler path — the opposite of what the rest of the executor
does (`_run_step_safe`, `_rollback_prior_steps`, `_ensure_destination_healthy`
all narrow to `Exception` with that exact rationale documented inline).

Narrow this `except` to `Exception` for consistency, so signals still
terminate the run mid-finally while genuine drain bugs are logged and
swallowed.

Refs #23
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-23-S5754-executor-bare-except:Claude-fix/issue-23-S5754-executor-bare-except
git switch Claude-fix/issue-23-S5754-executor-bare-except

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-23-S5754-executor-bare-except
git switch Claude-fix/issue-23-S5754-executor-bare-except
git rebase master
git switch master
git merge --ff-only Claude-fix/issue-23-S5754-executor-bare-except
git switch Claude-fix/issue-23-S5754-executor-bare-except
git rebase master
git switch master
git merge --no-ff Claude-fix/issue-23-S5754-executor-bare-except
git switch master
git merge --squash Claude-fix/issue-23-S5754-executor-bare-except
git switch master
git merge --ff-only Claude-fix/issue-23-S5754-executor-bare-except
git switch master
git merge Claude-fix/issue-23-S5754-executor-bare-except
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
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!75
No description provided.