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

Should server shutdown after receiving "lifespan.shutdown.failed"? #2308

Open
euri10 opened this issue Apr 16, 2024 Discussed in #2298 · 4 comments · May be fixed by #2309
Open

Should server shutdown after receiving "lifespan.shutdown.failed"? #2308

euri10 opened this issue Apr 16, 2024 Discussed in #2298 · 4 comments · May be fixed by #2309

Comments

@euri10
Copy link
Member

euri10 commented Apr 16, 2024

Discussed in #2298

Originally posted by peterschutt April 5, 2024
If the shutdown failure message isn't initiated by the app receiving a "lifespan.shutdown" from uvicorn, then the app continues to run after the "lifespan.shutdown.failure" message is received.

Reproducer:

import asyncio
import contextlib
from collections.abc import AsyncIterator

import anyio
import uvicorn
from starlette.applications import Starlette
from starlette.responses import JSONResponse
from starlette.routing import Route


async def homepage(_) -> JSONResponse:
    return JSONResponse({'hello': 'world'})


async def sleep_and_raise() -> None:
    await asyncio.sleep(1)
    raise RuntimeError("An error occurred")


@contextlib.asynccontextmanager
async def lifespan(_: Starlette) -> AsyncIterator[None]:
    async with contextlib.AsyncExitStack() as stack:
        tg = await stack.enter_async_context(anyio.create_task_group())
        tg.start_soon(sleep_and_raise)
        yield


app = Starlette(debug=True, routes=[Route('/', homepage)], lifespan=lifespan)


if __name__ == "__main__":
    uvicorn.run(app, lifespan="on")

After the error occurs in the lifespan task, the app continues to serve:

image

Given that apps like starlette and litestar encourage use of the lifespan context for orchestration of things that should have a lifespan equivalent to the application object, then I think it would make sense for the app to stop if something has failed within that lifespan after the app has sent "startup.complete" but before the server has sent "shutdown" to the app.

The spec seems to agree:

If a server sees this it should log/print the message provided and then terminate.

[!IMPORTANT]

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@euri10
Copy link
Member Author

euri10 commented Apr 16, 2024

the issue and patch seems legit to me, can you submit a PR @peterschutt ?

@peterschutt
Copy link

Thanks @euri10 - I'll polish up the patch and submit a PR.

@peterschutt peterschutt linked a pull request Apr 17, 2024 that will close this issue
3 tasks
@Kludex
Copy link
Sponsor Member

Kludex commented Apr 17, 2024

I don't think this is a bug. My interpretation from the spec is that once the "lifespan.startup.complete" is sent, your application is the only task running.

Also, this is the same behavior as the other ASGI servers.

I think what you want is something like this:

import anyio
from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import JSONResponse
from starlette.routing import Route

import uvicorn


async def infinite_task():
    while True:
        await anyio.sleep(1)
        print("I am alive!")


async def health(request: Request):
    return JSONResponse({"status": "ok"})


app = Starlette(routes=[Route("/health", health)])


async def main():
    async with anyio.create_task_group() as tg:
        tg.start_soon(infinite_task)

        config = uvicorn.Config(app=app, host="0.0.0.0", port=8002)
        server = uvicorn.Server(config=config)
        await server.serve()


if __name__ == "__main__":
    anyio.run(main, backend_options={"use_uvloop": True})

@peterschutt
Copy link

Thanks @Kludex - that pattern will work just fine!

once the "lifespan.startup.complete" is sent, your application is the only task running.

Do you feel there is a disconnect between the spec, and how some frameworks advertise lifespan? For example, from starlette's docs (ref):

Consider using anyio.create_task_group() for managing asynchronous tasks.

And from litestar (ref):

This can be useful when dealing with long running tasks

And lastly, from a framework perspective, can we do any better than returning the "lifespan.shutdown.failed" message, in the case where an exception is caught within the lifespan?

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 a pull request may close this issue.

3 participants