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

allow middleware to catch any exception #1884

Open
kapilt opened this issue Feb 2, 2022 · 6 comments
Open

allow middleware to catch any exception #1884

kapilt opened this issue Feb 2, 2022 · 6 comments

Comments

@kapilt
Copy link
Contributor

kapilt commented Feb 2, 2022

as part of #1541 and #1549 middleware support for catching exceptions was added, but the interface to it is unwieldy as it requires all exceptions to subclass a specific base exception .. given use of any library code or larger applications this is rather difficult to accomodate in practice. Given we have a default global error handler that does the exact same thing as the view response, i think uncaught exceptions (minus chalice view error) should propagate by default so middleware has a chance to handle it, and the global error acts as a fallback.

@ervinne13
Copy link

Thank you, this works fine for now even if we need to subclass. To same people who wonder, it's actually ChaliceUnhandledError and not ChaliceUnhandledException (or at least that was in my case). I thought #1549 wasn't actually pushed or was reverted but I guess it was just mistyped by sir jamesls

@kapilt
Copy link
Contributor Author

kapilt commented Feb 22, 2022

Thank you, this works fine for now even if we need to subclass.

for you perhaps, but as i mentioned the underlying issue is handling exceptions from third party libraries which makes the current interface more than unwieldy, and effectively requires reimplementing the middleware mechanisms on chalice for developer experience (app exceptions bubble). the native lambda interface handles that well.. chalice not so much atm unless every app reimplements redundant top level handlers to rethrow everything as exception subclasses

@nmenardg-keeper
Copy link

the underlying issue is handling exceptions from third party libraries
Yep, exactly. We're using pydantic and we would've liked to use the middleware to manage the exceptions

@cloutierMat
Copy link

cloutierMat commented Jul 13, 2023

I am also looking for a solution for this issue.

Here is my current objective. I will be submitting a PR shortly if all seems right. The approach proposed is similar to the solution that already exists for other framework (ie Flask) and is trying to stay as close to the existing design of Chalice.

Problem

While the ChaliceUnhandledError works fine for custom errors, third party libraries can raise Exceptions that can't be subclassed. Having a try/except in a middleware won't successfully catch the Exception as it is already a Response at that point.

Goals

  • Exceptions on REST APIs must be intercepted before the InternalServerError response is built.
  • Must have knowledge of the context of the raised exception.
  • Must be able to return a Response
  • An invalid Response or an exception raised in the handler should still return the standard InternalServerError and have the stack trace replaced by the expected json object when not in debug mode

As existing middlewares can already successfully intercept all exceptions for other events, and ChaliceUnhandledError only exists in the scope of http event, so will the focus of this handler.

Specification

A new @app.errorhandler is to be added. The decorated function will take one arg, the exception.

Similar to the way middlewares can be registered, we will be able to register the error handler in app or in a blueprint with either a decorator or app.register_error.

# Decorator
@app.errorhandler(MyCustomError)
def my_handler(e: MyCustomError):
    return Response(body="", status_code:400)

# Register
app.register_error_handler(MyCustomError, my_handler)

# Blueprint
@my_blueprint.errorhandler(MyCustomError)
def my_handler(e: MyCustomError):
    return Response(body="", status_code:400)

In the examples above, any http endpoint raising MyCustomError will then return a 400 with no body.

If, however, the return value isn't a Response or an exception was raised during execution of the handler, the normal flow of the RestAPIEventHandler will be maintained.

@app.errorhandler(MyCustomError)
def my_handler(e: MyCustomError):
    return "Not a Response object"

Will return

{
    "Code": "InternalServerError",
    "Message": "An internal server error occurred."
}

@rosscdh-tpg
Copy link

Hey there any movement on this?

@nineclicks
Copy link

Kind of a hacky way around around this, but you have to add a decorator to all routes.

def error_wrapper(fn):
    def wrapper():
        try:
            return fn()
        except Exception as ex:
            print('Error wrapper')
            print(str(ex))
            return Response(
                {'result': 'error', 'error': 'server_error'},
                status_code=500,
            )
    return wrapper

@app.route('/api/something', methods=['GET'])
@error_wrapper
def do_something():
    raise ValueError('a fake error.')

Or instead of decorating each route, if you like to live dangerously:

def wrap_routes(app):
    for path, views in app.routes.items():
        for view in list(views.values()):
            print(f'Wrapping route {view.method} {path}, fn: {view.view_function.__name__}')
            view.view_function = error_wrapper(view.view_function)

wrap_routes(app)

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

6 participants