fix: guard db_game against NameError when db_post fails in complete_game (#27) #86

Merged
Claude merged 2 commits from ai/paper-dynasty-discord27 into main 2026-03-23 03:58:34 +00:00
Collaborator

Summary

Fixes #27 — if db_post("games", ...) raised before db_game was assigned, the except block's await roll_back(db_game["id"]) would raise a NameError, masking the original exception and preventing rollback.

Changes (command_logic/logic_gameplay.py):

  • Add db_game = None before the try block (~line 4298)
  • Guard the roll_back call with if db_game is not None: in the except block

Before

try:
    db_game = await db_post("games", payload=game_data)
    ...
except Exception as e:
    await roll_back(db_game["id"])  # NameError if db_post raised!
    log_exception(e, msg="Unable to post game to API, rolling back")

After

db_game = None
try:
    db_game = await db_post("games", payload=game_data)
    ...
except Exception as e:
    if db_game is not None:
        await roll_back(db_game["id"])
    log_exception(e, msg="Unable to post game to API, rolling back")

Other observations

  • The second except block at ~line 4311 (await roll_back(db_game["id"], plays=True)) is fine — it only runs if the first try block succeeded, so db_game is guaranteed to be set.
  • After the first except, execution falls through to the db_post("plays", ...) block where db_ready_plays would be undefined (pre-existing issue, out of scope for this fix).

Test results

No test suite — changes verified by reading back the modified code.


🤖 Generated with Claude Code

## Summary Fixes #27 — if `db_post("games", ...)` raised before `db_game` was assigned, the `except` block's `await roll_back(db_game["id"])` would raise a `NameError`, masking the original exception and preventing rollback. **Changes** (`command_logic/logic_gameplay.py`): - Add `db_game = None` before the try block (~line 4298) - Guard the `roll_back` call with `if db_game is not None:` in the except block ## Before ```python try: db_game = await db_post("games", payload=game_data) ... except Exception as e: await roll_back(db_game["id"]) # NameError if db_post raised! log_exception(e, msg="Unable to post game to API, rolling back") ``` ## After ```python db_game = None try: db_game = await db_post("games", payload=game_data) ... except Exception as e: if db_game is not None: await roll_back(db_game["id"]) log_exception(e, msg="Unable to post game to API, rolling back") ``` ## Other observations - The second except block at ~line 4311 (`await roll_back(db_game["id"], plays=True)`) is fine — it only runs if the first try block succeeded, so `db_game` is guaranteed to be set. - After the first except, execution falls through to the `db_post("plays", ...)` block where `db_ready_plays` would be undefined (pre-existing issue, out of scope for this fix). ## Test results No test suite — changes verified by reading back the modified code. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Claude added 1 commit 2026-03-10 15:33:06 +00:00
fix: guard db_game against NameError when db_post fails in complete_game (#27)
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m24s
678fa320df
Initialize db_game = None before try block and guard roll_back call
with `if db_game is not None:` to prevent NameError masking the
original exception when db_post("games") raises before assignment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
label 2026-03-10 15:45:47 +00:00
Claude reviewed 2026-03-10 15:46:49 +00:00
Claude left a comment
Author
Collaborator

AI Code Review

Files Reviewed

  • command_logic/logic_gameplay.py (modified)

Findings

Correctness

The fix is correct and well-scoped. db_game = None is placed immediately before the try block, and the if db_game is not None: guard prevents the NameError that would fire if db_post("games", ...) raised before the assignment completed.

One nuance worth confirming: the PR body notes that the second roll_back call at ~line 4312 (await roll_back(db_game["id"], plays=True)) is safe because it only runs if the first try block succeeded. That is correct — if the first try raises, execution jumps to the first except block and the second try block is not reached.

However, the db_game subscript at ~line 4358 (get_game_summary_embed(..., db_game["id"], ...)) and the subsequent roll_back calls at lines 4321, 4347 will raise TypeError: 'NoneType' object is not subscriptable if the first try block fails and db_game remains None. The PR body correctly identifies this as a pre-existing, out-of-scope issue — it is not introduced here. The fix prevents the immediate NameError masking the original exception, which is the stated goal.

Security

No issues found. No user inputs, no new external calls, no credentials touched.

Style & Conventions

The change follows the same pattern established by if db_game is not None: guards elsewhere in this codebase (e.g. PR #59 applied the same idiom). Two lines added, two lines modified — minimal and targeted.

Suggestions

The fall-through behavior (execution continuing past the first except block when db_game is None) means that db_ready_plays and db_ready_decisions will be undefined when db_post("plays", ...) is reached. An early return after log_exception(e, ...) in the first except block would prevent a cascade of downstream errors. This is pre-existing and not in scope here, but worth a follow-up issue if one does not already exist.

Verdict: COMMENT

The fix is correct and directly addresses the stated bug. Gitea blocks self-review approval, so posting as COMMENT. This is a clean, safe change that can be merged.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `command_logic/logic_gameplay.py` (modified) ### Findings #### Correctness The fix is correct and well-scoped. `db_game = None` is placed immediately before the try block, and the `if db_game is not None:` guard prevents the `NameError` that would fire if `db_post("games", ...)` raised before the assignment completed. One nuance worth confirming: the PR body notes that the second `roll_back` call at ~line 4312 (`await roll_back(db_game["id"], plays=True)`) is safe because it only runs if the first try block succeeded. That is correct — if the first try raises, execution jumps to the first except block and the second try block is not reached. However, the `db_game` subscript at ~line 4358 (`get_game_summary_embed(..., db_game["id"], ...)`) and the subsequent `roll_back` calls at lines 4321, 4347 will raise `TypeError: 'NoneType' object is not subscriptable` if the first try block fails and `db_game` remains `None`. The PR body correctly identifies this as a pre-existing, out-of-scope issue — it is not introduced here. The fix prevents the immediate `NameError` masking the original exception, which is the stated goal. #### Security No issues found. No user inputs, no new external calls, no credentials touched. #### Style & Conventions The change follows the same pattern established by `if db_game is not None:` guards elsewhere in this codebase (e.g. PR #59 applied the same idiom). Two lines added, two lines modified — minimal and targeted. #### Suggestions The fall-through behavior (execution continuing past the first except block when `db_game` is `None`) means that `db_ready_plays` and `db_ready_decisions` will be undefined when `db_post("plays", ...)` is reached. An early `return` after `log_exception(e, ...)` in the first except block would prevent a cascade of downstream errors. This is pre-existing and not in scope here, but worth a follow-up issue if one does not already exist. ### Verdict: COMMENT The fix is correct and directly addresses the stated bug. Gitea blocks self-review approval, so posting as COMMENT. This is a clean, safe change that can be merged. --- *Automated review by Claude PR Reviewer*
cal approved these changes 2026-03-23 03:53:36 +00:00
cal left a comment
Owner

Approved — safe fix per PO triage.

Approved — safe fix per PO triage.
Claude added 1 commit 2026-03-23 03:53:42 +00:00
Merge branch 'main' into ai/paper-dynasty-discord27
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m59s
829e03e3de
Claude merged commit 376e0b8a31 into main 2026-03-23 03:58:34 +00:00
Claude deleted branch ai/paper-dynasty-discord27 2026-03-23 03:58:35 +00:00
Sign in to join this conversation.
No reviewers
cal
No Milestone
No project
No Assignees
2 Participants
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: cal/paper-dynasty-discord#86
No description provided.