Add support to partial batch response

[Kralizek]

Let's discuss here how to best address partial batch responses.

This issue was spawned by #25

[bartpio]

I drafted #29 with an approach that changes the two SQS event handlers (SqsEventHandler and ParallelSqsEventHandler) to implement IRequestResponseHandler<SQSEvent, SQSBatchResponse>, in addition to IEventHandler<SQSEvent>.

The IRequestResponseHandler<SQSEvent, SQSBatchResponse> flavor can be used by writing an entry point class of the form:

public class Function : RequestResponseFunction<SQSEvent, SQSBatchResponse>
{
    // …

    protected override void ConfigureServices(IServiceCollection services, IExecutionEnvironment executionEnvironment)
    {
        // no difference here
        services.UseQueueMessageHandler<TestMessage, TestMessageHandler>();
    }
}

When this flavor is used, exceptions thrown by the IMessageHandler implementation are caught, and reported to Lambda in the response payload.

When the original IEventHandler<SQSEvent> flavor is used, exceptions propagate to the Lambda runtime, as in the prior state. The updated SQS handler implementations include exception filters to help reuse code, while ensuring that the code flow remains unchanged from the prior state (contrast with catching and re-throwing) when partial batch support is not used.

I had another thought to write an EventResponseFunction that is sort of a hybrid between EventFunction and RequestResponseFunction, but this turned out to be disruptive and overcomplicated.

@Kralizek please let me know what you think!

[Kralizek]

I find your solution quite elegant but also hard to discover.

Maybe adding another template might be enough to solve the discoverability issue.

[bartpio]

I added a lambda-template-sqs-partial-batch template. This does help discoverability a bit. It is tricky to make this feature highly discoverable, especially for users not yet aware of its existence on the AWS side. If you have any further ideas, I would be glad to implement.

[bartpio]

Maybe another layer of abstract functions for entry point classes like in e63e887 helps discoverability a little?

[bartpio]

A downside of e63e887 is that while it's not breaking, it introduces an additional convention to choose from when writing a function (e.g. for regular SQS functions, should we derive from SqsEventFunction or EventFunction<SQSEvent>? is there a recommended path?).

For partial batch functions in particular, I find deriving from a pre-supplied SqsBatchResponseFunction rather than RequestResponseFunction<SQSEvent, SQSBatchResponse> mildly appealing from a discoverability and memorization standpoint. But this is not really a compelling argument for introducing an additional convention and potentially changing the recommended convention across the board.

If you do like e63e887 I would of course be glad to add it to the PR along with any ideas you might have regarding documentation.

I am now thinking your idea of adding another template does adequately solve the discoverability issue. I appreciate your feedback so far and your time spent thinking about the issue!

[Kralizek]

Hi, sorry I disappeared.

I've been thinking a bit about the best way to go forward and I would like to hear your opinion.

Basically, I am thinking about going back to a single template, make the current template use the RequestResponse version by default and leave the existing components for backward compatibility and advanced use cases.

Then users can specify if they want to invalidate the whole batch or each item one by one when registering the message handler.

I haven't had the time to play around with it yet, but I was thinking about giving it a go by looking at what you did in #29, unless you want to give it a go yourself.

I'm sorry it's taking so long but I'm a bit swamped personally.

[bartpio]

No worries at all! We can go back to using a single template, using the RequestResponse version, effectively making this the default. We can do something roughly analogous to WithParallelExecution (like the ParallelSqsExecutionOptions part, not the registering services part) when registering the message handler, so that the user can elect to invalidate batch items one by one.

Invalidating the whole batch should be default, as:

1. This lines up with defaults on the AWS λ trigger side.
2. This matches our prior state.

I'm thinking the new template should return a nullable SQSBatchResponse, like this:

public class Function : RequestResponseFunction<SQSEvent, SQSBatchResponse?>
{

We can return null in the default configuration (where the user has not elected to invalidate batch items one by one), pending research and validation that returning null from RequestResponse functions doesn't break anything. Sound reasonable?