Repository navigation
fix(event_handler): propagate UnauthorizedException and await async class-based middlewares - #8633
SuellenQwQ wants to merge 5 commits into
Conversation
…pSync Events resolvers The per-event resolvers (`aggregate=False`) catch every exception and turn it into a per-event error payload. `UnauthorizedException` is a control-flow signal rather than a payload: re-raise it so AppSync denies the whole operation. Both the sync and the async paths now behave like the aggregate and subscribe resolvers. Signed-off-by: SuellenQAQ <sulun6693485@163.com>
`AsyncMiddlewareFrame` only awaited middlewares that are themselves coroutine functions. A `BaseMiddlewareHandler` subclass with an `async def handler` is not a coroutine function, so the frame returned the coroutine and the response leaked through un-awaited. Detect the class-based `handler` too and await whatever the middleware returns. Also return a short-circuiting middleware's own response in `wrap_middleware_async` instead of raising IndexError when the next middleware was never called. Signed-off-by: SuellenQAQ <sulun6693485@163.com>
…rcuiting Added tests for class-based async middleware and sync short-circuit behavior. Signed-off-by: SuellenQAQ <sulun6693485@163.com>
…vent resolvers Add tests for handling unauthorized exceptions in AppSync events. Signed-off-by: SuellenQAQ <sulun6693485@163.com>
powertools-for-aws-oss-automation
Bot
added
the
size/L
leandrodamascena
left a comment
leandrodamascena
left a comment
There was a problem hiding this comment.
Thanks @SuellenQwQ, both fixes are right. I deployed this behind a real AppSync Events API with a Lambda data source: a publish handler raising UnauthorizedException with the default aggregate=False now makes AppSync answer 401 with its own generic message, for sync and async handlers, where develop returned 200 with the exception text in a per-event CustomError. The class-based async middleware works under resolve_async, including the short-circuit cases.
Your branch imported two helpers from api_gateway that only reached develop after you branched, so on its own it failed 121 tests and conflicted on async_utils.py. I merged develop into it keeping your version of the file, which already accounted for them, and everything is green now. Nothing for you to do.
Approving. This goes out in 3.39.0, so it will sit here for a while before I merge it. #8634 covers item 3 of the issue, so keeping closes here is fine. You'll be credited in the release notes.
|
Issue number: closes #8582
Summary
Changes
AppSyncEventsResolvernow re-raisesUnauthorizedExceptionraised by a per-event (aggregate=False) publish resolver, in both the sync and the async path, instead of converting it into a per-event error payload.AsyncMiddlewareFramenow awaits class-based middlewares whosehandleris a coroutine function, soBaseMiddlewareHandlersubclasses keep working withapp.resolve_async().wrap_middleware_asyncreturns the response of a middleware that short-circuits without calling the next one, instead of raisingIndexError.User experience
A resolver that denies access with
UnauthorizedExceptionmakes AppSync deny the whole operation regardless ofaggregate, which matches the intent of the exception and what the subscribe path already does. Middlewares written asBaseMiddlewareHandlersubclasses withasync def handlernow run underapp.resolve_async()instead of returning an un-awaited coroutine.This covers items 1 and 2 of the issue. Item 3 (
HttpResolverLocalas a Lambda handler) is a change to the ASGI/Lambda dispatch in__call__and is left out of this PR. If you would rather keep #8582 open for item 3, say so and I will drop thecloseskeyword and open a separate issue for it.Checklist
pytest tests/functional/event_handler/required_dependencies/appsync tests/functional/event_handler/required_dependencies/test_async_middleware_frame.pyDisclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.