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

✨ Update internal AsyncExitStack to fix context for dependencies with yield #4575

Merged
merged 12 commits into from Feb 17, 2022

Conversation

tiangolo
Copy link
Owner

@tiangolo tiangolo commented Feb 14, 2022

This also allows catching exceptions like HTTPException inside of dependencies with yield. ✨


Dependencies with yield are converted to asynccontextmanagers internally and then are run with AsyncExitStack.

But before this change, the AsyncExitStack was created on the application's __call__(), right before custom user middlewares.

But custom user middlewares are created using a new AnyIO task group, and that creates a new contextvars context.

The call to enter the asynccontextmanager is done when calling the internal app, inside of the custom middlewares. But the call to exit the AsyncExitStack is done outside, so, the exit code of dependencies with yield (the code after yield) was run in a different context than the code above yield.

Because of this, it was not possible to set a context variable in a dependency with yield before yield and then reset it or use its value afterwards:

from contextvars import ContextVar
from typing import Any, Dict, Optional


legacy_request_state_context_var: ContextVar[Optional[Dict[str, Any]]] = ContextVar(
    "legacy_request_state_context_var", default=None
)

async def set_up_request_state_dependency():
    request_state = {"user": "deadpond"}
    contextvar_token = legacy_request_state_context_var.set(request_state)
    yield request_state
    legacy_request_state_context_var.reset(contextvar_token)

...that example would throw an error at legacy_request_state_context_var.reset(contextvar_token).

This PR sets up the AsyncExitStack in a new middleware that is run inside of the custom user middlewares and is run directly, without a new task group. This way the same contextvar context used to enter the asynccontextmanager created for a dependency with yield is also used to run the exit code (after yield) of the dependency.


Note: contextvars are very powerful, but also complex and the way they work can be a bit magical and hard to debug. If you can, try not to use them, avoid them, pass parameters to functions directly, and save contextvars for only the advanced use cases that really need them.

An example of an advanced use case that would need contextvars is if you are migrating from Flask to FastAPI (or anything else) and you are using Flask's g semi-global variable to store data. And then you use that g in some function hidden deep down, and passing parameters directly through all the functions up to that point would be a refactor so big and cumbersome that would prevent the migration entirely. In that case, you can replace g with a context var, independent of Flask. This would apply to a migration to FastAPI or to anything else. In the case of FastAPI, you could set any of that g data in a context variable instead, in a dependency with yield, and then use the context variable in that function deep down instead of Flask's g object. You can also do this with Flask, and that would make your internal function independent of Flask.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 14, 2022

@github-actions
Copy link
Contributor

📝 Docs preview for commit 017c235 at: https://620a9717c1c59f5273ea6a49--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit cea488d at: https://620a981053e73a57a147c75e--fastapi.netlify.app

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #4575 (a69d916) into master (78b07cb) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            master     #4575    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          522       525     +3     
  Lines        13136     13281   +145     
==========================================
+ Hits         13136     13281   +145     
Impacted Files Coverage Δ
fastapi/applications.py 100.00% <100.00%> (ø)
fastapi/middleware/asyncexitstack.py 100.00% <100.00%> (ø)
tests/test_dependency_contextmanager.py 100.00% <100.00%> (ø)
tests/test_dependency_contextvars.py 100.00% <100.00%> (ø)
tests/test_dependency_normal_exceptions.py 100.00% <100.00%> (ø)
tests/test_exception_handlers.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78b07cb...a69d916. Read the comment docs.

@github-actions
Copy link
Contributor

📝 Docs preview for commit 4531f6a at: https://620e3a9379f92813c67853d1--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit a69d916 at: https://620e41e363bc9107a4202168--fastapi.netlify.app

self.context_name = context_name

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if AsyncExitStack:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that AsyncExitStack is always true, it’s imported and concurrency provides it from either of two locations, but it’s always provided.

Copy link
Sponsor Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... 🤔

@fortzi
Copy link

fortzi commented Sep 19, 2022

Is it (or will it be) possible to access the contextvars in the view that depends on said dependency? For example:

def my_dep():
    # Define some context var here
    yield

@router.get('/', dependencies=[Depends(my_dep)])
def view():
    # Use context var set in my_dep

In my case It's done for carrying log context fields (using structlog)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants