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

HTTPMiddleware breaks BackgroundTasks in the same connection #2093

Closed
1 task done
Molkree opened this issue Mar 23, 2023 · 12 comments · Fixed by #2194
Closed
1 task done

HTTPMiddleware breaks BackgroundTasks in the same connection #2093

Molkree opened this issue Mar 23, 2023 · 12 comments · Fixed by #2194
Labels
bug Something isn't working

Comments

@Molkree
Copy link

Molkree commented Mar 23, 2023

Consider the following simple example of background tasks through Starlette:
server.py

import logging
from time import sleep

from starlette.applications import Starlette
from starlette.background import BackgroundTasks
from starlette.middleware import Middleware
from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
from starlette.requests import Request
from starlette.responses import JSONResponse, Response
from starlette.routing import Route

logging.basicConfig(
    level=logging.INFO,
    format="[%(asctime)s] [%(levelname)s] %(message)s",
    datefmt="%Y-%m-%d %H:%M:%S",
)


class CustomMiddleware(BaseHTTPMiddleware):
    async def dispatch(
        self, request: Request, call_next: RequestResponseEndpoint
    ) -> Response:
        return await call_next(request)


def create_task(request: Request) -> JSONResponse:
    logging.info("Entered POST method")
    tasks = BackgroundTasks()
    tasks.add_task(background_task)
    message = {"status": "Task created"}
    return JSONResponse(message, background=tasks, status_code=201)


def background_task() -> None:
    logging.info("Started background task")
    sleep(10)
    logging.info("Finished background task")


routes = [Route("/task", endpoint=create_task, methods=["POST"])]
middleware = [Middleware(CustomMiddleware)]
app = Starlette(routes=routes, middleware=middleware)

I start this code with uvicorn server:app --reload.

Then I start the task twice in one connection using this code:
client.py

from httpx import Client

with Client(base_url="http://localhost:8000") as client:
    client.post("/task")
    client.post("/task")

Which results in a sequential execution of background tasks (and read timeout from the client as a consequence):

[2023-03-23 19:32:08] [INFO] Entered POST method
INFO:     127.0.0.1:51448 - "POST /task HTTP/1.1" 201 Created
[2023-03-23 19:32:08] [INFO] Started background task
[2023-03-23 19:32:18] [INFO] Finished background task
[2023-03-23 19:32:18] [INFO] Entered POST method
[2023-03-23 19:32:18] [INFO] Started background task
[2023-03-23 19:32:29] [INFO] Finished background task

If I just send simple curl requests (curl -X 'POST' 'http://127.0.0.1:8000/task') one after the other (so they are different connections), everything works as expected:

[2023-03-23 19:44:35] [INFO] Entered POST method
INFO:     127.0.0.1:55528 - "POST /task HTTP/1.1" 201 Created
[2023-03-23 19:44:35] [INFO] Started background task
[2023-03-23 19:44:37] [INFO] Entered POST method
INFO:     127.0.0.1:55532 - "POST /task HTTP/1.1" 201 Created
[2023-03-23 19:44:37] [INFO] Started background task
[2023-03-23 19:44:45] [INFO] Finished background task
[2023-03-23 19:44:47] [INFO] Finished background task

It looks like this regression was introduced in Starlette 0.23.1, this PR. If I pin Starlette to 0.23.0 everything works fine.

@Kludex
Copy link
Member

Kludex commented Mar 23, 2023

Thanks for the report @Molkree 👍

@MO-CTO
Copy link

MO-CTO commented Apr 25, 2023

I have experienced the same issue. It took me a wile to figure out this was the issue. Only after establishing that the move from Starlette 0.23.0 to 0.23.1 partially broke my FastApi application was I able to find these posts as an explanation of what was going on.

I have been able to resolve it by ditching my @api.middleware('http') middelware and rewriting it as pure ASGI middleware instead, following the documentation here: https://www.starlette.io/middleware/#writing-pure-asgi-middleware.
Hope this helps.

@jsg921019
Copy link

@Kludex Is it ok to PR regarding this issue for a review?

@Kludex
Copy link
Member

Kludex commented Apr 26, 2023

You mean that you want to create a PR? Yeah, go ahead.

@nikita-b
Copy link

Is anyone working on this issue? It's quite important for us and I think whether it is necessary to fix it myself or someone is already doing it?

@Kludex Kludex added the bug Something isn't working label May 24, 2023
@Kludex
Copy link
Member

Kludex commented May 24, 2023

@nikita-b No one is working on this issue. Go ahead. 🙏

@Kludex
Copy link
Member

Kludex commented Jun 1, 2023

I need this for the next release.

@Kludex Kludex added this to the Version 0.28.0 milestone Jun 1, 2023
@Kludex
Copy link
Member

Kludex commented Jun 1, 2023

It looks like this was the old behavior, then I fix it without knowing in #1609, then I broke it again on #1940.

@Kludex
Copy link
Member

Kludex commented Jun 1, 2023

I'm not sure if we can fix this without breaking what was previously fixed.

@Kludex Kludex removed this from the Version 0.28.0 milestone Jun 1, 2023
@Kludex Kludex added need confirmation This issue needs confirmation. and removed bug Something isn't working labels Jun 1, 2023
@adriangb
Copy link
Member

adriangb commented Jun 5, 2023

I’m going to check what the behavior is like under #1700, maybe that’s a viable fix

@adriangb
Copy link
Member

adriangb commented Jun 6, 2023

I can confirm that #1700 fixes this. I'll re-open it.

@Kludex
Copy link
Member

Kludex commented Jun 13, 2023

I think we should try to find the cause first, and try to find a solution that just fixes the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants