Better supports for Events API on FaaS

[seratch]

Description

Related to #361, I would like to introduce a new option to AppOptions in the next minor version or Bolt 2.

I would love to get you all's suggestions for better naming but it is a flag to turn on/off the automatic Events API request acknowledgment. It allows developers to customize the behavior here: https://github.com/slackapi/bolt/blob/%40slack/bolt%401.5.0/src/App.ts#L474-L475

If we go with a name like autoEventAck, the default value is supposed to be true. If a developer set it as false, the app.event listener has ack function and it's expected to call it for sure.

This is necessary to run Bolt apps safely on FaaS (Function-as-a-Service). Without this, Events API handlers may unexpectedly be terminated even while they're still running.

What type of issue is this? (place an x in one of the [ ])

• bug
• enhancement (feature request)
• question
• documentation related
• testing related
• discussion

Requirements (place an x in each of the [ ])

• I've read and understood the Contributing guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.
• I've searched for any related issues and avoided creating a duplicate issue.

[seratch]

@aoberoi @stevengill @shaydewael
Could you check the followings?

• Do you agree with this idea?
• If so, do you have better names for the flag in mind?

[aoberoi]

My biggest concern is introducing an option for this behavior makes sample code, middleware, and documentation less portable. If some apps have the option set to the opposite of the default, they have to be very careful to comb through all their code to modify .event() listenerse (and middleware which operate on events) to do the right thing. In documentation, we'll have to add caveats about this in many places.

I think the optionality will fragment our audience and increase our effort. Is that worth it?

Why can't the FaaS implementation send responses after the event is fully processed (which it can in v2)? Even the .action() listeners who call ack() shouldn't actually send the response until the event is fully processed. This alternate behavior can be full encapsulated in a Receiver implementation.

[seratch]

@aoberoi
Thanks for your prompt response! I agree there is a downside you pointed out.

Why can't the FaaS implementation send responses after the event is fully processed (which it can in v2)?

This is also an option. One thing I need to mention here is that if a Receiver awaits for the completion of listeners, it may result in a 3-second timeout.

If we go with your idea, we may should mention the possibility of the ack timeouts and also encourage developers to consider some ways to handle async executions (e.g., Promises for class app hosting, queues/function-invocations for FaaS) in Bolt documents.

Thanks for the great suggestion. Now, I'm happy to go with changing the ack() behavior in v2 to await for listener completion.

[seratch]

PR #444 is going to add a new flag to enable developers to customize the behavior of ack() to await for the listener's completion. The option will be a solution for the issue described here.

[aoberoi]

Is this issue a duplicate of #361? Or can it also be closed because we've converged on a solution and the discussion is done?

[seratch]

We don't have anything further to discuss but in my opinion, we can close this issue when we merge #444 - we're almost fixing this but still in progress.

[seratch]

As mentioned in the version 2.0.0, #444 has been merged and the option called processBeforeResponse is available for FaaS users.
https://github.com/slackapi/bolt/releases/tag/%40slack%2Fbolt%402.0.0

[seratch]

Let me reopen this issue. I noticed that processBeforeReponse option doesn't work as expected. I will come up with a pull request addressing the issue.

[aoberoi]

I'd prefer to close this issue, and create a new issue to track the bug in the intended behavior. The feature is implemented, but we're now investigating a reported issue with the implementation.

[seratch]

Agreed. The issue for the bug is #462 (for reference)