🐛 Preserve traceback when an exception is raised in sync dependency with yield
#5823
Conversation
|
📝 Docs preview for commit 202133b at: https://63af5d1c07bec913f8d350a9--fastapi.netlify.app |
|
great workaround. thanks! |
|
📝 Docs preview for commit 411ad02 at: https://63af6350238dde190a34aafc--fastapi.netlify.app |
|
Nice work! We didn't upgrade to 3.11 because of this. |
|
📝 Docs preview for commit efd395c at: https://63bab4ceb11ed37e88ef871e--fastapi.netlify.app |
|
I patched your change with the following code and now the traceback's back. Thanks. from contextlib import asynccontextmanager as asynccontextmanager
from typing import AsyncGenerator, ContextManager, TypeVar
import anyio
from anyio import CapacityLimiter
from starlette.concurrency import run_in_threadpool as run_in_threadpool # noqa
_T = TypeVar("_T")
#
@asynccontextmanager
async def contextmanager_in_threadpool(
cm: ContextManager[_T],
) -> AsyncGenerator[_T, None]:
# blocking __exit__ from running waiting on a free thread
# can create race conditions/deadlocks if the context manager itself
# has it's own internal pool (e.g. a database connection pool)
# to avoid this we let __exit__ run without a capacity limit
# since we're creating a new limiter for each call, any non-zero limit
# works (1 is arbitrary)
exit_limiter = CapacityLimiter(1)
try:
yield await run_in_threadpool(cm.__enter__)
except Exception as e:
traceback_ = e.__traceback__
ok = bool(
await anyio.to_thread.run_sync(
cm.__exit__, type(e), e, None, limiter=exit_limiter
)
)
if not ok:
if e.__traceback__ is None:
e.__traceback__ = traceback_
raise e
else:
await anyio.to_thread.run_sync(
cm.__exit__, None, None, None, limiter=exit_limiter
)
# Run this before starting anything FastAPI related
from fastapi.dependencies import utils
utils.contextmanager_in_threadpool = contextmanager_in_threadpool |
|
📝 Docs preview for commit 45317a3 at: https://63ccf7d40fb8da632502fbec--fastapi.netlify.app |
|
@tiangolo Any idea when can we merge this? |
|
📝 Docs preview for commit 11fd1ce at: https://63eb512c20977663c11a0846--fastapi.netlify.app |
|
Does this PR need additional work or help? |
|
Copied code from #8428 (comment), thanks @guillaumegenthial |
| @@ -1,7 +1,7 @@ | |||
| from contextlib import asynccontextmanager as asynccontextmanager | |||
| from typing import AsyncGenerator, ContextManager, TypeVar | |||
|
|
|||
| import anyio | |||
| import anyio.to_thread | |||
contextmanager_in_threadpool
| assert str(last_frame.path) == __file__ | ||
| assert last_frame.lineno == raise_value_error.__code__.co_firstlineno |
There was a problem hiding this comment.
If you revert the changes, you can verify that the dependency will not be the last frame (or in any frame, because it will not be present).
There was a problem hiding this comment.
Awesome, great, I did this to verify. 🤓
contextmanager_in_threadpoolyield
yieldyield
There is a missing of traceback when an exception raised in contextmanager_in_threadpool
Happens on Python 3.11
I noticed the exception.traceback gets flushed after exiting the context manager.
There is an issue is opened #5740
Edit by @Kludex :