Repository navigation
fix(event_handler): keep catch-all exception handlers off 404s and adapt exception handler returns - #8624
Conversation
|
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
…ion handler returns - An unmatched route no longer resolves to a handler registered for Exception or BaseException. No exception was raised, so a catch-all 500 handler (as the docs suggest) turned every unmatched route into a 500. Handlers for NotFoundError and ServiceError still apply. - Whatever an exception handler returns now goes through _to_response, like a route's return value: Response(body=None) on ALB becomes "" instead of null (ALB 502), and dict / (dict, status) returns are serialized instead of raising AttributeError. Fixes aws-powertools#8579
6cd0a9d to
54e7e6f
Compare
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks @MrNK2107! I ran the suites and deployed this behind a real REST API: an unmatched route goes back to the default 404 even with a catch-all handler registered, a handler returning a tuple gives a proper 400 JSON instead of the AttributeError, and real exceptions still reach the catch-all. I also checked @app.not_found, a ServiceError handler, the [Exception, NotFoundError] list, the ALB None body, the Depends path and Bedrock, all behave. Nice touch dropping the now redundant _to_response in the ALB size path.
Approving now. This goes out in 3.38.0, so it will sit here a bit before I merge. No need to rebase or touch anything, I'll take care of it and you'll get the credit in the release notes. Thank you!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8624 +/- ##
===========================================
- Coverage 96.94% 96.93% -0.01%
===========================================
Files 318 318
Lines 16051 16058 +7
Branches 1427 1430 +3
===========================================
+ Hits 15560 15566 +6
Misses 349 349
- Partials 142 143 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
781f60e
into
aws-powertools:develop
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
|
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |



Issue number: closes #8579
Summary
Changes
not_found_handler(sync and async) usedlookup_exception_handler(NotFoundError), which walks the MRO up toException. A new_lookup_not_found_handler()walks the same MRO but stops beforeException/BaseException. Handlers forNotFoundError(@app.not_found) andServiceErrorstill format 404s as before. Only catch-all handlers are skipped, since no exception was actually raised.body: nullon ALB, and dict/tuple returns crashing._call_exception_handlernow passes the handler's return value through_to_response, the same adaptation a route's return value gets. As a result,ALBResolver._to_responseturnsbody=Noneinto"", and{...}/({...}, status)are serialized as JSON._call_exception_handler, so I removed that now-redundant line.New tests:
test_api_gateway.py: a catch-all handler no longer replaces the 404, aServiceErrorhandler still formats 404s, aNonebody on ALB becomes"", and dict and tuple returns are serialized.test_resolve_async.py: a catch-all handler no longer replaces the 404 on the async path.All event_handler tests pass. Locally on Windows,
test_openapi_encode_purepathandtest_load_resolver_file_not_foundalso fail on an unmodifieddevelop, because of path separators.User experience
Before:
After:
GET /nopereturns the default 404, and the tuple becomes a 400 JSON response. OnALBResolver, a handler returningResponse(status_code=400, body=None)sends"body": ""instead ofnull.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.