Custom event handler feedback

[natflow]

I thought I'd share some feedback on the custom event handler feature, especially given using it produces a warning about it being provisional and possibly eventually removed: I think they're great! I've written other event handlers by hand and done all the routing myself and it's a hassle and a lot of weird code.

My use-case was for a Lambda that is configured for Cognito trigger events in addition to API GW. I wrote a little class that did some checks (event.get("triggerSource", "").startswith("PreSignUp_") and translations (NOTIFY HTTP method and /cognito/presignup_signup path) and got the routing and dependency injection from FastAPI that I'd always wanted for these non-HTTP events.

A few thoughts on improvements:

1. I didn't see that LambdaHandler.__init__ was actually used in the adapter's infer or __call__ methods. If it's not being used anywhere else either I'd suggest removing it from the protocol to allow greater flexibility for the custom handlers.
2. I know it's an ASGI thing but I wish that when Lambda is given structured input I didn't have to json.dumps it for LambdaHandler.body and json.loads it again in __call__. Or maybe there's some ASGI extension or workaround? I haven't dug into it but I'm afraid any solution would be a combination of ASGI middleware, FastAPI middleware, and custom FastAPI Request/Response classes.
3. There are a lot of fields for HTTPScope and many of them are typed as literals. LambdaHandler.scope could be a lot simpler if there was some concept of default values -- either allowing for a partial object return (and Mangum would fill in the rest) or constructing a full object using some default object exported from Mangum (eg return { **mangum.http_scope_defaults, **my_overrides }).

[jordaneremieff]

Hi @natflow, thanks for the detailed feedback!

The warning is mainly intended to signal that these handlers may see several breaking changes before settling on a stable design, though I think what currently exists mostly represents how these handlers will look going forward. I'll go ahead and state now that support for custom handlers will no longer be considered provisional in the next release.

To address your feedback specifically:

1. The LambdaHandler.__init__ is defined for all of the default handler classes. The infer method for these handlers instantiate the handler by passing along the event, context, and config arguments (e.g. the APIGateway) constructor. That said, perhaps the protocol does not need to specify this since a custom handler could potentially implement the infer method differently but otherwise return a valid handler type. I primarily wanted to ensure that necessary inputs required for all of the default handlers (event, context, config) were persisted to the instance to be made accessible for the scope definition. I guess even then I would not necessarily expect the config to be required for other handlers since that is specific to Mangum. I'll have to think about this one a bit more, but if you have an example of a custom handler that is different than the default handlers that would be very helpful towards clarifying this.

2. I'm not sure if I understand this point fully (or at least not enough to point you to a particular direction). Could you provide an example that demonstrates the problem?

3. Could you provide an example of an alternative HTTPScope event for a customer handler? The scope definition right now represents the precise structure expected by the cycle class and/or applications to implement the generic behavior according to a combination of the ASGI spec and some values specific to the default handlers. I'm open to this suggestion, but need to understand what kind of differences there might be.

It would seem the gist of my responses above is that I'm open to changes here to better accommodate the feature, but I need more clarity on how a custom handler might actually look and behave.

[natflow]

Great to hear they're moving beyond provisional support. I think they'll be great for the library and its users.

1. Yes, not specifying it in the protocol is all I'd suggest changing here. Kind of a minor point as generally this whole protocol implementation was pretty easy to implement and gave me the great results I was hoping for, but still an issue with a potential for improvement I think.
2. Here's most of the source to the Cognito trigger handler I implemented. Note that the request is structured (self.event["request"] is a dict, not bytes) and the response is expected to be structured as well (.response should be a dict), but I had to serialize and deserialize them to JSON and back.

class CognitoTriggerHandler:

    event: LambdaEvent
    context: LambdaContext

    @property
    def body(self) -> bytes:
        """
        Returns the raw body to be used for the HTTP request
        """
        return json.dumps(self.event["request"]).encode()

    @property
    def scope(self) -> HTTPScope:
        """
        Returns the ASGI scope for the HTTP request

        See the following for the event keys common to all User pool Lambda trigger events:
        https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-identity-pools-working-with-aws-lambda-triggers.html#cognito-user-pools-lambda-trigger-syntax-shared
        """
        return {
            "type": "http",
            "http_version": "1.1",
            "method": "NOTIFY",
            "headers": [],
            "path": "/cognito/" + self.event["triggerSource"].lower(),
            "raw_path": None,
            "root_path": "",
            "scheme": "http",
            "query_string": b"",
            "server": ("my-service-name", 80),
            "client": ("127.0.0.1", 0),
            "asgi": {"version": "3.0", "spec_version": "2.0"},
            "aws.event": self.event,
            "aws.context": self.context,
        }

    def __call__(self, response: HTTPResponse) -> dict:
        """
        Returns a translated version of the HTTP response suitable for the trigger event.
        """
        if response["status"] == 200:
            return {
                **self.event,
                "response": json.loads(response["body"]),
            }

3. In this class's scope method, you can see that most of the fields are just dummy values (only the path is specific to my code. Granted, this is a translation between HTTP and non-HTTP, but the HTTPScope definition also has many Literals which are always going to be duplicated for every implementation. I'm not sure how it could be done so all I can say is that I wish it was somehow possible to specify fewer of the keys. Like, it's boilerplate that's unspecific to most implementations, and so it would be nice to reduce that aspect.

I hope that helps clear things up with my second and third points. I'm afraid I can't be as specific as would be most helpful since I'm unfamiliar with the guts of these things and so unclear myself on how they could possibly be resolved. Also, I'm offering these points as general ideas and not at all as significant hindrances -- I was overall very happy with the outcome for my code.

[jordaneremieff]

Thanks for the follow-up @natflow. I'll see if I can come up with something a bit more flexible.