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

Bug: IntegrityError in ModelView.delete method causes PendingRollbackError in some cases #100

Closed
dolamroth opened this issue Feb 25, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@dolamroth
Copy link
Contributor

dolamroth commented Feb 25, 2023

Describe the bug
I am using starlette_admin with sqlalchemy contrib. DBSessionMiddleware, as first middleware, creates session and then properly closes it at the end. However, if error occurs during ModelView.delete method, the IntegrityError occurs, which should be handled.

I have inherited BaseAdmin and re-defined exception handlers to handle StarletteAdminException and SQLAlchemy's IntegrityError with the same function _render_error, which was used for HttpException. However, this function renders template "error.html" by using AuthProvider.get_admin_user, which (in my case, at least) internally asks session for user fields. And since session is in invalid state, but not rollbacked yet by DBSessionMiddleware, there occurs a sqlalchemy.exc.PendingRollbackError exception.

The traceback is as follows:

Traceback (most recent call last):
  File "D:\Projects\starlette-web\venv\lib\site-packages\uvicorn\protocols\http\h11_impl.py", line 403, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "D:\Projects\starlette-web\venv\lib\site-packages\uvicorn\middleware\proxy_headers.py", line 74, in __call__
    return await self.app(scope, receive, send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\applications.py", line 112, in __call__
    await self.middleware_stack(scope, receive, send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\errors.py", line 184, in __call__
    raise exc
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sentry_sdk\integrations\asgi.py", line 139, in _run_asgi3
    return await self._run_app(scope, lambda: self.app(scope, receive, send))
  File "D:\Projects\starlette-web\venv\lib\site-packages\sentry_sdk\integrations\asgi.py", line 186, in _run_app
    raise exc from None
  File "D:\Projects\starlette-web\venv\lib\site-packages\sentry_sdk\integrations\asgi.py", line 183, in _run_app
    return await callback()
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\exceptions.py", line 75, in __call__
    raise exc
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\exceptions.py", line 64, in __call__
    await self.app(scope, receive, sender)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\routing.py", line 687, in __call__
    await route.handle(scope, receive, send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\routing.py", line 436, in handle
    await self.app(scope, receive, send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\applications.py", line 112, in __call__
    await self.middleware_stack(scope, receive, send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\errors.py", line 184, in __call__
    raise exc
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\base.py", line 104, in __call__
    response = await self.dispatch_func(request, call_next)
  File "D:\Projects\starlette-web\starlette_web\contrib\admin\middleware.py", line 28, in dispatch
    response = await call_next(request)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\base.py", line 80, in call_next
    raise app_exc
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\base.py", line 66, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "D:\Projects\starlette-web\starlette_web\contrib\admin\middleware.py", line 75, in __call__
    await self.app(scope, receive, send_wrapper)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\base.py", line 104, in __call__
    response = await self.dispatch_func(request, call_next)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette_admin\auth.py", line 161, in dispatch
    return await call_next(request)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\base.py", line 80, in call_next
    raise app_exc
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\base.py", line 66, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\middleware\exceptions.py", line 84, in __call__
    response = await handler(request, exc)
  File "D:\Projects\starlette-web\starlette_web\contrib\admin\admin.py", line 101, in _render_error
    return self.templates.TemplateResponse(
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\templating.py", line 112, in TemplateResponse
    return _TemplateResponse(
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette\templating.py", line 38, in __init__
    content = template.render(context)
  File "D:\Projects\starlette-web\venv\lib\site-packages\jinja2\environment.py", line 1285, in render
    self.environment.handle_exception()
  File "D:\Projects\starlette-web\venv\lib\site-packages\jinja2\environment.py", line 926, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "D:\Projects\starlette-web\venv\lib\site-packages\starlette_admin\templates\error.html", line 1, in top-level template code
    {% extends "layout.html" %}
File "D:\Projects\starlette-web\venv\lib\site-packages\starlette_admin\templates\layout.html", line 3, in top-level template code
    {% set current_user = (request | get_admin_user) %}
  File "D:\Projects\starlette-web\starlette_web\contrib\admin\auth_provider.py", line 65, in get_admin_user
    username = request.scope["user"].email
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 478, in __get__
    return self.impl.get(state, dict_)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 919, in get
    value = self._fire_loader_callables(state, key, passive)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 946, in _fire_loader_callables
    return state._load_expired(state, passive)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\state.py", line 695, in _load_expired
    self.manager.expired_attribute_loader(self, toload, passive)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\loading.py", line 1383, in load_scalar_attributes
    result = load_on_ident(
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\loading.py", line 385, in load_on_ident
    return load_on_pk_identity(
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\loading.py", line 499, in load_on_pk_identity
    session.execute(
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\session.py", line 1676, in execute
    conn = self._connection_for_bind(bind)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\session.py", line 1529, in _connection_for_bind
    return self._transaction._connection_for_bind(engine, execution_options)
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\session.py", line 714, in _connection_for_bind
    self._assert_active()
  File "D:\Projects\starlette-web\venv\lib\site-packages\sqlalchemy\orm\session.py", line 598, in _assert_active
    raise sa_exc.PendingRollbackError(
sqlalchemy.exc.PendingRollbackError: This Session's transaction has been rolled back due to a previous exception during flush. To begin a new transaction with this Session, first issue Session.rollback(). Original exception was:

(sqlalchemy.dialects.postgresql.asyncpg.IntegrityError) <class 'asyncpg.exceptions.ForeignKeyViolationError'>: UPDATE или DELETE в таблице "auth_users" нарушает ограничение внешнего ключа "auth_sessions_user_id_fkey" таблицы "auth_sessions"
DETAIL:  На ключ (id)=(1) всё ещё есть ссылки в таблице "auth_sessions".
[SQL: DELETE FROM auth_users WHERE auth_users.id = %s]
[parameters: ((1,), (2,))]
(Background on this error at: https://sqlalche.me/e/14/gkpj) (Background on this error at: https://sqlalche.me/e/14/7s2a)

(Sorry that the last lines are in Russian, but idea is understandable, I hope)

Environment (please complete the following information):

  • starlette==0.25.0
  • starlette-admin==0.5.2
  • SQLAlchemy==1.4.46

Additional context
This seems to be a part of more general problem, that points out, that whenever exception occurs in methods, session should be rolled back immediately.

Luckly, the bug seems to be solvable by simply wrapping this 2 lines in async with session.begin_nested() https://github.com/jowilf/starlette-admin/blob/main/starlette_admin/contrib/sqla/view.py#L378-L379 (in the same way, with sync-session)

I haven't studied this behavior on methods, other than delete.

@jowilf
Copy link
Owner

jowilf commented Feb 26, 2023

It's recommended to fully load your user object in AuthProvider.is_authenticated method and save it into the request state. You can then, retrieve the user from request state in AuthProvider.get_admin_user without accessing the request session.

For example, https://github.com/jowilf/starlette-admin/blob/main/examples/auth/provider.py

@dolamroth
Copy link
Contributor Author

It appears, that in my case it was non-obvious logic in DBSessionMiddleware, which I tweaked to use custom sessionmaker and commit at end.

I thought, that the exception would be propagated all up to DBSessionMiddleware, but in that middleware call to call_next returned _StreamingResponse. I removed session.commit and everything works.

As for AuthProvider.is_authenticated, I did everything as per example and it works.

To sum up, it was error in my code. Thank you for your help. I'll close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants