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

middleware causes exceptions to not be raised/handled silently (back again) #2625

Open
fraser-langton opened this issue Jun 19, 2024 · 11 comments · May be fixed by #2812
Open

middleware causes exceptions to not be raised/handled silently (back again) #2625

fraser-langton opened this issue Jun 19, 2024 · 11 comments · May be fixed by #2812

Comments

@fraser-langton
Copy link

fraser-langton commented Jun 19, 2024

Regression of #1976 #1977 #1609 #1940

This time I have noticed that changing MyExc(Exception) to MyExc(BaseException) means the error does get sent to stdout (if that helps) I tried to have a dig but I am not too sure where that catch exception is that is catching the Exception (and silently passing) and not the BaseException

starlette==0.37.2
fastapi==0.111.0

import uvicorn
from fastapi import FastAPI
from starlette.middleware.base import BaseHTTPMiddleware

app = FastAPI()


class MyExc(Exception):  # change to BaseException and then both exceptions are sent to stdout
    ...


@app.get("/info")
def info():
    # raises Exception as expected, the traceback is seen in console
    raise MyExc


private_api = FastAPI()


@private_api.get("/info")
def info():
    # exception is handled silently, no traceback is seen in console
    raise MyExc


app.mount("/private", private_api)


class Middleware(BaseHTTPMiddleware):
    async def dispatch(self, request, call_next):
        return await call_next(request)


app.add_middleware(Middleware)  # when this is removed, the exceptions are raised for all routes

if __name__ == "__main__":
    uvicorn.run(app, port=8000)

Important

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@daniilmastrangeli
Copy link

I have the same issue

@fraser-langton
Copy link
Author

fraser-langton commented Jun 27, 2024

@daniilmastrangeli a fix I have found for now is to add_exception_handler which catches the exception and if re-raised will display the trace to stdout as expected

import uvicorn
from fastapi import FastAPI, Request
from starlette.middleware.base import BaseHTTPMiddleware

app = FastAPI()


class MyExc(Exception):  
    ...


@app.get("/info")
def info():
    raise MyExc


private_api = FastAPI()


@private_api.get("/info")
def info():
    raise MyExc


app.mount("/private", private_api)


class Middleware(BaseHTTPMiddleware):
    async def dispatch(self, request, call_next):
        return await call_next(request)


def re_raise_exception(_: Request, exc: Exception):
    raise exc from exc


app.add_middleware(Middleware) 
app.add_exception_handler(Exception, re_raise_exception)  # raises the exception in the handler which results in getting handled by a different context

if __name__ == "__main__":
    uvicorn.run(app, port=8000)

@daniilmastrangeli
Copy link

@fraser-langton ty, it's worked

@b4stien
Copy link

b4stien commented Aug 6, 2024

The root cause is here: #2194 (comment).

For more references, see also: fastapi/fastapi#8577 (comment)

@Kludex
Copy link
Member

Kludex commented Sep 21, 2024

This regression happened on 0.29.0.

@Kludex
Copy link
Member

Kludex commented Sep 21, 2024

I've started some work on #2696.

@mike-schiller
Copy link

Is there a way to work around this for background tasks as well? The workaround functions for me when the exception occurs in my endpoint's code. But if the endpoint starts a background task and that task throws an exception, it gets lost.

@fraser-langton
Copy link
Author

@mike-schiller I haven't got one, didn't need it for background tasks personally

@adriangb
Copy link
Member

adriangb commented Dec 9, 2024

You can use https://github.com/adriangb/asgi-background which isn't impacted by this.

@mike-schiller
Copy link

Thanks @adriangb. I'll check that out. For the moment I implemented a decorator that I apply to all my background tasks to wrap them in try/except with logging.

@Kludex
Copy link
Member

Kludex commented Dec 26, 2024

Can someone test #2812, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants