Differentiation of library and user transactions to correct the "A transaction is already begun on this Session" error

[mixa2130]

Hello guys!
I'm new at fastapi-users, so pls don't throw slippers at me...

Working with this lib I've found out an interesting problem: in the endpoint where there is a dependency on fastapi_users.current_user I can't isolate my transactions without making an extra commit before.
Getting InvalidRequestError: A transaction is already begun on this Session. error at

async with session.begin():
      .....

According to sqlalchemy docs:

This “virtual” transaction is created automatically when needed, or can alternatively be started using the Session.begin() method.

I think it would be great to add extra session.commit() at functions which works with the db

I'll explain why this seems like a good idea to me.
For instance, there is a bug in the new version of fastapi-users lib that makes some extra objects in db. There are no commits at the end of functions in BaseUserManager. So this objects would be created with the first of my commits in the endpoint. Same story about rollback. Sounds not cool, isn't it?
Making extra commit at the highest level of abstraction allows us to separate user transactions with the lib ones. Even if anything will go wrong - it would be more easier to find-out where the problem comes: lib or endpoint code.

Ok, time to code 🐢

To test error

take an example from the website, create basic config

auth_backend = AuthenticationBackend(
    name="jwt",
    transport=cookie_transport,
    get_strategy=get_jwt_strategy,
)

fastapi_users = FastAPIUsers[User, uuid.UUID](
    get_user_manager,
    [auth_backend],
)

current_user = fastapi_users.current_user()

Basic endpoint:

@router.post("/")
async def add_event(event: schemas.NewEventSchema,
                    session: AsyncSession = Depends(APP_CTX.pg_controller.get_async_session),
                    user: User = Depends(current_user)):
    async with session.begin():
         # just try to add smth to db

Without making session.commit() before async with session.begin(): you won't be able to isolate your transaction.

To fix the problem

Experimenting with the lib code I've found out that this problem has a very simple decision: just add session.commit() at the end of each BaseUserManager(Generic[models.UP, models.ID]) functions:

    async def get(self, id: models.ID) -> models.UP:
        """
        Get a user by id.

        :param id: Id. of the user to retrieve.
        :raises UserNotExists: The user does not exist.
        :return: A user.
        """
        user = await self.user_db.get(id)

        if user is None:
            raise exceptions.UserNotExists()
        
        ########### FIX ############
        await self.user_db.session.commit()
        ########### FIX ############
        return user

I'm ready to fix this problem with pull request which fully passes the tests, but don't know is it really a good idea for all other people. Maybe smth isn't right with me

[frankie567]

Thank you @mixa2130 for the detailed explanation. There are several things I don't get though:

For instance, there is a bug in the new version of fastapi-users lib that makes some extra objects in db.

Can you add more details about this? If this is confirmed, it looks like the first thing to fix.

---

Now, regarding the session.begin() call, I think it's actually not needed in your code. SQLAlchemy docs state this:

The Session object features autobegin behavior, so that normally it is not necessary to call the Session.begin() method explicitly. However, it may be used in order to control the scope of when the transactional state is begun.

So, when you're inside your router logic, the session has already begun (because FastAPI Users indeed make a call to query the user). If you need to have another level, you can set the nested argument to True or use begin_nested:

nested – if True, begins a SAVEPOINT transaction and is equivalent to calling Session.begin_nested(). For documentation on SAVEPOINT transactions, please see Using SAVEPOINT.

[mixa2130]

Hello @frankie567 ! Thank you for your answer!
I am completely familiar with this "autocommit" behavior of sqlalchemy. I think you misunderstood me. I wasn't talking about the session, but about the transaction. session.begin() allows us to start transaction manually, isn't it? That's the point! If I want to separate my router logic and make a dedicated transaction with my own logic(independent from everything that happened previously at code) I have to start my accessing the db with session.begin(), right? But if I'm using fastapi_users I can't do this without making extra session.commit() which ends previous lib virtual transaction.

[frankie567]

Yes, I get it. But I wasn't talking about autocommit, but autobegin. The fact that the transaction automatically opens is not the fault of FastAPI Users, but of SQLAlchemy Session which implicitly opens the transaction by default. Ref: https://docs.sqlalchemy.org/en/20/orm/session_basics.html#auto-begin

It means that, when FastAPI Users will query for the current user, a transaction has to be started. Then, for your use-case, what you can do is to start a nested transaction using .begin(nested=True) or .begin_nested():

@router.post("/")
async def add_event(event: schemas.NewEventSchema,
                    session: AsyncSession = Depends(APP_CTX.pg_controller.get_async_session),
                    user: User = Depends(current_user)):
    async with session.begin_nested():
         # just try to add smth to db

This way, you can finely control the SAVEPOINT in your logic and rollback the parts you want. Ref: https://docs.sqlalchemy.org/en/20/orm/session_transaction.html#session-begin-nested