Skip to content

fix: open a Transaction in the read use cases so a retry can recover - #51

Merged
lesnik512 merged 1 commit into
mainfrom
fix/read-use-cases-transaction
Oct 16, 2026
Merged

lesnik512 merged 1 commit into
mainfrom
fix/read-use-cases-transaction

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Follow-up to modern-python/db-retry#68.

Six use cases had @postgres_retry but ran their queries directly on the injected request session: authenticate_user, check_token_revoked, fetch_chat, fetch_chats, fetch_messages and fetch_user. When the session's pooled connection dies, the first attempt fails and leaves the session inside the failed transaction. postgres_retry has no handle on the session, so the retry reuses it and fails with PendingRollbackError: Can't reconnect until invalid transaction is rolled back. The auth middleware calls check_token_revoked on the request session before the handler runs, so every authenticated request takes this path.

Each of the six now takes transaction: Transaction, like the write use cases, and runs its queries inside async with self.transaction:. When an attempt fails, Transaction.__aexit__ rolls back and closes the session, and the next attempt checks out a fresh connection.

Each one also commits before leaving the block. __aexit__ rolls back any open transaction, and a rollback expires every instance the session loaded, so without the commit the returned user, chat or messages come back detached and expired. My first version skipped the commit and 26 existing tests failed. Because the session uses expire_on_commit=False, committing keeps the objects loaded. The return sits after the block, so these use cases don't need the comment that edit_message has.

authenticate_user commits before the argon2 check, so a login no longer keeps a transaction open while it hashes.

Test

tests/use_cases/test_connection_loss.py resolves each use case from a request container backed by a real engine. The rolled-back test connection can't be used here because it cannot reconnect. The fixture runs a query on the request session, terminates that backend with pg_terminate_backend, waits until asyncpg sees the connection close, and then the test calls the use case.

db-retry is not bumped

modern-python/db-retry#69 makes postgres_retry retry asyncpg's InterfaceError: connection is closed. It is still open, and the latest release (0.7.0) doesn't include it. With released db-retry the first attempt is never retried, so the new tests can't pass yet. They are marked strict xfail, with raises= limited to the two errors a missing retry produces here: advanced-alchemy's wrapped IntegrityError and SQLAlchemy's InterfaceError. When db-retry ships #69 and the lock is upgraded, the tests will XPASS and strict mode will fail the run. Delete the pytestmark at that point.

Verified locally

  • just test on db-retry 0.5.1 (the current lock): 123 passed, 6 xfailed, 100% coverage.
  • #69's branch installed in the container, without the xfail marker: 129 passed, 100% coverage. Before the use case change, all six new tests failed with PendingRollbackError.
  • ruff check, ruff format and ty check are clean.

AGENTS.md now says read-only use cases open a Transaction too, and why.

A retry on the shared request session failed with PendingRollbackError after a lost connection, because the session was still inside the failed transaction.
@lesnik512
lesnik512 merged commit 3e27d65 into main Oct 16, 2026
4 checks passed
@lesnik512
lesnik512 deleted the fix/read-use-cases-transaction branch October 16, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant