Add support for type-safe Start* function

[logandavies181]

This PR adds a new Start function, which uses go generics to only accept functions which will be valid as described by lambda.Start and verified by lambda.validateArguments and lambda.validateReturns.

This is to give developers assurance at compile-time that their supplied function is valid.

Also updates the comment for lambda.Start to include all valid cases and removes one redundant one.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[bmoffatt]

Thanks for opening this!

[bmoffatt]

I think we can come up with a better name, but I don't have a suggestion just this moment

[bmoffatt]

Maybe StartHandlerFunc? 🤷‍♂️

If there's a way to update the current Handler interface in a backwards compatible way, then un-deprecating and changing StartHandler/StartHandlerWithOptions could be an option too

[logandavies181]

I don't think there's a backwards compatible way due to the inference situation. Short of breaking the API only for users of go 1.18+

[bmoffatt]

I need to play with it more, but I think from when I played with this in the past having all the variants this way means that the in/out types end up always needed to be specified rather than being inferred, whereas we could have a little less boilerplate if we agreed only on the last variant. eg:

func StartBetterNamed[I, O any, H func(context.context, I) (O, error)] (handler H, options ...Option) { 
     /*... */ 
}

should be usable without the type brackets that show up in your test cases

func main() {
    lambda.StartBetterNamed(func (ctx context.Context, event any) (any, error) {
        /* ... */
    })
}

[logandavies181]

Yeah I agree it'll be much nicer to help go infer the types and avoid passing in unused types.

Makes the naming problem a bit harder though 😄

[logandavies181]

Hmm so the type inference doesn't seem to want to play ball https://go.dev/play/p/Ls5MrkTK76g

I could split up the interface so at least the caller doesn't need to specify a type that they don't use. What do you reckon?

[bmoffatt]

I'd be OK with

type HandlerFunc interface {
    func(context.Context, TIn) (TOut, error)
}

since then the inference on TIn/TOut works https://go.dev/play/p/r3TIVrSuc4j

I'm not super attached to the polymorphic-ish style that Start's reflection code currently supports, if Go had generics in 2017 I'd probably not have done it that way

I could split up the interface so at least the caller doesn't need to specify a type that they don't use

I'm not sure what you mean by this, got an example?

[logandavies181]

Type inference seems to only work if the interface only has a single variant.

In this example the user needs to supply a type for I, despite not using it https://go.dev/play/p/-7nPycevowb

I was suggesting having more interfaces, so that could be avoided:

// more types more things to name though (:
type HandlerFunc interface {
	func() | func() error
}

type HandlerFuncTIn[I any] interface {
	func(I) | func(I) error | func(context.Context, I) | func(context.Context, I) error
}

type HandlerFuncTOut[O any] interface {
	func() (O, error)
}

type HandlerFuncTInTOut[I, O any] interface {
	func(context.Context, I) (O, error)
}

But then that's four interfaces and doesn't solve the inference problem (except for HandlerFuncTOut). So maybe one interface per variant...

[bmoffatt]

What's the use case you have for exporting the validation logic?

[logandavies181]

So the original goal of this PR was to have assurance that the function supplied to lambda.Start is valid without having to compile and run and invoke the running lambda. It's shown in this PR this can be achieved at compile time with generics, but I'm guessing that the inference issue means that you're probably not keen to have that monolithic interface or the 4+ smaller ones.

Exporting the validation logic gives an easy path to validate the function in a unit test, or I suppose by checking and panicking early at runtime. Plus it's a much smaller change.

What do you reckon?

[bmoffatt]

but I'm guessing that the inference issue means that you're probably not keen to have that monolithic interface or the 4+ smaller ones.

I am still good with a single interface (ref: #468 (comment)) - if the inference gets smarter in the future, then it could be grown to look like the your original monolithic interface

Exporting the validation logic gives an easy path to validate the function in a unit test, or I suppose by checking and panicking early at runtime.

:) I think for this you'd want to rely on the unit test (or the generic type, when added!) rather than panic-ing at runtime. A panic triggers a sort of cold start, which under load could contribute to consuming the account's Lambda concurrency limit.

[logandavies181]

I am still good with a single interface (ref: #468 (comment)) - if the inference gets smarter in the future, then it could be grown to look like the your original monolithic interface

So just to confirm, you're saying you're happy adding just that single interface with the single variant? So the plan would be to start with adding that to the public interface, and then when the inference gets better, adding support for the other variants?

type HandlerFunc interface {
    func(context.Context, TIn) (TOut, error)
}

:) I think for this you'd want to rely on the unit test (or the generic type, when added!) rather than panic-ing at runtime. A panic triggers a sort of cold start, which under load could contribute to consuming the account's Lambda concurrency limit.

100% agree. I'm never a fan of panics. session.Must never seems right to me

[bmoffatt]

So just to confirm, you're saying you're happy adding just that single interface with the single variant? So the plan would be to start with adding that to the public interface, and then when the inference gets better, adding support for the other variants?

Yup!

[codecov-commenter]

Codecov Report

Merging #468 (e81678e) into main (0f46684) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #468      +/-   ##
==========================================
+ Coverage   69.69%   69.74%   +0.05%     
==========================================
  Files          20       21       +1     
  Lines        1201     1203       +2     
==========================================
+ Hits          837      839       +2     
  Misses        297      297              
  Partials       67       67              

Impacted Files	Coverage Δ
lambda/entry.go	81.81% <ø> (ø)
lambda/entry_generic.go	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[logandavies181]

Thanks @bmoffatt 🚀🚀