From 8bf14238e31b6231ca8475d33c114c23bcc3f4f3 Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Sun, 16 Jul 2023 17:24:37 -0600 Subject: [PATCH 1/7] Expanded the Decorator api to allow for error handler registration --- chalice/app.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/chalice/app.py b/chalice/app.py index 921534510..cd6c33e66 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -31,6 +31,7 @@ from chalice.local import LambdaContext _PARAMS = re.compile(r'{\w+}') +ErrorHandlerFuncType = Callable[[Exception], Any] MiddlewareFuncType = Callable[[Any, Callable[[Any], Any]], Any] UserHandlerFuncType = Callable[..., Any] @@ -727,6 +728,17 @@ def _middleware_wrapper( return func return _middleware_wrapper + def error( + self, + exception: Exception + ) -> Callable[[ErrorHandlerFuncType], Any]: + def _error_wrapper( + func: Callable[[Exception], Response] + ) -> Callable[[Exception], Response]: + self.register_error(exception, func) + return func + return _error_wrapper + def authorizer(self, ttl_seconds: Optional[int] = None, execution_role: Optional[str] = None, name: Optional[str] = None, @@ -938,6 +950,10 @@ def _register_handler(self, handler_type: str, name: str, options: Optional[Dict[Any, Any]] = None) -> None: raise NotImplementedError("_register_handler") + def register_error(self, exception: Exception, + func: MiddlewareFuncType) -> None: + raise NotImplementedError("register_error") + def register_middleware(self, func: MiddlewareFuncType, event_type: str = 'all') -> None: raise NotImplementedError("register_middleware") @@ -954,11 +970,20 @@ def __init__(self) -> None: self.api: APIGateway = APIGateway() self.handler_map: Dict[str, Callable[..., Any]] = {} self.middleware_handlers: List[Tuple[MiddlewareFuncType, str]] = [] + self.error_handlers: List[Tuple[ErrorHandlerFuncType, str]] = [] def register_middleware(self, func: MiddlewareFuncType, event_type: str = 'all') -> None: self.middleware_handlers.append((func, event_type)) + def register_error(self, exception: Exception, func: Callable) -> None: + if not issubclass(exception, Exception): + raise ValueError( + f"{exception.__name__} is not a subclass of Exception." + "Error handlers can only be registered for Exception classes." + ) + self.error_handlers.append((func, exception.__name__)) + def _do_register_handler(self, handler_type: str, name: str, user_handler: UserHandlerFuncType, wrapped_handler: Callable[..., Any], kwargs: Any, @@ -2204,6 +2229,13 @@ def register_middleware(self, func: Callable, ) ) + def register_error(self, exception: Exception, func: Callable) -> None: + self._deferred_registrations.append( + lambda app, options: app.register_error( + exception, func + ) + ) + def _register_handler(self, handler_type: str, name: str, user_handler: UserHandlerFuncType, wrapped_handler: Any, kwargs: Dict[str, Any], From a83601d8a564c3d8221b7f1c11ecff2d2a3cc6db Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Sun, 16 Jul 2023 17:33:30 -0600 Subject: [PATCH 2/7] Modify Rest api event handler to return response from registered error handlers --- chalice/app.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/chalice/app.py b/chalice/app.py index cd6c33e66..8065e3303 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -1368,6 +1368,7 @@ def __call__(self, event: Any, context: Any) -> Dict[str, Any]: handler = RestAPIEventHandler( self.routes, self.api, self.log, self.debug, middleware_handlers=self._get_middleware_handlers('http'), + error_handlers=self.error_handlers ) self.current_request: \ Optional[Request] = handler.create_request_object(event, context) @@ -1810,7 +1811,10 @@ def __call__(self, event: Dict[str, Any], class RestAPIEventHandler(BaseLambdaHandler): def __init__(self, route_table: Dict[str, Dict[str, RouteEntry]], api: APIGateway, log: logging.Logger, debug: bool, - middleware_handlers: Optional[List[Callable[..., Any]]] = None + middleware_handlers: Optional[ + List[Callable[..., Any]]] = None, + error_handlers: Optional[ + List[Tuple[ErrorHandlerFuncType, str]]] = None ) -> None: self.routes: Dict[str, Dict[str, RouteEntry]] = route_table self.api: APIGateway = api @@ -1822,6 +1826,9 @@ def __init__(self, route_table: Dict[str, Dict[str, RouteEntry]], middleware_handlers = [] self._middleware_handlers: \ List[Callable[..., Any]] = middleware_handlers + if error_handlers is None: + error_handlers = [] + self._error_handlers = error_handlers def _global_error_handler(self, event: Any, get_response: Callable[..., Any]) -> Response: @@ -1947,13 +1954,28 @@ def _get_view_function_response(self, view_function: Callable[..., Any], except ChaliceViewError as e: # Any chalice view error should propagate. These # get mapped to various HTTP status codes in API Gateway. - response = Response(body={'Code': e.__class__.__name__, - 'Message': str(e)}, - status_code=e.STATUS_CODE) - except Exception: - response = self._unhandled_exception_to_response() + response = self._get_error_handler_response(e) or \ + Response(body={'Code': e.__class__.__name__, + 'Message': str(e)}, + status_code=e.STATUS_CODE) + except Exception as e: + response = self._get_error_handler_response(e) or \ + self._unhandled_exception_to_response() return response + def _get_error_handler_response(self, e: Exception) -> Response or None: + # Loops through the registered error handlers and returns the first + # `Response` result from handlers. If no handlers are matched or no + # matched handlers returned a `Response`, returns None to allow for + # chalice to handle the error. + raised = e.__class__.__name__ + handlers = (func for func, exc_type in self._error_handlers if + exc_type == raised) + for func in handlers: + response = func(e) + if isinstance(response, Response): + return response + def _unhandled_exception_to_response(self) -> Response: headers = {} path = getattr(self.current_request, 'path', 'unknown') From 747cfcea5216aa723271c1b623fd0b2a0b4f8aa0 Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Sun, 16 Jul 2023 19:23:33 -0600 Subject: [PATCH 3/7] Improved Typing --- chalice/app.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/chalice/app.py b/chalice/app.py index 8065e3303..1a6e6e540 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -733,8 +733,8 @@ def error( exception: Exception ) -> Callable[[ErrorHandlerFuncType], Any]: def _error_wrapper( - func: Callable[[Exception], Response] - ) -> Callable[[Exception], Response]: + func: ErrorHandlerFuncType + ) -> ErrorHandlerFuncType: self.register_error(exception, func) return func return _error_wrapper @@ -951,7 +951,7 @@ def _register_handler(self, handler_type: str, name: str, raise NotImplementedError("_register_handler") def register_error(self, exception: Exception, - func: MiddlewareFuncType) -> None: + func: ErrorHandlerFuncType) -> None: raise NotImplementedError("register_error") def register_middleware(self, func: MiddlewareFuncType, @@ -976,7 +976,7 @@ def register_middleware(self, func: MiddlewareFuncType, event_type: str = 'all') -> None: self.middleware_handlers.append((func, event_type)) - def register_error(self, exception: Exception, func: Callable) -> None: + def register_error(self, exception: Any, func: ErrorHandlerFuncType) -> None: if not issubclass(exception, Exception): raise ValueError( f"{exception.__name__} is not a subclass of Exception." @@ -1963,7 +1963,7 @@ def _get_view_function_response(self, view_function: Callable[..., Any], self._unhandled_exception_to_response() return response - def _get_error_handler_response(self, e: Exception) -> Response or None: + def _get_error_handler_response(self, e: Exception) -> Any: # Loops through the registered error handlers and returns the first # `Response` result from handlers. If no handlers are matched or no # matched handlers returned a `Response`, returns None to allow for @@ -1975,6 +1975,7 @@ def _get_error_handler_response(self, e: Exception) -> Response or None: response = func(e) if isinstance(response, Response): return response + return def _unhandled_exception_to_response(self) -> Response: headers = {} From d0055affe41e9841a974458bdf5f46e02f4415c0 Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Sun, 16 Jul 2023 19:24:01 -0600 Subject: [PATCH 4/7] Updated Documentation --- docs/source/topics/errorhandler.rst | 92 +++++++++++++++++++++++++++++ docs/source/topics/index.rst | 1 + docs/source/topics/middleware.rst | 8 ++- 3 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 docs/source/topics/errorhandler.rst diff --git a/docs/source/topics/errorhandler.rst b/docs/source/topics/errorhandler.rst new file mode 100644 index 000000000..526a1c689 --- /dev/null +++ b/docs/source/topics/errorhandler.rst @@ -0,0 +1,92 @@ +===================== +Custom Error Handling +===================== + +While chalice middleware allow for catching of user defined errors, exceptions +raised by a third party library can't be seen by the middleware and chalice +will set the response without giving the middleware a chance to see the +exception. These error handlers will only by used in the case of 'http' event, +as the middleware for other types of events can catch other exceptions +(see :ref:`middleware-error-handling-rest`). + +In the case where you want to return your own ``Response`` for those exceptions +you can register an error handler to intercept the exception. + +Below is an example of Chalice error hanler: + +.. code-block:: python + + from chalice import Chalice, Response + from thirdparty import func, ThirdPartyError + + app = Chalice(app_name='demo-errorhandler') + + @app.error(ThirdPartyError) + def my_error_handler(error: Exception): + app.log.error("ThirdPartyError was raised") + return Response( + body=e.__class__.__name__, + status_code=500 + ) + + @app.route('/') + def index(): + return func() + +In this example, our error handler is registered to catch ``ThirdPartyError`` +raised in a http event. In this case, if `func` were to raise a +``ThirdPartyError``, ``my_error_handler`` will be called and our custom +``Response`` will be returned. + +Writing Error Handlers +====================== + +Error handlers must adhere to these requirements: + +* Must be a callable object that accepts one parameter. It will be of + ``Exception`` type. +* Must return a response. If the response is not of ``chalice.Response`` type, + Chalice will either try to call the next error handler registered for the + same error or in the event where no handlers return a valid response, Chalice + will return a ``chalice.ChaliceViewError``. + + +Error Propagation +----------------- + +Cahlice will propagatet the error through all registered handlers until a valid +response is returned. If no handlers return a valid response, chalice will +handle the error as if no handlers were registered. + +.. code-block:: python + + @app.error(ThirdPartyError) + def my_error_handler_1(error: Exception): + if error.message == '1': + return Response( + body='Error 1 was raised', + status_code=200 + ) + + @app.error(ThirdPartyError) + def my_error_handler_2(error: Exception): + if error.message == '2': + return Response( + body='Error 2 was raised', + status_code=400 + ) + + @app.route('/1') + def index(): + # The response from `my_error_handler_1` will be returned + raise ThirdPartyError('1') + + @app.route('/2') + def index(): + # The response from `my_error_handler_2` will be returned + raise ThirdPartyError('2') + + @app.route('/3') + def index(): + # A ChaliceViewError will be returned + raise ThirdPartyError('3') diff --git a/docs/source/topics/index.rst b/docs/source/topics/index.rst index 057ca8126..ce0b56e78 100644 --- a/docs/source/topics/index.rst +++ b/docs/source/topics/index.rst @@ -25,3 +25,4 @@ Topics experimental testing middleware + errorhandler diff --git a/docs/source/topics/middleware.rst b/docs/source/topics/middleware.rst index 90a8ea0de..467f52fe0 100644 --- a/docs/source/topics/middleware.rst +++ b/docs/source/topics/middleware.rst @@ -96,6 +96,8 @@ If an exception is raised in a Lambda handler and no middleware catches the exception, the exception will be returned back to the client that invoked the Lambda function. +.. _middleware-error-handling-rest: + Rest APIs ~~~~~~~~~ @@ -271,15 +273,15 @@ Integrating with AWS Lambda Powertools -------------------------------------- `AWS Lambda Powertools -`__ is a suite of +`__ is a suite of utilities for AWS Lambda functions that makes tracing with AWS X-Ray, structured logging and creating custom metrics asynchronously easier. You can use Chalice middleware to easily integrate Lambda Powertools with your Chalice apps. In this example, we'll use the `Logger -`__ -and `Tracer `__ +`__ +and `Tracer `__ and convert them to Chalice middleware so they will be automatically applied to all Lambda functions in our application. From 95a6c28cfd31225331a9a9db2b15f1ecaf210eb1 Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Sun, 16 Jul 2023 23:40:34 -0600 Subject: [PATCH 5/7] Added tests --- tests/unit/test_app.py | 177 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index ea33f3ce3..371e29baa 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -19,6 +19,7 @@ from chalice.test import Client from chalice.app import ( APIGateway, + ChaliceViewError, Request, Response, handle_extra_types, @@ -151,6 +152,10 @@ def client(self, name, endpoint_url=None): return self._client +class CustomError(Exception): + pass + + @pytest.fixture def view_function(): def _func(): @@ -3745,3 +3750,175 @@ def myfunction(event, context): {'name': 'wrapped', 'event': {'input-event': True}}, {'name': 'myfunction', 'event': {'input-event': True}}, ] + + +class TestErrorHandler: + def test_error_handler_decorator(self): + demo = app.Chalice('app-name') + expected_response = Response(body='In error Handler', status_code=400) + + @demo.error(CustomError) + def custom_error_handler(e: Exception) -> Response: + return expected_response + + @demo.route('/') + def index(): + raise CustomError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_register(self): + demo = app.Chalice('app-name') + expected_response = Response(body='In error Handler', status_code=400) + + def custom_error_handler(e: Exception) -> Response: + return expected_response + + demo.register_error(CustomError, custom_error_handler) + + @demo.route('/') + def index(): + raise CustomError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_in_blueprint(self): + demo = app.Chalice('app-name') + bp = app.Blueprint('bp') + expected_response = Response(body='In error Handler', status_code=400) + + @bp.error(CustomError) + def custom_error_handler(e: Exception) -> Response: + return expected_response + + demo.register_blueprint(bp) + + @demo.route('/') + def index(): + raise CustomError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_from_blueprint(self): + demo = app.Chalice('app-name') + bp = app.Blueprint('bp') + expected_response = Response(body='In error Handler', status_code=400) + + @demo.error(CustomError) + def custom_error_handler(e: Exception) -> Response: + return expected_response + + @bp.route('/') + def index(): + raise CustomError() + + demo.register_blueprint(bp) + + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_invalid_response_type(self): + demo = app.Chalice('app-name') + + @demo.error(CustomError) + def custom_error_handler(e: Exception) -> Response: + return "Not a Response" + + @demo.route('/') + def index(): + raise CustomError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.status_code == ChaliceViewError.STATUS_CODE + + def test_error_handler_multiple_handlers_returns_first(self): + demo = app.Chalice('app-name') + expected_response = Response(body='In error Handler', status_code=400) + + @demo.error(CustomError) + def custom_error_handler(e: Exception) -> Response: + return expected_response + + @demo.error(CustomError) + def custom_error_handler_2(e: Exception) -> Response: + return Response(body="Not Expecting", status_code=200) + + @demo.route('/') + def index(): + raise CustomError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_multiple_handlers_returns_second(self): + demo = app.Chalice('app-name') + expected_response = Response(body='In error Handler', status_code=400) + + @demo.error(CustomError) + def custom_error_handler(e: Exception) -> Response: + return "Not a valid Response" + + @demo.error(CustomError) + def custom_error_handler_2(e: Exception) -> Response: + return expected_response + + @demo.route('/') + def index(): + raise CustomError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_overides_chalice_view_error(self): + demo = app.Chalice('app-name') + expected_response = Response(body='In error Handler', status_code=400) + + @demo.error(ChaliceViewError) + def custom_error_handler(e: Exception) -> Response: + return expected_response + + @demo.route('/') + def index(): + raise ChaliceViewError() + + with Client(demo) as c: + response = c.http.get("/") + + assert response.body.decode() == expected_response.body + assert response.status_code == expected_response.status_code + + def test_error_handler_invalid_registration(self): + demo = app.Chalice('app-name') + + class NotAnException: + pass + + def custom_error_handler(e: Exception) -> Response: + return + + with pytest.raises(ValueError): + demo.register_error(NotAnException, custom_error_handler) From 4de3c216b1631f4f135aea6b1aed330ed11e4f45 Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Sun, 16 Jul 2023 23:52:49 -0600 Subject: [PATCH 6/7] linting --- chalice/app.py | 3 ++- tests/unit/test_app.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/chalice/app.py b/chalice/app.py index 1a6e6e540..6fafb6cd1 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -976,7 +976,8 @@ def register_middleware(self, func: MiddlewareFuncType, event_type: str = 'all') -> None: self.middleware_handlers.append((func, event_type)) - def register_error(self, exception: Any, func: ErrorHandlerFuncType) -> None: + def register_error(self, exception: Any, + func: ErrorHandlerFuncType) -> None: if not issubclass(exception, Exception): raise ValueError( f"{exception.__name__} is not a subclass of Exception." diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index 371e29baa..4a59a2715 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -3826,7 +3826,6 @@ def index(): demo.register_blueprint(bp) - with Client(demo) as c: response = c.http.get("/") From 646b3c1ef1143bf57195f1c6a021e40d61b571f0 Mon Sep 17 00:00:00 2001 From: Mathieu Cloutier Date: Mon, 17 Jul 2023 00:04:52 -0600 Subject: [PATCH 7/7] Changed error raised to TypeError --- chalice/app.py | 2 +- tests/unit/test_app.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/chalice/app.py b/chalice/app.py index 6fafb6cd1..2442b3e42 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -979,7 +979,7 @@ def register_middleware(self, func: MiddlewareFuncType, def register_error(self, exception: Any, func: ErrorHandlerFuncType) -> None: if not issubclass(exception, Exception): - raise ValueError( + raise TypeError( f"{exception.__name__} is not a subclass of Exception." "Error handlers can only be registered for Exception classes." ) diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index 4a59a2715..e4dfbf2f5 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -3919,5 +3919,5 @@ class NotAnException: def custom_error_handler(e: Exception) -> Response: return - with pytest.raises(ValueError): + with pytest.raises(TypeError): demo.register_error(NotAnException, custom_error_handler)