databases icon indicating copy to clipboard operation
databases copied to clipboard

Handle errors in transaction.commit() (for sqlite)

Open voidZXL opened this issue 1 year ago • 0 comments

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 = None

the _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:

  1. to catch errors in commit() and call rollback() manually
  2. move the _transaction_stack.pop() after the commit() 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 e

I 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

voidZXL avatar Oct 06 '24 08:10 voidZXL