DeanWay / fastapi-versioning

api versioning for fastapi web applications
MIT License
659 stars 64 forks source link

Bug for exception handler #30

Open andy-yu-y opened 3 years ago

andy-yu-y commented 3 years ago

Describe the bug When trying to define global exception handler, catching user defined exception fails with error "Caught handled exception, but response already started."

def custom_exception_handler( request: Request, exception: CustomNotFoundError ) -> JSONResponse: return JSONResponse( status_code=status.HTTP_404_NOT_FOUND, content={"message": f"Oops! {exception.name}"} )

exception_handlers = {CustomNotFoundError: custom_exception_handler}

app = FastAPI( title="Root app", version="0.1.0", exception_handlers=exception_handlers, ) app.include_router(api_router) app = VersionedFastAPI( app, version_format=f"{MAJOR_PLACEHOLDER}", prefix_format=f"/v{MAJOR_PLACEHOLDER}", default_version=(DEFAULT_MAJOR_VERSION, DEFAULT_MINOR_VERSION), exception_handlers=exception_handlers, )

To Reproduce Steps to reproduce the behavior:

  1. initialize app using VersionedFastAPI
  2. make a request to service that raise user defined exception CustomNotFoundError
  3. returns a Http status 500 with error "Caught handled exception, but response already started."

Expected behavior Global exception handler should catch CustomNotFoundError exception and return HTTP_404_NOT_FOUND with message

Additional context kwargs, should be passed to Fastapi in the constructor of VersionedFastAPI like below: versioned_app = FastAPI( title=app.title, description=app.description, version=semver, openapi_prefix=prefix, kwargs, )

bhandanyan-nomad commented 3 years ago

Experiencing the same. This workaround seems to fix it, though it's clearly not ideal:

app = VersionedFastAPI(app, ...)

for sub_app in app.routes:
    if hasattr(sub_app.app, "add_exception_handler"):
        sub_app.app.add_exception_handler(CustomNotFoundError, custom_exception_handler)
andy-yu-y commented 3 years ago

I think it will be good to solve it in the coming version. Do you have any plan for it?

rmerren commented 3 years ago

The solution from bhandanyan-nomad above solved the problem for me, but I adapted it to be a little more automatic. In his version, you need to explicitly list the exception handlers to add. This adaptation gets the list of existing exception handlers from the original and applies them all to the subapps. This way I can be lazy and not have to specifically keep track of my exception handlers.

handlers_to_apply = {}
for ex, func in app.exception_handlers.items():
    handlers_to_apply[ex]=func

app = VersionedFastAPI(app, ...)

for sub_app in app.routes:
    if hasattr(sub_app.app, "add_exception_handler"):
        for ex, func in handlers_to_apply.items():
            sub_app.app.add_exception_handler(ex, func)

I would also like to see this resolved in future versions...

thrix commented 3 years ago

+1 we are hitting this also

avinashjoshi commented 2 years ago

+1 for this issue. Could be related to #40

ysnghr commented 2 years ago

+1 I hope it will be good to solve it in the coming version.

cabudies commented 2 years ago

add_exception_handler

@tijptjik rmerren

Can you show the preview of the folder structure.. I'm trying to implement the same however it's not allowing me. I'm currently having routes specified in different folders as compared to my main file.

Butterweck commented 2 years ago

@bhandanyan-nomad thanks so much for your solution, this just cost me 1.5 days of my life :D

JenySadadia commented 1 year ago

+1 I am encountering the same issue.

vdymna commented 1 year ago

+1 We are encountering the same issue as well.