Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
voidZXL opened this issue Oct 6, 2024 · 0 comments
Open

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

voidZXL opened this issue Oct 6, 2024 · 0 comments

Comments

@voidZXL
Copy link

voidZXL commented Oct 6, 2024

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

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

No branches or pull requests

1 participant