Fix workflow step execution sample code to not await the workflowSteps Slack API responses

[filmaj]

Since the workflowSteps.completed API may take 3+ seconds to respond, with our existing sample code on how to write workflow step execution handlers, this may lead users' apps to not respond to Event API payloads within the required 3 seconds time.

This PR fixes #1156 by not awaiting the Slack API response for the workflow step completed/failure callbacks, freeing up the event handling loop in Bolt to respond in time.

[codecov]

Codecov Report

Merging #1178 (1ffe13a) into main (2c12dba) will decrease coverage by 0.50%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1178      +/-   ##
==========================================
- Coverage   71.83%   71.33%   -0.51%     
==========================================
  Files          15       17       +2     
  Lines        1360     1399      +39     
  Branches      405      415      +10     
==========================================
+ Hits          977      998      +21     
- Misses        312      331      +19     
+ Partials       71       70       -1     

Impacted Files	Coverage Δ
src/errors.ts	98.11% <0.00%> (-1.89%)	⬇️
src/App.ts	83.63% <0.00%> (-0.27%)	⬇️
src/receivers/http-utils.ts	11.11% <0.00%> (ø)
src/types/middleware.ts	100.00% <0.00%> (ø)
src/receivers/SocketModeReceiver.ts	89.28% <0.00%> (+0.26%)	⬆️
src/receivers/ExpressReceiver.ts	60.97% <0.00%> (+0.57%)	⬆️
src/receivers/HTTPReceiver.ts	38.04% <0.00%> (+0.92%)	⬆️
src/receivers/AwsLambdaReceiver.ts	62.50% <0.00%> (+2.50%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c12dba...1ffe13a. Read the comment docs.

[stevengill]

LGTM! Nice find dude!

[seratch]

@filmaj One thing I would like to mention for people that will visit this page in the future is that, if you don't run your apps with processBeforeResponse: true option, the current code snippet does not have any issues. As the default behavior of app.event listeners in bolt-js is immediately acknowledging incoming requests before the completion of listeners. That being said, having this change in the documents would be fine. Perhaps, we can add more comments in the code. I will add a suggestion about the comments later.

Also, I left a comment to the linked issue: #1156 (comment)

[filmaj]

As the default behavior of app.event listeners in bolt-js is immediately acknowledging incoming requests before the completion of listeners.

@seratch is this true? I think one complicating factor about the processBeforeResponse: true option is that it depends on what Receiver your app uses / implements.

For example, when using the HTTPReceiver, processBeforeResponse: true actually ensures that the response is done after the listener has completed execution, not before, since we "store" the response and only return it after the eventing middleware has completed, which is how workflow steps are handled in bolt-js. Confusingly, having processBeforeResponse: false ensures that any call to ack() immediately writes the response.

For the situation in #1156, the users were using the AWSReceiver. There, ack() only handles a timer for logging a warning to the console, so the events-api-auto-acknowledgement actually does not impact how an HTTP response is returned to the Slack API, as the response is always returned after an event is processed and all middleware has executed (including the workflow step execution handle, which is treated like middleware).

[seratch]

@filmaj I mentioned the default behavior. With the default settings, processBeforeResponse is false. The AWS Lambda receiver is an exception. It works only with the true option.

[filmaj]

@seratch I see, you are right of course, the workflow examples apply to all different kinds of Receivers with processBeforeResponse set to both true and false. I lost the perspective on what a 'default' bolt setup when working on #1156, thanks for your reminder. With that in mind, I will review and change this PR to ensure the documentation applies cleanly to all situations: FaaS or not (i.e. different Receivers) as well as processBeforeResponse set to true or false.

[filmaj]

I expanded on your suggestions @seratch. What do you think?

I was also trying to see if perhaps the AWS Deployment guide in the docs should be updated to specifically mention this issue, but couldn't think of how to naturally incorporate that into the structure of the document. Alternatively, perhaps the "Workflow steps" document introduction could use a clarifying section discussing how the workfow step event handlers need to be kept performant when used in conjunction with processBeforeResponse?

[seratch]

Thanks for updating the comments!

[seratch]

@filmaj All good 👍 I just added a suggestion on the Japanese comment. Can you have the change as well before merging this PR?