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

supertokens FastAPI middleware uses BaseHTTPMiddleware which breaks async/background tasks #343

Open
atambo opened this issue Jun 10, 2023 · 7 comments

Comments

@atambo
Copy link

atambo commented Jun 10, 2023

See the starlette issue as well as the limitations section of the documentation.

It seems like using BaseHTTPMiddleware breaks some things with async/background tasks in FastAPI and people generally recommend switching your middleware to be pure ASGI middleware for maximum compatibility.

Unfortunately, I've been noticing this issue ever since I integrated the supertokens middleware into my application.

@rishabhpoddar
Copy link
Contributor

Thanks for opening this issue @atambo . We will have a look at this in the coming week.

@atambo
Copy link
Author

atambo commented Jun 12, 2023

Seems like starlette was thinking about deprecating BaseHTTPMiddleware but ended up backing off from doing so:
encode/starlette#1898

However, it still seems like the recommended approach is to use a pure ASGI middleware.

@KShivendu
Copy link
Contributor

KShivendu commented Jun 13, 2023

Hi @atambo, Thanks for pointing this out. I quickly tried to replace BaseHTTPMiddleware. But it was getting slightly complicated. Starlette is on a lower level and the objects don't seem easily convertable into each other (esp. Message <-> Response). So we will take a look at this when at a later point in time.

Keeping this issue open since it's valid. We will try to get resolve this as soon as we have time. In the meantime, I can help you with finding a workaround for your use case if you provide more details :)

@AngusParsonson
Copy link

Hi, this is causing issues for me currently, I'd like to run an async function in a BackgroundTask but having the SuperTokens middleware is blocking my main event loop. I need something like the following:

import asyncio
from fastapi import BackgroundTasks, FastAPI
from fastapi import FastAPI
from supertokens_python import init, InputAppInfo, SupertokensConfig
from supertokens_python.framework.fastapi import get_middleware
from supertokens_python.recipe import dashboard, emailpassword, session, userroles


app = FastAPI()

app.add_middleware(get_middleware())

init(
    app_info=InputAppInfo(
        app_name="name",
        api_domain="localhost",
        website_domain="localhost",
        api_base_path="/auth",
        website_base_path="/auth",
    ),
    supertokens_config=SupertokensConfig(
        connection_uri= "uri",
        api_key= "key",
    ),
    framework="fastapi",
    recipe_list=[
        session.init(),
        userroles.init(),
        dashboard.init(),
        emailpassword.init(),
    ],
    mode="asgi",
)

@app.get("/")
async def root():
    return {"message": "Hello World"}

async def simple_task():
    await asyncio.sleep(10)
    print("completed")

@app.post("/test")
async def test(background_tasks: BackgroundTasks):
    background_tasks.add_task(simple_task)
    return {"message": "Task started"}

@KShivendu
Copy link
Contributor

KShivendu commented Aug 2, 2023

@AngusParsonson I tried your code, it seems to be working for me and is able to run the async tasks. What's the error that your're seeing on your machine?

@AngusParsonson
Copy link

Hi @KShivendu, thanks for the response.

There is no error, it is just that the main event loop is blocked whilst the task is running. If you try to hit a simple get endpoint whilst the background task is running, then it will not complete until the background task is finished.

This is an issue with starlette, the way I got around it was by using asyncio.create_task() instead of the background task.

@KShivendu
Copy link
Contributor

KShivendu commented Aug 8, 2023

Ahh yes. I'll try to dive into this and find a clean solution sometime soon.

Good to know that you already found a workaround in the meantime. I believe it's sufficient to solve your problem for now?

@KShivendu KShivendu removed their assignment Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants