Document BaseHTTPMiddleware bugs

[Kludex]

• Closes Improve Documentation on Writing Custom ASGI Middleware and BaseHTTPMiddleware #1029

Unfortunately we have those bugs.

I think it would be a good idea to document them.

How does it look like

Questions

• Should we document how to create middlewares in plain ASGI?
• Should we link the issues for those bugs? Which ones are those... ?

[adriangb]

What about the copying of contextvars?

[erewok]

What about this comment from the original issue:

A warning not to use BaseHTTPMiddleware with StreamingResponse or FileResponse endpoints.

[Kludex]

How that warning would be triggered?

[Kludex]

What about the copying of contextvars?

Sure. Would you mind adding the suggestion?

[tomchristie]

Documenting limitations in this way seems like a good tactic, yup.

[Kludex]

How that warning would be triggered?

@erewok I'm going to merge this, but your reply on this is still appreciated. 🙏

[erewok]

My apologies @Kludex: I didn't see this until this morning. I have some opinions about BaseHTTPMiddleware which other people may not agree with, but they are as follows:

• In our Starlette projects at work, we have banned custom implementations and libraries based on this middleware class. An example of a library we would otherwise want to use would be starlette-prometheus which also suffers from the background-task issue.

• I believe that this base middleware class is the wrong abstraction in almost all cases because it encourages viewing your middleware operations as having complete Request and Response objects to work with, as opposed to all the custom middleware that ships with Starlette, such as the AuthenticationMiddleware and the ExceptionMiddleware, which have a __call__ method. This abstraction creates problems around async for or background types of operations.

• The problems with BaseHTTPMiddleware usually surface in three areas (unless some of these have been fixed, but I was under the impression that they are unfixable in the current form):

  • Force background tasks to evaluate (in the foreground) before responses are returned
  • Force streaming responses to be completely loaded into memory before responses are returned
  • Same as above for FileResponses.

Thus, when I think about documenting this middleware class, my own opinion is that users should be totally discouraged from using it and they should also be loudly notified of the above issues.

[Kludex]

How can we loudly notify of the above issues? Directly raising warning on the BaseHTTPMiddleware informing about them?

[erewok]

If it were up to me I'd probably do the following:

• deprecate this class and issue DeprecationWarning
• document all of the above issues in Starlette docs
• offer examples of making better middleware in Starlette docs
• include deprecation in changelog and probably major version bump

This may be drastic and out of step with what others would want to do, so I offer it as one Starlette user's opinion.

[Kludex]

Thanks for your opinion. 🙏

[Kludex]

I'm not sure if this is true... I'll need to come back to this.

This BaseHTTPMiddleware is a real challenge.

[abersheeran]

Maybe it hurts a lot of people to say that, but I don't think a web framework suitable for large projects should use too low abstraction... I like the design of Starlette so much that I even implemented a similar one for WSGI thing.

But I continue to practice, observe the starlette/fastapi community feedback and communicate with colleagues in the company. Maybe we should use only Request and Response objects inside the framework like Django or flask.

We can see that many of the problems stem from Starlette's direct use of ASGI internally. Like this issue.

[Kludex]

I'm not sure if I get what you say exactly. But if I do, you're implying on doing a major refactor on the code source?

[abersheeran]

I have no intention of doing a major refactoring of starlette, it's too much of an impact. But maybe we should change our positioning to be more of a toolkit than a framework.

Then transfer part of the work to fastapi.

For example, reimplement a Django-like Application in fastapi, so that only Request and Response are used in the entire framework.

[abersheeran]

In this way we can not only solve the BaseHTTPMiddleware problem, but also improve the other problems like #944.

[abersheeran]

I've been trying to fix BaseHTTPMiddleware for the past three or four days, and have looked at tons of other web frameworks. The results have been negative. 😞

Perhaps the above point is cowardly, but we can really think about the overall structure.

[Kludex]

I've been trying to fix BaseHTTPMiddleware for a long time as well. No need to feel ashamed about not succeeding... 😅 And I'll probably try again next weekend... 😞

But maybe we should change our positioning to be more of a toolkit than a framework.

You mean like https://github.com/klen/asgi-tools , right? I don't agree with that change of mindset, but you can create a discussion to see how others feel about it, if you want.

[erewok]

I actually think the problems with BaseHTTPMiddleware are terminal: it's not fixable.

I also disagree about what the problem is: I don't think Starlette is offering too many low-level tools but not encouraging developers to use them enough. For example of this, the other middleware shipped by Starlette works fine, and those all take the scope, receive, send args. Here, Starlette's layers-of-an-onion design where everything is an ASGI app that wraps another ASGI app (until you get to the center where a Request object is created) is both simple and makes a lot of sense in my opinion: even after all this time I can still hold almost the whole Starlette codebase in my head. If other libraries (such as the starlette-prometheus one I linked above) wrote their Middleware using this style, they'd probably be fine. We've written lots of our own Middleware using these patterns at work and our background tasks do run in the background: we don't have this issue.

I actually think we run into problems with the leaky abstractions in Async programming in Python in general when we start to work with the following:

• Streaming response bodies
• Background tasks

My suspicion is that to fix these problems you would likely have to spin up channels and separate the processing of requests -> responses, requests -> streaming responses, and background. This is probably where the refactoring effort should be placed (if there is to be one). A Starlette app should spin up a dedicated channel for background tasks and sending responses and then the Response class should route things to the proper channel.

The real problem in my opinion is ever thinking about a "complete" Request, Response, or Background when everything should be treated as streaming by. You can only map or filter over these things or send work to other channels. (My last preference here would be to leverage the kinds of tools that trio provides, so you get much improved channels, nested scoping, and cancellation over standard library async, but that would probably be a disruptive change for Starlette users.)

Again, I offer my opinions humbly as someone who has a lot of respect for the Starlette codebase, someone who sees this project as a very nice implementation of the Python ASGI spec and someone who's built a couple dozen projects at work using it.

[adriangb]

I also believe it's not fixable / not worth fixing and it was the wrong abstraction to begin with.

Something I'm wondering is: can we track down the packages that are using BaseHTTPMiddleware and make PRs to them to switch them over? That might have an even bigger impact on the community than docs and if we focus on just popular projects it wouldn't take that much time (I think).

separate the processing of requests -> responses, requests -> streaming responses, and background

I do think that we should be spinning up a lifespan scoped anyio.TaskGroup and pushing background tasks into there instead of processing them after sending the response but before returning from our ASGI coroutine.

But maybe we should change our positioning to be more of a toolkit than a framework
Then transfer part of the work to fastapi.

I do think there's a lot of things that Starlette offers right now that it could do without / could be transferred upstream or to other libraries. Off the top of my head:

• Config management
• Schema / OpenAPI stuff
• Authentication