Bug: Event handler middleware not triggered for not found routes

[tomsimpkins]

Expected Behaviour

I think I would expect (global) middleware to be triggered when a not-found route is used. I have middleware that implements desirable behaviour even when the route is not found.

Current Behaviour

Currently global middleware seems to be bypassed when a not found route is hit.

Code snippet

from aws_lambda_powertools.event_handler import APIGatewayRestResolver, Response
from aws_lambda_powertools.event_handler.middlewares import NextMiddleware


def test_resolve():
    def log_middleware(
        app: APIGatewayRestResolver, next_middleware: NextMiddleware
    ) -> Response:
        print("middleware before")
        return next_middleware(app)

    app = APIGatewayRestResolver()
    app.use(middlewares=[log_middleware])

    @app.get("/listItems")
    def list_items():
        return [{"id": 1}, {"id": 2}]

    @app.not_found
    def not_found(exc):
        return Response(status_code=404)

    # calling this triggers middleware
    app.resolve(
        {
            "path": "/listItems",
            "httpMethod": "GET",
            "requestContext": {},
        },
        {},
    )

    # calling this does not trigger middleware
    app.resolve(
        {
            "path": "/junk",
            "httpMethod": "GET",
            "requestContext": {},
        },
        {},
    )

Possible Solution

The solution I've used currently is to use catch all route handlers to sweep up any unexpected routes, which looks to app like a "valid" route. This means middleware is triggered.

Another solution would be to trigger global middleware before route resolution.

Steps to Reproduce

I think I've supplied enough context. The above code snippet should run under pytest -rP Please let me know if more is needed.

Powertools for AWS Lambda (Python) version

latest

AWS Lambda function runtime

3.11

Packaging format used

PyPi

Debugging logs

No response

[boring-cyborg]

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

[leandrodamascena]

Hi @tomsimpkins! Thanks for raising this issue and identifying this bug. We should process the global middlewares even when invoking not_found method.

Do you want to submit a PR to fix this? We would love to have your contribution here. Please let us know.

Thanks.

[tomsimpkins]

Hi @leandrodamascena thanks for getting back. I can't dive into this immediately but I've done some reading and think I know how this might be solved. Please let me know if you agree/disagree.

I'd propose modelling the not found handler itself as middleware. So in the resolver we would have:

def _not_found(self, method: str):
   def not_found_middleware(app, next_middleware):
       # call handler for not found here and return Response
    
    middleware_stack = not_found_middleware
    for handler in reversed(self._router_middlewares + [_registered_api_adapter]):
        middleware_stack = MiddlewareFrame(current_middleware=handler, next_middleware=middleware_stack)
  
    return ResponseBuilder(middleware_stack(self))

The same issue seems to apply with the exception handlers. I think we could provide the exception handler as a middleware to the Route as the last element of the _route_middlewares:

response=self._to_response(
    route(router_middlewares=self._router_middlewares + [exception_handler_as_middleware], app=self, route_arguments=route_arguments),
),

There's definitely benefit to reusing the logic from Route, so I prefer the answer for the exception handlers to the not found route because of the reuse. cors for example has to be duplicated in the not found case. I considered making a NotFoundRoute which implements the overlapping functionality that we'd want so there's symmetry, but I don't know the codebase well enough to make that change.

[heitorlessa]

resurfacing to @leandrodamascena for later this week. I'm gonna tackle as many other issues as possible today.

[leandrodamascena]

Assigning to @heitorlessa!