Fastapi: [BUG] Yield dependency doesn't get http exceptions being raised.

[hadrien]

Describe the bug

HTTP exceptions are not ~raised~ propagated inside a dependency which encapsulates the yield in a try except statement.
Only custom exceptions are ~raised~ propagated if I understand correctly.

To Reproduce

1. Create a main.py with:

   from fastapi import Depends, FastAPI, HTTPException
   
   app = FastAPI()
   
   
   def new_db_session():
       try:
           yield
   
       except Exception:
           print("rollback")
   
       else:
           print("commit")
   
       finally:
           print("close")
   
   
   @app.get("/exception")
   def get_exception(db=Depends(new_db_session)):
       raise Exception()
   
   
   @app.get("/http_exception")
   def get_exception(db=Depends(new_db_session)):
       raise HTTPException(status_code=400, detail="invalid request")
   

2. run app using uvicorn main:app

3. curl http://127.0.0.1:8000/exception; app prints:

   INFO:     127.0.0.1:50069 - "GET /exception HTTP/1.1" 500 Internal Server Error
   rollback
   close
   

4. curl http://127.0.0.1:8000/http_exception; app prints:

   INFO:     127.0.0.1:50072 - "GET /http_exception HTTP/1.1" 400 Bad Request
   commit
   close
   

   Expected behavior

HTTP Exception should be ~raised~ propagated inside the dependency try except statement.

Environment

• OS: Darwin Kernel Version 19.0.0: Thu Oct 17 16:17:15 PDT 2019; root:xnu-6153.41.3~29/RELEASE_X86_64 x86_64 i386 MacBookPro13,1 Darwin
• FastAPI Version: 0.46.0
• Python version: Python 3.7.3

Additional context

• PR: Dependencies with yield (used as context managers)
• From @dmontagu in #570:
  > @cjw296 As implemented, it would require some care to ensure teardown was done in a guaranteed order. I think it would currently amount to constructing a consistent "teardown" dependency. But I think in most cases, you would be able to just build a single dependency that sets everything up in order, and tears it down in order, and then to build other dependencies on top of that. For example, handling your use case:
  >
  > python > def db_contextmanager(): > conn = connect_to_db() > t = conn.transaction() > try: > yield conn, t > t.commit() > except: > t.rollback() > conn.close() > > def get_connection(conn_t = Depends(db_contextmanager)): > conn, t = conn_t > return conn > > def get_transaction(conn_t = Depends(db_contextmanager)): > conn, t = conn_t > return t > > def endpoint(conn = Depends(get_connection)): > # Do something with the connection > pass >

[cjw296]

@dmontagu - this does look legit, I suspect @hadrien meant their fixture to be:

def new_db_session():
    try:
        yield
        print("commit")
    except:
        print("rollback")
        raise
    finally:
        print("close")

[hadrien]

@cjw296 Thanks for getting your attention on this. 🙏

I suspect @hadrien meant their fixture to be:

Why should a context manager dependency re-raise the exception? Wouldn't it break exiting others context manager dependencies?

(Excuse my english, this is not my first language)

[cjw296]

If I:

raise HTTPException(status_code=404, detail="Item not found")

...the database dependency has no business swallowing that exception and turning it into a 200 response.

[hadrien]

...the database dependency has no business swallowing that exception and turning it into a 200 response.

Which is not the behaviour of the framework in the case of an exception: as shown in my example, the custom exception is _swallowed_ by the dependency, still, app returns http 500. Totally what I expect as a framework user.

[tiangolo]

@hadrien dependencies with yield are above/around other exception handlers. So the handler for HTTPException will catch and handle that exception and the dependency with yield won't see it.

This is what allows exceptions that could occur after the response is sent to still be handled by the dependency.

For example, a background task could need to use a DB session from the dependency. That background task would be executed after the response is sent to the client. But still, it could raise a DB exception, and the dependency would be able to catch it.

If you want to catch an HTTPException and return something different in the response, that would be in an exception handler. Dependencies with yield are not made to replace them, but to use resources (e.g. db sessions) that can have some closing code, and that can be used even after the response is sent, by background tasks.

I created #981 to clarify this in the docs.

[hadrien]

@tiangolo Thanks so much for your explanation.

If you want to catch an HTTPException and return something different in the response, that would be in an exception handler.

I totally agree. I do not intend to use yield dependency as an exception handler.

As a framework user, I expect HTTPException as well as any Exception to be raised in the dependency with yield.

In my specific example, I intend to rollback db session any time any exception occurs.

It is very well documented in FastAPI documentation. It also follows best practices from sqlalchemy.

BTW:
Thanks again for FastAPI, we are very fond of it at @dialoguemd and actually have a _beer & learn_ presentation on FastAPI in 30 minutes to easily convince the whole tech team to use it EVERYWHERE. 🎉

Last but not least:

1. I've tried to fix that issue by myself but was not able to fully get how to. If you believe it is a bug and can give me few pointers, I'd love to contribute.

