Closing does not respect if an exception has been thrown
Opened this issue · 2 comments
When using a method with an inject decorator, and we inject a resource that is wrapped up with the Closing marker, if that method throws an exception then I would expect the except in the resource to catch the exception and (in this case) perform a rollback. However, actually what happens is that the code after the yield continues and the finally statement is executed but the except is never run.
The below code is a basic example to show this happening.
import asyncio
from asyncio import sleep
from dependency_injector import containers, providers
from dependency_injector.wiring import Closing, Provide, inject
class Database:
@staticmethod
async def get_session():
session = "a test"
try:
yield session
await sleep(0.1)
print("Session commited.")
except Exception:
await sleep(0.1)
print("Session rollback.")
raise
finally:
await sleep(0.1)
print("Session closed.")
class Container(containers.DeclarativeContainer):
session = providers.Resource(Database.get_session)
@inject
async def test_function(session = Closing[Provide["session"]]):
print("test_function")
# session.add()
await sleep(0.1)
@inject
async def test_exception_function(session = Closing[Provide["session"]]):
print("test_exception_function")
# session.add()
await sleep(0.1)
raise Exception
async def main():
container = Container()
container.wire(modules=[__name__])
await test_function()
await test_exception_function()
asyncio.run(main())
Running this produces
test_function
Session commited.
Session closed.
test_exception_function
Session commited.
Session closed.
<stack_track>
Exception
Naively, I would have expected the output to be
test_function
Session commited.
Session closed.
test_exception_function
Session rollback.
Session closed.
<stack_track>
Exception
I am very new to using this package so it could be simply a misunderstanding on my part. My only comparison is that when using dependency injection within fastAPI (using their dependency injection and not this package), the same equivalent example does work e.g. the except catches the exception and the rollback happens as expected.
Is this an issue with the above implementation or is this intended behaviour?
That's how it was historically implemented, mimicking pytest yield fixture behavior. Initial implementation had no exception info around, so we did not raise anything here. Since then we've refactored Resource providers to be proper context managers under the hood and now we do have exception info, but if we start throwing it now it'll most likely break a lot of existing code, so I think it is better to postpone it to the next major release.
As of your particular use case, I'd recommend implementing UoW pattern and doing commits explicitly vs implicitly (your example).
Awesome, thank you for the information and references.