fix: replace 467 manual db.close() calls with middleware (#30) #33

cal · 2026-03-03T05:37:40Z

cal commented

2026-03-03 05:37:40 +00:00

Summary

Add db_session_middleware to app/main.py that opens the DB connection at the start of every HTTP request and closes it in a try/finally block — guaranteeing connections are always released, even on uncaught exceptions
Remove all 467 individual db.close() calls from 30 router files in app/routers_v2/

What was fixed

Every endpoint manually called db.close() before each raise HTTPException(...) and before each return. Any code path that threw an unexpected exception would leak the connection. With 486 call sites, the pattern was also extremely brittle to maintain.

The middleware approach is the standard Peewee + FastAPI pattern:

@app.middleware("http")
async def db_session_middleware(request: Request, call_next):
    try:
        db.connect(reuse_if_open=True)
        response = await call_next(request)
        return response
    finally:
        if not db.is_closed():
            db.close()

db.connect(reuse_if_open=True) is safe for both SQLite and PooledPostgresqlDatabase (which returns the connection to the pool on close()).

Files changed

app/main.py — added from .db_engine import db import + middleware
app/routers_v2/*.py — removed db.close() from 30 files (467 calls)

Other observations

Some router files now have db as an unused import (e.g. admin.py, awards.py, current.py, rarity.py, rewards.py and a few others) where the only use was db.close(). These are harmless but could be cleaned up in a follow-up.

## Summary - Add `db_session_middleware` to `app/main.py` that opens the DB connection at the start of every HTTP request and closes it in a `try/finally` block — guaranteeing connections are always released, even on uncaught exceptions - Remove all 467 individual `db.close()` calls from 30 router files in `app/routers_v2/` ## What was fixed Every endpoint manually called `db.close()` before each `raise HTTPException(...)` and before each `return`. Any code path that threw an unexpected exception would leak the connection. With 486 call sites, the pattern was also extremely brittle to maintain. The middleware approach is the standard Peewee + FastAPI pattern: ```python @app.middleware("http") async def db_session_middleware(request: Request, call_next): try: db.connect(reuse_if_open=True) response = await call_next(request) return response finally: if not db.is_closed(): db.close() ``` `db.connect(reuse_if_open=True)` is safe for both SQLite and `PooledPostgresqlDatabase` (which returns the connection to the pool on `close()`). ## Files changed - `app/main.py` — added `from .db_engine import db` import + middleware - `app/routers_v2/*.py` — removed `db.close()` from 30 files (467 calls) ## Other observations Some router files now have `db` as an unused import (e.g. `admin.py`, `awards.py`, `current.py`, `rarity.py`, `rewards.py` and a few others) where the only use was `db.close()`. These are harmless but could be cleaned up in a follow-up.

cal added 1 commit 2026-03-03 05:37:41 +00:00

fix: replace 467 manual db.close() calls with middleware (#30 )

Build Docker Image / build (pull_request) Successful in 3m9s

Details

8c6a99253b

Add db_session_middleware to main.py that opens the connection at the
start of each request and closes it in a try/finally block, ensuring
connections are always returned even on uncaught exceptions.

Remove all individual db.close() calls from 30 router files in
app/routers_v2/ — the middleware now handles all code paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal referenced this pull request

2026-03-03 05:37:51 +00:00

Replace 488 manual db.close() calls with middleware/dependency context manager #30

cal added the

ai-reviewing

label 2026-03-03 06:02:03 +00:00

cal reviewed 2026-03-03 06:05:59 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/main.py (modified — added db import + middleware)
app/routers_v2/*.py — 30 router files (modified, db.close() removed)

Findings

Correctness

Middleware implementation is correct. Request is already imported on line 1 of main.py (from fastapi import FastAPI, Request), so the new middleware signature compiles without changes.
db.connect(reuse_if_open=True) is the documented safe pattern for both SqliteDatabase and PooledPostgresqlDatabase. With the pool (autoconnect=True in db_engine.py), close() returns the connection to the pool rather than destroying it — correct behaviour for prod.
The try/finally block guarantees the connection is released even when an endpoint raises an unhandled exception, which is exactly the bug being fixed.
The if not db.is_closed(): db.close() guard prevents a double-close on code paths where an endpoint may have already closed. Defensive and safe.
The one +db.close() line in the diff is the finally block in the middleware itself — not a missed removal.
No StreamingResponse or BackgroundTasks patterns exist in the routers, so there is no risk of the finally block releasing the connection before response body generation completes.
db.atomic() in players.py (put_players) is unaffected — Peewee atomic context managers work correctly with a pre-opened connection.

Security

No issues found. The middleware does not expose any new attack surface.

Style & Conventions

Import reformatting (one router per line, double-quoted strings) in main.py is a minor cosmetic improvement.
Unused db imports: Several router files (admin.py, awards.py, current.py, rarity.py, rewards.py, and others) now import db but never use it — the only use was db.close(). The PR body already flags these for follow-up cleanup. Harmless.

Suggestions

Follow-up: remove the now-unused db import from the router files. A simple grep -rn "^from \.\.db_engine import db" app/routers_v2/ after merge will identify the exact list.

Verdict: APPROVED ✓

Correct implementation of the standard Peewee + FastAPI connection-management pattern. Eliminates a real connection-leak bug across all 467 unguarded call sites. The middleware is the right architectural fix and is safe for both the SQLite (dev) and PooledPostgresqlDatabase (prod) backends. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified — added `db` import + middleware) - `app/routers_v2/*.py` — 30 router files (modified, `db.close()` removed) ### Findings #### Correctness - Middleware implementation is correct. `Request` is already imported on line 1 of `main.py` (`from fastapi import FastAPI, Request`), so the new middleware signature compiles without changes. - `db.connect(reuse_if_open=True)` is the documented safe pattern for both `SqliteDatabase` and `PooledPostgresqlDatabase`. With the pool (`autoconnect=True` in `db_engine.py`), `close()` returns the connection to the pool rather than destroying it — correct behaviour for prod. - The `try/finally` block guarantees the connection is released even when an endpoint raises an unhandled exception, which is exactly the bug being fixed. - The `if not db.is_closed(): db.close()` guard prevents a double-close on code paths where an endpoint may have already closed. Defensive and safe. - The one `+db.close()` line in the diff is the `finally` block in the middleware itself — not a missed removal. - No `StreamingResponse` or `BackgroundTasks` patterns exist in the routers, so there is no risk of the `finally` block releasing the connection before response body generation completes. - `db.atomic()` in `players.py` (`put_players`) is unaffected — Peewee atomic context managers work correctly with a pre-opened connection. #### Security - No issues found. The middleware does not expose any new attack surface. #### Style & Conventions - Import reformatting (one router per line, double-quoted strings) in `main.py` is a minor cosmetic improvement. - **Unused `db` imports**: Several router files (`admin.py`, `awards.py`, `current.py`, `rarity.py`, `rewards.py`, and others) now import `db` but never use it — the only use was `db.close()`. The PR body already flags these for follow-up cleanup. Harmless. #### Suggestions - Follow-up: remove the now-unused `db` import from the router files. A simple `grep -rn "^from \.\.db_engine import db" app/routers_v2/` after merge will identify the exact list. ### Verdict: APPROVED ✓ Correct implementation of the standard Peewee + FastAPI connection-management pattern. Eliminates a real connection-leak bug across all 467 unguarded call sites. The middleware is the right architectural fix and is safe for both the SQLite (dev) and `PooledPostgresqlDatabase` (prod) backends. Ready to merge. --- *Automated review by Claude PR Reviewer*

cal added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-03 06:06:23 +00:00

cal changed target branch from main to next-release

2026-03-03 16:54:11 +00:00

cal added

ai-pr-opened

and removed

ai-reviewed

labels 2026-03-03 16:54:39 +00:00

cal added the

ai-reviewing

label 2026-03-03 17:31:14 +00:00

cal reviewed 2026-03-03 17:34:38 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/main.py (modified — middleware added, imports reformatted)
app/routers_v2/*.py — all 30 router files (pure db.close() removals)
app/db_engine.py (read for context, not modified)

Findings

Correctness

The middleware implementation is correct. db.connect(reuse_if_open=True) is safe for both SqliteDatabase and PooledPostgresqlDatabase — for the pool, close() returns the connection to the pool rather than destroying it.
The finally block correctly guards against connection leaks on uncaught exceptions, which the prior 467 manual db.close() calls could not guarantee.
Request is already imported in main.py (used by get_docs), so no missing import.
All 30 router file changes are pure deletions of db.close() calls — no logic was altered.
db_engine.py calls db.close() at module level (line 1067) after table creation at startup. This pre-existing behaviour is unaffected; the middleware correctly handles per-request lifecycle after that.
One pre-existing correctness improvement incidentally achieved: in players.py, the old code called db.close() mid-function before a for x in card_vals: loop and then continued executing — a latent bug. The removal is strictly correct.
get_scouting_dfs in teams.py is a non-async helper; removing db.close() from it is correct since connection lifecycle is now owned by the middleware at the request boundary.

Security

No issues. The change narrows the attack surface by removing redundant, error-prone resource management.

Style & Conventions

Import block and FastAPI constructor args in main.py reformatted from compressed one-liners to one-per-line style — clean improvement, no concerns.
Several files retain a now-unused db import (e.g. admin.py, awards.py, current.py, rarity.py, rewards.py) where db was only used for db.close(). These are harmless but should be removed in a follow-up to keep linting clean.

Suggestions

Follow-up: Remove the now-unused db import from router files where the only previous use was db.close(). A grep pass will identify them quickly.

Verdict: APPROVED

Clean, correct refactor. The middleware pattern is the standard Peewee + FastAPI approach and strictly improves connection safety over 467 scattered db.close() calls. No security issues, no broken edge cases, no logic changes in the routers. Safe to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified — middleware added, imports reformatted) - `app/routers_v2/*.py` — all 30 router files (pure `db.close()` removals) - `app/db_engine.py` (read for context, not modified) ### Findings #### Correctness - The middleware implementation is correct. `db.connect(reuse_if_open=True)` is safe for both `SqliteDatabase` and `PooledPostgresqlDatabase` — for the pool, `close()` returns the connection to the pool rather than destroying it. - The `finally` block correctly guards against connection leaks on uncaught exceptions, which the prior 467 manual `db.close()` calls could not guarantee. - `Request` is already imported in `main.py` (used by `get_docs`), so no missing import. - All 30 router file changes are pure deletions of `db.close()` calls — no logic was altered. - `db_engine.py` calls `db.close()` at module level (line 1067) after table creation at startup. This pre-existing behaviour is unaffected; the middleware correctly handles per-request lifecycle after that. - One pre-existing correctness improvement incidentally achieved: in `players.py`, the old code called `db.close()` mid-function before a `for x in card_vals:` loop and then continued executing — a latent bug. The removal is strictly correct. - `get_scouting_dfs` in `teams.py` is a non-async helper; removing `db.close()` from it is correct since connection lifecycle is now owned by the middleware at the request boundary. #### Security - No issues. The change narrows the attack surface by removing redundant, error-prone resource management. #### Style & Conventions - Import block and FastAPI constructor args in `main.py` reformatted from compressed one-liners to one-per-line style — clean improvement, no concerns. - Several files retain a now-unused `db` import (e.g. `admin.py`, `awards.py`, `current.py`, `rarity.py`, `rewards.py`) where `db` was only used for `db.close()`. These are harmless but should be removed in a follow-up to keep linting clean. #### Suggestions - **Follow-up**: Remove the now-unused `db` import from router files where the only previous use was `db.close()`. A `grep` pass will identify them quickly. ### Verdict: APPROVED Clean, correct refactor. The middleware pattern is the standard Peewee + FastAPI approach and strictly improves connection safety over 467 scattered `db.close()` calls. No security issues, no broken edge cases, no logic changes in the routers. Safe to merge. --- *Automated review by Claude PR Reviewer*

cal removed the

ai-reviewing

label 2026-03-03 17:34:48 +00:00

cal reviewed 2026-03-03 18:08:55 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/main.py (modified — middleware added)
app/routers_v2/*.py (30 files — db.close() removals only)

Findings

Correctness

Middleware implementation is correct: db.connect(reuse_if_open=True) in try, db.close() in finally with an is_closed() guard.
Diff confirms exactly 467 db.close() lines removed across 30 router files, 0 other logic changes in any router.
The 1 added db.close() is the middleware's finally block — expected.
db is properly exported from db_engine.py (both SqliteDatabase and PooledPostgresqlDatabase paths).
Middleware placement after include_router() calls is valid — Starlette processes middleware at request time, not definition time.

Security

No issues. No user input flows into the middleware. No credentials introduced.

Style & Conventions

Follows the standard Peewee + FastAPI middleware pattern.
Import formatting in main.py (one-per-line routers) is an improvement to readability.

Suggestions

15 router files will have unused db imports after merge (files where db was only used for db.close()): admin.py, awards.py, cardsets.py, current.py, events.py, gamerewards.py, gauntletruns.py, notifications.py, packtypes.py, paperdex.py, rarity.py, results.py, rewards.py, scouting.py, stratgame.py. PR author already flagged this for follow-up — no blocker.

Verdict: APPROVED ✓

Clean, correct mechanical refactor. The middleware pattern eliminates the connection-leak risk and removes 467 brittle call sites. Safe for both SQLite (dev) and PooledPostgresqlDatabase (prod). The unused-import cleanup can follow in a separate PR.

Note: Gitea does not allow self-approval — posting as COMMENT. Recommendation is to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified — middleware added) - `app/routers_v2/*.py` (30 files — `db.close()` removals only) ### Findings #### Correctness - Middleware implementation is correct: `db.connect(reuse_if_open=True)` in `try`, `db.close()` in `finally` with an `is_closed()` guard. - Diff confirms exactly **467 `db.close()` lines removed** across 30 router files, **0 other logic changes** in any router. - The 1 added `db.close()` is the middleware's `finally` block — expected. - `db` is properly exported from `db_engine.py` (both `SqliteDatabase` and `PooledPostgresqlDatabase` paths). - Middleware placement after `include_router()` calls is valid — Starlette processes middleware at request time, not definition time. #### Security - No issues. No user input flows into the middleware. No credentials introduced. #### Style & Conventions - Follows the standard Peewee + FastAPI middleware pattern. - Import formatting in `main.py` (one-per-line routers) is an improvement to readability. #### Suggestions - **15 router files will have unused `db` imports after merge** (files where `db` was only used for `db.close()`): `admin.py`, `awards.py`, `cardsets.py`, `current.py`, `events.py`, `gamerewards.py`, `gauntletruns.py`, `notifications.py`, `packtypes.py`, `paperdex.py`, `rarity.py`, `results.py`, `rewards.py`, `scouting.py`, `stratgame.py`. PR author already flagged this for follow-up — no blocker. ### Verdict: APPROVED ✓ Clean, correct mechanical refactor. The middleware pattern eliminates the connection-leak risk and removes 467 brittle call sites. Safe for both SQLite (dev) and `PooledPostgresqlDatabase` (prod). The unused-import cleanup can follow in a separate PR. > Note: Gitea does not allow self-approval — posting as COMMENT. Recommendation is to **merge**. --- *Automated review by Claude PR Reviewer*

cal added the

ai-reviewed

label 2026-03-03 18:09:23 +00:00

cal referenced this issue from a commit

2026-03-03 21:51:28 +00:00

fix: remove unused imports in PR #33 files

cal added 1 commit 2026-03-03 21:51:28 +00:00

fix: remove unused imports in PR #33 files 46b9aa27be

Removed 55 unused imports across 26 router files. Most were `db` imports
left over after the db.close() removal in the previous commit, plus
additional stale imports (scipy.stats, chunked, copy, base64, Html2Image,
pandas.DataFrame, pydantic.validator, etc.) that were already unused.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal force-pushed ai/paper-dynasty-database#30 from 46b9aa27be to bcdbf2add1

2026-03-03 21:53:26 +00:00

Compare

cal referenced this issue from a commit

2026-03-03 21:53:26 +00:00

fix: remove unused imports in PR #33 files

cal merged commit 761c0a6dab into next-release

2026-03-03 21:53:38 +00:00

cal deleted branch ai/paper-dynasty-database#30

2026-03-03 21:53:38 +00:00

cal referenced this issue from a commit

2026-03-03 21:53:39 +00:00

Merge pull request 'fix: replace 467 manual db.close() calls with middleware (#30)' (#33) from ai/paper-dynasty-database#30 into next-release

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 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-database#33