2. Dependencies with yield are not made to replace them, but to use resources (e.g. db sessions) that can have some closing code, and that can be used even after the response is sent, by background tasks.

   _"that can be used even after the response is sent"_: Do you mean a db session yielded in a path operation would not be exited when response is sent?

[jonathanunderwood]

I too am still confused by this. Indeed in all our use cases of FastAPI we've had to stop using Dependencies for SQLAlchemy session management, and instead use a middleware.

To summarise, this use case:

• Create a SQLA session on request arrival, and start a transaction
• If an exception occurs in the endpoint, rollback the transaction
• If an exception doesn't occur, then commit the transaction

doesn't seem to be achievable with a Dependency, right?

If so, Dependencies really seem to be quite limited through the choice of where they lie in the stack. This use case alone is so common, I think there's a case to be made for having an option to inject a Dependency higher in the stack so that it can catch exceptions before exception handlers, do something, and re-raise.

[tiangolo]

That's great to hear @hadrien ! :tada: :rocket:

@jonathanunderwood you should be able to do that. What you can't do is raise another exception (e.g. an HTTPException) from the exit code in the dependency with yield, the code after the yield.

But if you have code that could raise an exception in a controlled way and you want to handle that as a known error that is part of the flow of your program, I think it would be better to put that in a normal try block and handle it directly in the corresponding section of the code, not in a dependency. If your code is randomly raising an exception without your knowledge of where or why, that's probably a bug in the code. And in that case, the default 500 Server Error exception would actually be more or less "correct", as it wouldn't really be a client error but an error in the code. But even after the response of a 500 Server Error, the exit code in your dependency (the code after the yield) could catch that exception and rollback the transaction, even if there was an error with a 500 HTTP code, you could still roll back the transaction.

For example, if your path operation code triggered a background job that creates a record, and that background task raises an exception, your dependency will be able to handle it and rollback, despite the fact that the response was already sent before even starting the background task.

[tiangolo]

As I see that this seems to be a source of confusion, I spent some time clarifying it in the docs and adding diagrams (and support for diagrams :sweat_smile: ) with the whole flow of execution.

I hope that helps to clarify all this. https://fastapi.tiangolo.com/tutorial/dependencies/dependencies-with-yield/#dependencies-with-yield-and-httpexception

[dmontagu]

@tiangolo these docs are incredible 👏👏

[hadrien]

This is very nice doc.

Still, I believe there is a misunderstanding with the current issue.

In the current issue, I do not want the yield dependency to raise any exception, and I do not expect any exception thrown by yield dependencies to be handled nicely by the framework.

As a framework user, if ever an endpoint raises any exception: http or custom one, I expect that exception to be _propagated_ in the yield dependency if it has a try except statement:

from fastapi import Depends, FastAPI, HTTPException

app = FastAPI()

def with_this():
    try:
        yield
    except Exception:
        print("Hello!")

@app.get("/exception")
def get_exception(db=Depends(with_this)):
    raise Exception()


@app.get("/http_exception")
def get_exception(db=Depends(with_this)):
    raise HTTPException(status_code=400, detail="invalid request")

I expect Hello! to be printed when doing a get on /exception AND /http_exception. Currently, only custom exception are propagated.

Does it clarify what I tried to explain initially?

[dmontagu]

I think this behavior is explained in the new docs.

It sounds to me like your question is closer to "how can I detect whether an error was raised during the execution of the yield statement, even if it was handled by an exception handler". Unfortunately, as of now, I think the answer is just that you can't. So maybe this issue is better classified as a feature request than a bug.

For what it's worth, I do think this is a reasonable feature request, though I'm not sure 1) what a good API for it would be, or 2) whether there are any common problems that could be caused by "double-handling" the exception like that.

That said, I would be in favor of having exceptions raised in contextmanager dependencies even if they were also handled by request-handling code, as it would make it easier to perform proper cleanup (e.g., rolling back a transaction when using a contextmanager-based sqlalchemy session dependency).

[tiangolo]

I expect Hello! to be printed when doing a get on /exception AND /http_exception. Currently, only custom exception are propagated.

These HTTPExceptions have a custom meaning and use internally, to return an error to the user right away, and they have that specific custom meaning including that they are not really errors in your code but in the client's request. So it wouldn't make sense for dependencies with yield to handle them as exceptions in your code. But apart from that, dependencies with yield could provide resources for background tasks that live longer than the request-response cycle, so that wouldn't really be possible.

Check this section in the docs for more info: https://fastapi.tiangolo.com/tutorial/dependencies/dependencies-with-yield/#dependencies-with-yield-and-httpexception

[hadrien]

I've read and understand the doc and the way it is done internally.

Thanks a lot for your time and attention!

[artslob]

This is also clarified in starlette docs: https://www.starlette.io/exceptions/.
HTTPException is actually _handled_ exception, not an _error_. Thats why its not propagated to dependencies.