[WIP] feat!: Replace Handler with tower::Service

[nmoutschen]

Issue #, if available: #374

Description of changes:

This PR is meant to compare the current implementation with Handler with using tower::Service and should not be merged as-is. See the discussion in #374 for more information.

List of changes:

• Removes Handler and HandlerFn.
• Makes handler_fn return a ServiceFn instead.
• Adds a new LambdaRequest type to wrap request and context in a single struct.
• Changes run signature to take a Service<LambdaRequest<A>> instead.

BREAKING CHANGE: this removes the Handler trait, which could be used by existing libraries. However, developers that are using lambda_runtime::run and lambda_runtime::handler_fn should not be impacted. The closure taken by HandlerFn remains the same, so developers can keep using function with separate event and context parameters.

By submitting this pull request

• I confirm that my contribution is made under the terms of the Apache 2.0 license.
• I confirm that I've made a best effort attempt to update all relevant documentation.

[davidbarsky]

Is this intentionally not public?

[nmoutschen]

Yes! My thinking here was to only use this function as part of handler_fn to help wrap a function that takes two arguments.

[davidbarsky]

gotcha. for the public interface, i think i'd personally try to use (and-reexport) tower::service_fn.

[davidbarsky]

Nit: I'd make these fields private and only expose them via getters to not make adding fields a semver-breaking change.

[nmoutschen]

Good point!

For reference if anyone else reads this, I wanted to dig into why getters would be a good idea here and found this message on the Rust forum.

That said, given that the only benefit of getters here would be preventing breaking changes, isn't it better to use #[non_exhaustive] instead?

[davidbarsky]

That said, given that the only benefit of getters here would be preventing breaking changes, isn't it better to use #[non_exhaustive] instead?

#[non_exhaustive] also makes it impossible for the Context struct to be constructed in e.g., test code. i'd consider creating a builder for this, but i need to remember the standard behavior.

[rimutaka]

I may be confusing what is being discussed. Feel free to ignore ...
I've been trying to add a cache (#372) that can be managed by the handler and preserved by the runtime between invocations. I wonder how it may work with this change.

My understanding is that &mut is not possible due to move, so we need to pass the ownership of the cache to the handler, get it back with the response, preserve until the next invocation and then pass onto the handler again. The type of the cache is unknown. It will be up to the handler_fn implementers to define it.

Would it be a good idea to have something like this?

pub struct LambdaRequest<T,Q> {
    /// Event payload.
    pub event: T,
    /// Invocation context.
    pub context: Context,
    /// User-defined cache
    pub cache: Q,
}

Q would probably need Send bound, but otherwise keep it as unconstrained as possible.

It probably doesn't matter if the example given by @softprops in #372 (comment) would still work after this change.

[nmoutschen]

Hey @rimutaka ! Thanks for taking the time to look at this. 😄

With this change, the example from @softprops would not directly, but you could make your own tower::Service or tower::Layer. Basically, there would be 2 interfaces for creating Lambda functions:

• A high-level interface that works in the same way as other languages on Lambda (with handler_fn and functions that take 2 arguments (event and context);
• A low-level interface that takes a Service<LambdaRequest<A>>, where you can adopt the same strategy as the comment you linked, but substitute Handler<A, B> with Service<LambdaRequest<A>>. See this example using the proposed changes.

That said, for your specific use-case, you could also use the high-level interface if you're OK with using a RefCell.

[nmoutschen]

Actually, taking a step back, I think the LambdaRequest could work slightly differently. I'm not too keen on the context key, so we could have an extensions pattern similar to http::Request.

Then from there, you could provide tower::Layers that would inject values into the request that you could retrieve later. We'd inject the Context by default, but you could use that to inject a cache for example, and retrieve it with request::extensions_mut::<MyCache>().

[rimutaka]

Extensions would be a nice addition to the current option of implementing Handler trait. I am concerned we may overcomplicate the user interface, though. Just a thought.

[nmoutschen]

Closing this one since this was just for experimentation purposes. I'll create a new one for real this time.