adriangb / xpresso

A composable Python ASGI web framework
https://xpresso-api.dev/
MIT License
178 stars 4 forks source link

feat: run dependencies even when no route matches #66

Open adriangb opened 2 years ago

adriangb commented 2 years ago

Currently the following test fails:

def test_app_dependency_no_route_match() -> None:
    log: List[str] = []

    async def dep() -> None:
        log.append("called")

    app = App(routes=[], dependencies=[Depends(dep)])

    client = TestClient(app=app)

    resp = client.get("/")
    assert resp.status_code == 404, resp.content
    assert log == ["called"]

The reason for this is that the dependency DAG is not executed until a route is found. The same applies to FastAPI.

This is a bit confusing, and prevents implementing logging, tracing, etc. as a dependency (usually you'd want to log 404s).

I think it should be possible to change this: we'd run each App's and Rotuer's dependencies as the request traverses through them. It'd be roughly the same as what we are doing in Operation:

https://github.com/adriangb/xpresso/blob/bf71495c8050af574810bd13b926cb8050ea8a59/xpresso/routing/operation.py#L50-L58

Ideally this change would mean that anything you do with ASGI middleware you can do with dependencies (there's still a good reason to support both: there is generic ASGI middleware but not generic Xpresso dependencies).

The main con is that I expect this to have a relatively large impact on performance and make the implementation a lot messier. These are two very good arguments for not supporting this usage.

adriangb commented 2 years ago

On a related note, this could maybe address the issues in https://github.com/tiangolo/fastapi/releases/tag/0.74.0 if we change our execution model somehow. I'll have to think about it a bit.

adriangb commented 2 years ago

I'm thinking what might work is for Router and Path to build their own dependency graph (all of the dependencies of the routing nodes traversed to get to them). Then if Router doesn't match any routes it can run its own dependency graph, if Path doesn't match any methods (i.e. if it returns a PartialMatch and then handle gets called) it can run its dependency graph and otherwise Operations dependency graph will be called. But this feels like "special casing", which I don't particularly like. But maybe those are the only cases, so it's okay. The other routing primitives that Starlette provides (Mount and Host) don't raise exceptions, they either match or they don't. And I haven't seen any 3rd party routing primitivities that might be incompatible.

adriangb commented 2 years ago

So after a bit of testing, it looks like https://github.com/adriangb/xpresso/issues/66#issuecomment-1079967427 will work if we want it to, but there is a big con: we need to dive into the routing and HTTP method matching, which are currently things that Starlette just handles, and provides no real hooks into. So this would mean a lot more code in Xpresso 😞