Chalice: REST API exception handling through middleware

[michaeldimchuk]

I'm using Chalice for both Lambda events and as a REST API, with a consistent error response structure used for both of those. The error response structure is different from the default one provided through ChaliceViewError as we have extra requirements to be passed down to the caller.

Right now I'm using decorators on functions to try/catch handlers and convert to a response structure, but this is becoming unwieldy as the amount of endpoints grows. Middleware seemed like a good solution for this as a "register and forget" approach, and works perfectly for Lambda events, just not REST APIs.

The main issue comes from chalice.app.RestAPIEventHandler._get_view_function_response , which swallows any unknown Exception and converts it into a chalice.app.Response to be returned to the caller, which happens to be my error handling middleware. With the exception information gone, there isn't really any easy way to create and return a custom error response without doing overly complicated parsing of the response body.

One solution I have found was to have my custom exceptions extend BaseException instead of Exception, bypassing the except Exception in _get_view_function_response and allowing for it to be propagated. Extending that type doesn't seem to be the recommended way to create an exception, and my code would have to deal with two root types of exceptions, Exception and the custom one which extends BaseException.

Based on what I've seen of the code, an easy fix would be to have a custom ChaliceUnhandledError which is caught by chalice.app.RestAPIEventHandler._get_view_function_response and re-thrown without any extra operations. This should still be backwards compatible for REST APIs and would work just fine for Lambda events, but I'm not sure about others as I haven't looked too deeply into it.

Any thoughts on the above proposal or an alternative approach to building custom error responses without having to add decorators on every function?

[jamesls]

I think the proposal makes sense. What would the expected behavior be if no middleware catches/handles the ChaliceUnhandledError?

In that case, I would expect that for Rest APIs we'd still adhere to the guarantee that any uncaught exceptions will return a generic 500 error response back to the caller (so we don't leak any internal details) and that in debug we'll send the stack trace back to the user. Thoughts?

[michaeldimchuk]

We could actually use middleware as a solution for that as well. The contents of the except Exception block in _get_view_function_response could be extracted into a function and a global error handler could be defined that uses that in case all else fails. Would look something like this:

def _get_unhandled_exception_response(self, view_function):
    headers = {}
    self.log.error("Caught exception for %s", view_function,
                   exc_info=True)
    if self.debug:
        # If the user has turned on debug mode,
        # we'll let the original exception propagate so
        # they get more information about what went wrong.
        stack_trace = ''.join(traceback.format_exc())
        body = stack_trace
        headers['Content-Type'] = 'text/plain'
    else:
        body = {'Code': 'InternalServerError',
                'Message': 'An internal server error occurred.'}
    return Response(body=body, headers=headers, status_code=500)

def _get_global_error_handler(self, view_function):
    def global_error_handler(request, get_response):
        try:
            return get_response(request)
        except Exception:
            return self._get_unhandled_exception_response(view_function)
    return global_error_handler

The middleware returned by _get_global_error_handler can then be injected as the first element in the _middleware_handlers list, so not only will ChaliceUnhandledError be caught, but any errors thrown by any middleware will also be handled. I don't know if it was an intentional design element, but at least in 1.20.0 if an error is thrown in a middleware function, the response from the REST API with debug enabled is just this: {'message': 'Internal server error'}.

[jamesls]

Ok yeah I think that could work, let me take a crack at that and see how it goes. FWIW throwing errors in middleware isn't supported (in the sense that Chalice won't do anything to catch/process it). The original spec goes into more detail on why but either approach had a different set of tradeoffs.

I like this approach where we'll now preserve that any middleware that raises an exception will now return the same error response from a view function.