Refactor manual db.close() calls to middleware-based connection management #71

New Issue

cal · 2026-03-27T04:06:24Z

cal commented

2026-03-27 04:06:24 +00:00

Problem

There are ~180 manual db.close() calls scattered across 28 router files. With PooledPostgresqlDatabase, this creates several problems:

Connection returned before response serialization — Peewee lazy-loads foreign keys during model_to_dict(recurse=True). If the connection is returned to the pool before serialization completes, concurrent requests can steal it.
Redundant cleanup — The @handle_db_errors decorator already calls db.rollback() and db.close() on exceptions.
Double-close on error paths — When a handler calls db.close() then raises HTTPException, the decorator also closes.

Fix

Use Peewee's @db.connection_context() decorator or FastAPI middleware to manage connection lifecycle per-request. Remove all manual db.close() calls from routers.

Severity

High — latent reliability risk under load.

## Problem There are ~180 manual `db.close()` calls scattered across 28 router files. With `PooledPostgresqlDatabase`, this creates several problems: 1. **Connection returned before response serialization** — Peewee lazy-loads foreign keys during `model_to_dict(recurse=True)`. If the connection is returned to the pool before serialization completes, concurrent requests can steal it. 2. **Redundant cleanup** — The `@handle_db_errors` decorator already calls `db.rollback()` and `db.close()` on exceptions. 3. **Double-close on error paths** — When a handler calls `db.close()` then raises HTTPException, the decorator also closes. ## Fix Use Peewee's `@db.connection_context()` decorator or FastAPI middleware to manage connection lifecycle per-request. Remove all manual `db.close()` calls from routers. ## Severity High — latent reliability risk under load.

Claude added the

ai-working

label 2026-03-27 11:01:10 +00:00

cal referenced this issue from a commit

2026-03-27 11:05:04 +00:00

fix: replace manual db.close() calls with middleware-based connection management (#71)

Claude referenced a pull request that will close this issue

2026-03-27 11:05:19 +00:00

fix: replace manual db.close() calls with middleware-based connection management (#71) #97

Claude removed the

ai-working

label 2026-03-27 11:05:25 +00:00

Claude commented

2026-03-27 11:05:29 +00:00

PR #97 implements this fix: #97

Approach: Added db_connection_middleware to app/main.py that opens db.connect(reuse_if_open=True) at the start of each HTTP request and closes in a finally block after the response is fully sent. Removed all 177 manual db.close() calls from 26 router files.

This ensures the connection stays alive through FastAPI's response serialization (fixing the FK lazy-load race), eliminates double-close on error paths, and centralizes connection lifecycle in one place.

PR #97 implements this fix: https://git.manticorum.com/cal/major-domo-database/pulls/97 **Approach**: Added `db_connection_middleware` to `app/main.py` that opens `db.connect(reuse_if_open=True)` at the start of each HTTP request and closes in a `finally` block after the response is fully sent. Removed all 177 manual `db.close()` calls from 26 router files. This ensures the connection stays alive through FastAPI's response serialization (fixing the FK lazy-load race), eliminates double-close on error paths, and centralizes connection lifecycle in one place.

Claude added the

ai-pr-opened

label 2026-03-27 11:05:34 +00:00

Sign in to join this conversation.