-
Notifications
You must be signed in to change notification settings - Fork 402
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
feat(apigateway): add exception_handler support #898
feat(apigateway): add exception_handler support #898
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #898 +/- ##
========================================
Coverage 99.88% 99.88%
========================================
Files 118 118
Lines 5139 5162 +23
Branches 573 578 +5
========================================
+ Hits 5133 5156 +23
Misses 2 2
Partials 4 4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's looking great, thank you for implementing it that quick.
One major question to better understand the int as GitHub Mobile isn't showing me all the code easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor ask on testing subclass exceptions. If you can't I can do it tomorrow, as I'll merge this along with others in prep for the release.
Leave with me then, I’ll see if issubclass handles, and then merge - I’ll
do it tomorrow
…On Thu, 16 Dec 2021 at 16:31, Michael Brewer ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In aws_lambda_powertools/event_handler/api_gateway.py
<#898 (comment)>
:
> raise
+ def not_found(self, func: Callable):
+ return self.exception_handler(NotFoundError)(func)
+
+ def exception_handler(self, exc_class: Type[Exception]):
+ def register_exception_handler(func: Callable):
+ self._exception_handlers[exc_class] = func
+
+ return register_exception_handler
+
+ def _lookup_exception_handler(self, exp_type: Type) -> Optional[Callable]:
+ # Use "Method Resolution Order" to allow for matching against a base class
@heitorlessa <https://github.com/heitorlessa> there is a test
test_exception_handler_service_error which registers a ServiceError
handler and raises a InternalServerError
—
Reply to this email directly, view it on GitHub
<#898 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBFDKMJZD6M75SXTUE3URIA6FANCNFSM5KCXX7ZQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***
com>
|
@heitorlessa - it would have to be a different implementation def _lookup_exception_handler(self, exp_type: Type) -> Optional[Callable]:
for cls in exp_type.__mro__:
if cls in self._exception_handlers:
return self._exception_handlers[cls]
return None vs def _lookup_exception_handler(self, exp_type: Type) -> Optional[Callable]:
for cls in self._exception_handlers.keys()
if issubclass(exp_type, cls):
return self._exception_handlers[cls]
return None Also we will be aware that order should either matter OR not: app = ApiGatewayResolver()
@app.exception_handler(InternalServiceError)
def internal_service_error(ex: InternalServiceError):
...
@app.exception_handler(ServiceError)
def generic_service_error(ex: ServiceError):
... |
I see it now, this would likely happen and order is important (MRO is better). I'll merge as is and write the docs tomorrow |
We could possibly expand on the tests to be sure we have a contract of that the behavior would be. |
…tools-python into feat/batch-new-processor * 'develop' of https://github.com/awslabs/aws-lambda-powertools-python: fix(parser): kinesis sequence number is str, not int (aws-powertools#907) feat(apigateway): add exception_handler support (aws-powertools#898) fix(event-sources): Pass authorizer data to APIGatewayEventAuthorizer (aws-powertools#897) chore(deps): bump fastjsonschema from 2.15.1 to 2.15.2 (aws-powertools#891)
Issue #, if available:
Description of changes:
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.