feat: extend ack timeout for function executed events#2645
feat: extend ack timeout for function executed events#2645WilliamBergamin merged 9 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2645 +/- ##
==========================================
+ Coverage 93.42% 93.44% +0.01%
==========================================
Files 37 37
Lines 7636 7668 +32
Branches 666 669 +3
==========================================
+ Hits 7134 7165 +31
- Misses 497 498 +1
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/receivers/HTTPResponseAck.ts
Outdated
| this.isAcknowledged = false; | ||
| this.processBeforeResponse = args.processBeforeResponse; | ||
| this.unhandledRequestHandler = args.unhandledRequestHandler ?? httpFunc.defaultUnhandledRequestHandler; | ||
| this.unhandledFunctionRequestTimeoutMillis = 10001; |
There was a problem hiding this comment.
Since we might want to change the timeout in webapp, do we want to start with something smaller like 5sec?
cchensh
left a comment
There was a problem hiding this comment.
The change LGTM and it's a simple solution for now. Personally I'm fine with starting with 10 secs but we do want to keep boltjs, bolt python and webapp timeout consistent - it's worth documenting somewhere for the future
|
Yo yo yo |
|
Guys I'm working on a large scale whatsapp bot |
zimeg
left a comment
There was a problem hiding this comment.
@WilliamBergamin LGTM! This is a nice approach to make certain listeners more flexible in response to function events 🔮
One suggestion before merging might change the warning outputs here to use a variable "seconds" value instead of a hardcoded 3? I missed the change from "10" to "5" seconds and was wondering if these changes were being used at first...
bolt-js/src/receivers/HTTPModuleFunctions.ts
Lines 209 to 217 in 44ceb2f
| * 2. Refactor Bolt App and Receivers to implement proper Request and Response abstractions: | ||
| * - Receivers should translate their specific request types to standardized Bolt Requests/Responses | ||
| * - All acknowledgment behaviors and default routing should be handled by the App, not the receivers | ||
| * - Prevent multiple request body parsing happening both here and again in the App |
There was a problem hiding this comment.
⭐ praise: The idea of bolt requests and responses I think is so good as a shared interface. IIRC some oauth receivers could use this too?
Summary
This PR aims to extend the default ack timeout from 3 seconds to 10 seconds for
function_executedevents received over HTTP and Express receiverTesting
lack create -t slack-samples/bolt-ts-custom-step-templateunhandledRequestHandler, does not print an error and does not response to the request with a 404Requirements (place an
xin each[ ])