Handle errors in transaction.commit() (for sqlite) #594
Description
Context
For sqlite backend, when inside a transaction, errors are raised at the execution of the COMMIT statement instead of the SQL that causes the error, for example
PRAGMA foreign_keys = ON;
BEGIN TRANSACTION;
INSERT INTO "follow" ("user_id", "target_id") VALUES (0, 0);
COMMIT;It was line 3 causing the SQLITE_CONSTRAINT_FOREIGNKEY error, but the error is raised during the COMMIT execution;
Problem
in databases/core.py line 463:
class Transaction:
async def commit(self) -> None:
async with self._connection._transaction_lock:
assert self._connection._transaction_stack[-1] is self
self._connection._transaction_stack.pop() # this line
assert self._transaction is not None
await self._transaction.commit()
await self._connection.__aexit__()
self._transaction = Nonethe _transaction_stack.pop() is called before the _transaction.commit() is called, so if _transaction.commit() raised an error, the current transaction is lost in the connection and cannot be rolled back, thus leading the database to be locked;
Fix
To prevent this problem, I came up with a workaround:
- to catch errors in
commit()and callrollback()manually - move the
_transaction_stack.pop()after thecommit()statement, so the transaction will be poped after committed successfully
namely:
from databases.core import Transaction
class _Transaction(Transaction):
async def commit(self) -> None:
async with self._connection._transaction_lock:
assert self._connection._transaction_stack[-1] is self
assert self._transaction is not None
await self._transaction.commit()
# POP after committing successfully
self._connection._transaction_stack.pop()
await self._connection.__aexit__()
self._transaction = None
async def __aexit__(self, exc_type, exc_value, traceback):
"""
Called when exiting `async with database.transaction()`
"""
if exc_type is not None or self._force_rollback:
await self.rollback()
else:
try:
await self.commit()
except Exception as e:
await self.rollback()
raise eI am following the contributing guide here to submit an issue to discuss this modification before I make a pull request, and hope this can be merged to the repo soon