fix for invalid ASGI 2.0 event handling

[joshua-auchincloss]

Hello, I'm hoping to start a discussion to make the following changes to ASGI event handling in Starlette:

Refactor scope assertion to only check the event category (lifecycle, ws, http)

Rationale

Starlette (incorrectly) asserts lifecycle scope is in lifecycle, ws, or http. This conflicts with the spec directly, where valid lifecycle startup events received by the application include lifecycle.startup, lifecycle.shutdown. This is fixed with the following amendment to routing.py:

# line 696/697
    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
        """
        The main entry point to the Router class.
        """
        assert "type" in scope
        entry = scope["type"].split(".")[0]
        assert entry in ("http", "websocket", "lifespan")

        if "router" not in scope:
            scope["router"] = self

        if entry == "lifespan":
            await self.lifespan(scope, receive, send)
            return

Remove un-necessary await receive() in lifecycle events (routing.py)

Rationale

Remove restriction for server to send Receive awaitable during startup events (allow for None case). Raises RuntimeError when this is completely unused (and called twice in the same handler)

# line 668
    async def lifespan(self, scope: Scope, receive: Receive, send: Send) -> None:
        """
        Handle ASGI lifespan messages, which allows us to manage application
        startup and shutdown events.
        """
        started = False
        app: typing.Any = scope.get("app")
        # await receive() <- remove this
        try:
            async with self.lifespan_context(app) as maybe_state:
                if maybe_state is not None:
                    if "state" not in scope:
                        raise RuntimeError(
                            'The server does not support "state" in the lifespan scope.'
                        )
                    scope["state"].update(maybe_state)
                await send({"type": "lifespan.startup.complete"})
                started = True
                # await receive() <- remove this
        except BaseException:
            exc_text = traceback.format_exc()
            if started:
                await send({"type": "lifespan.shutdown.failed", "message": exc_text})
            else:
                await send({"type": "lifespan.startup.failed", "message": exc_text})
            raise
        else:
            await send({"type": "lifespan.shutdown.complete"})

Let me know thoughts and I'll work on a PR with these changes.

[joshua-auchincloss]

update: the receive() method is integrated with the lifecycle call in starlette tests, thus it is unclear whether that is a breaking change. PR #2173 has a draft for the scope assertion changes

[Kludex]

I don't understand what is wrong.

[joshua-auchincloss]

The spec expects the application to be able to receive "lifespan.startup" and "lifespan.shutdown" as type keys, therefore the assertion that scope.type be in lifespan, http, or ws is incorrect and raises runtime errors if anything other than anonymous lifespan is sent. From the spec directly "Nonetheless applications should be cooperative by properly naming their keys such that they will not collide with other frameworks or middleware." - link. Starlette contradicts this through the above assertion at the start of all __call__ attempts - hope that makes sense.

[joshua-auchincloss]

Actually nevermind, closing this as it was my own misinterpretation of the spec

[Kludex]

Thanks 🙏