BaseHTTPMiddleware structure and StreamingResponse()

[usama-elm]

Hello the Starlette community.
A few weeks back I built a FastAPI Middleware for caching responses using decorators and such (please hold and keep reading, it is relevant I promise it).

FastAPI in many areas being a mere Starlette wrapper I had to contend with reading and building code atop Starlette and not FastAPI really. As the code is somewhat late in versions I built the working code, and then when we updated Starlette as the necessarydependance I noticed something interesting in Starlette's recent codebase changes, specifically in the middleware.base file. There's a new _StreamingResponse type alongside the existing StreamingResponse. This caught my attention because it broke my Response detection and caching mechanism.

Old code:

    async def _prepare_response_for_caching_by_type(
        self,
        response: Response,
        cache_key: str,
        expiry: int,
    ) -> Response:
        content: bytes | None = None
        response_type: CacheResponseType | None = None
        background_tasks = BackgroundTasks()
        match response:
            case StreamingResponse():
                content = b"".join(
                    [
                        chunk.encode("utf-8") if isinstance(chunk, str) else chunk
                        async for chunk in response.body_iterator
                    ]
                )
     ...

New working code:

    async def _prepare_response_for_caching_by_type(
        self,
        response: Response,
        cache_key: str,
        expiry: int,
    ) -> Response:
        content: bytes | None = None
        response_type: CacheResponseType | None = None
        background_tasks = BackgroundTasks()
        match response:
            case StreamingResponse() | _StreamingResponse():
                content = b"".join(
                    [
                        chunk.encode("utf-8") if isinstance(chunk, str) else chunk
                        async for chunk in response.body_iterator
                    ]
                )
     ...

In fact I could drop the whole StreamingResponse() and just use the new one but I kept both for compatibility.

I wanted to understand the changed, why did the Response and why the duplication of code, wouldn't just inheriting from Response avoid this duplication of code, am I missing a piece of context or the inheritance structure could be improved ?

There has already been a discussion around it #2731 and d771bb7#r146823236 but remains unanswered.

[adriangb]

I answered this in d771bb7#r146823236: you can't assume the thing returned from a function that declares to return a Response will be a specific subclass of Response (StreamingResponse). _StreamingResponse does inherit from Response.

[adriangb]

You also can't assume that you know every single type of Response subclass in your app... that's hard to guarantee. It's not a closed set. Anyone (a middleware you depend on, etc.) could come up with a new class at any time.

[usama-elm]

I understand that but I am checking in case that the type of Reponse given for the function that declares to return Response is a subclass StreamingResponse and applying middleware caching, because it is a special (async) type of response I need to treat and parse. For the rest of cases I just ignore it (which is why I have used match)

The only thing is just like you mentioned in the comments, the abstraction and hierarchy of Response seems like a poor one.

[adriangb]

Some of your pain may also be coming from trying to cache an async stream. That's a difficult problem in and of itself.