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

[feature request] support ContextVars #45

Closed
dimaqq opened this issue Nov 26, 2021 · 9 comments
Closed

[feature request] support ContextVars #45

dimaqq opened this issue Nov 26, 2021 · 9 comments
Labels
question Further information is requested

Comments

@dimaqq
Copy link

dimaqq commented Nov 26, 2021

It seem that setting context variables in on_startup is not supported.

My naïve understanding is that there needs to be a new (copied) context in which all of on_startup, asgi request and then on_shutdown are ran.

Otherwise, the value that's been set is not accessible in the request handlers and/or the context var cannot be reset in the shutdown function using the token that was generated in the startup function.

@florimondmanca
Copy link
Owner

florimondmanca commented Nov 27, 2021

Hi,

Would this be a ASGI server/framework-dependant issue instead? Or do you mean that a no-server, plain asyncio code example produces this behavior as well?

Asking because I think I saw a similar discussion over either Uvicorn or Starlette repos — mostly likely Starlette, which would run startup handlers in a long running lifespan task, and handle requests in another task, leading to different contexts and more broadly a lack of contextvars support for lifespan in Starlette.

@dimaqq
Copy link
Author

dimaqq commented Nov 27, 2021

@florimondmanca I think you may be right, I'm using fastapi/starlette/uvicorn and andeed my context var setup needs tricky context saving/switching because lifespan is a separate call (from middleware's point of view).

@florimondmanca
Copy link
Owner

Right. I think that’s the current state of things, so gently going to close this as being not related to ASGI-lifespan per se. Thanks :)

@dimaqq
Copy link
Author

dimaqq commented Dec 2, 2021

I'm pretty sure I know what happens now:

I made a middelware that listens to lifespan events, and I'm using with fastapi/starlette/uvicorn.
It works fine in that config.

However, in the tests, I'm trying to use the context manager from this project.
And that does not work for me.

Key difference is that I await something in my lifespan hook.

Both call the middleware __init__ the same,
Both call the middleware __call__ multiple times,

fastapi/starlette/uvicorn waits for lifespan.startup processing to finish.
asgi-lifespan does not wait for lifespan.startup processing to finish.

This library kindof assumes that middleware lifespan processing is synchronous.
And in my code it's not.

Thus test requests arrive before my custom thing is actually set up.

@florimondmanca
Copy link
Owner

florimondmanca commented Dec 5, 2021

This library kindof assumes that middleware lifespan processing is synchronous.

I'm not sure this is accurate, as we do await for an event that's set once we receive lifespan.startup.complete, right?

async def startup(self) -> None:
await self._receive_queue.put({"type": "lifespan.startup"})
await self._concurrency_backend.run_and_fail_after(
self.startup_timeout, self._startup_complete.wait
)

I think at this point sharing a bit of minimal reproduction code would be super helpful to see if there's anything we should be doing differently.

In particular, you're mentioning "lifespan hook". asgi-lifespan was made at a time when Starlette only had on_startup/on_shutdown parameters, and now there's a newer lifespan=<async gen> style for which maybe we need to change something slightly to fully support?

@florimondmanca florimondmanca reopened this Dec 5, 2021
@florimondmanca florimondmanca added the question Further information is requested label Dec 5, 2021
@dimaqq
Copy link
Author

dimaqq commented Dec 6, 2021

🤔 the code appears to do the right thing...
starlette lifecycle is indeed quite peculiar, and as @ojii pointed out (separately) my middleware may not be doing it right.
another possibility is that there's a subtle bug somewhere

@jpgxs
Copy link

jpgxs commented May 29, 2022

Hi, I've also been running into this issue 😢

I haven't dug into the code yet but it seems that startup and shutdown are being invoked in different contexts.

With this minimal example:

from fastapi import FastAPI
from contextvars import ContextVar

api = FastAPI()
ctx_var = ContextVar('mycontextvar')

async def lifespan(*args):
    token = ctx_var.set(True)
    try:
        yield
    finally:
        ctx_var.reset(token)

api.router.lifespan_context = lifespan

The below exception occurs:

unhandled exception during asyncio.run() shutdown
task: <Task finished name='Task-6' coro=<<async_generator_athrow without __name__>()> exception=ValueError("<Token var=<ContextVar name='mycontextvar' at 0x7fe40ec69e50> at 0x7fe40ec78680> was created in a different Context")>
Traceback (most recent call last):
  File "api.py", line 73, in lifespan
    yield
asyncio.exceptions.CancelledError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "api.py", line 75, in lifespan
    ctx_var.reset(token)
ValueError: <Token var=<ContextVar name='mycontextvar' at 0x7fe40ec69e50> at 0x7fe40ec78680> was created in a different Context

@florimondmanca
Copy link
Owner

A few questions to get the investigation going…

  • Is there an even simpler example to allow us to reproduce this? Does it reproduce without Starlette? (Manual ASGI lifespan app with this async context manager lifespan handler)
  • Does it reproduce on other ASGI servers? Hypercorn?

If we know in what happens in these situations then we may be able to pinpoint where the problem might be.

Now that I think about it, I said at the beginning of this thread that lifespan is a long running task, but more precisely I believe that’s how Uvicorn does it. Still, this « token was created in a different Context » error seems different, even odd. I looked up Starlette code, and the lifespan_context there runs as a single async with call, no strange task stuff going on. Which strengthens my belief that we might want to look at how Uvicorn runs lifespan..

@florimondmanca
Copy link
Owner

florimondmanca commented Mar 28, 2023

It seems like ASGI lifespan state allows addressing this use case, by allowing to store state initialized in the lifespan task for use in other places in the app.

We just merged #57 which adds support for it in this package, so I'll close this now!

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

No branches or pull requests

3 participants