Fix #759 #1109 #1110 by adding custom properties in ReceiverEvent and Context objets #1177
Conversation
…ies in ReceiverEvent and Context objects
seratch
requested review from
filmaj,
misscoded,
srajiang,
mwbrooks and
stevengill
examples/custom-properties/app.js
Outdated
| let receiver; | ||
| receiver = new HTTPReceiver({ | ||
| signingSecret: process.env.SLACK_SIGNING_SECRET, | ||
| customPropertiesExtractor: (req) => { |
There was a problem hiding this comment.
I know this name is a bit verbose. Does anyone have a great idea for the naming?
| @@ -43,14 +43,14 @@ | |||
| "dependencies": { | |||
| "@slack/logger": "^3.0.0", | |||
| "@slack/oauth": "^2.3.0", | |||
| "@slack/socket-mode": "^1.1.0", | |||
| "@slack/socket-mode": "^1.2.0", | |||
There was a problem hiding this comment.
This is required for extracting retry_num and retry_attempt value from payload
| "@slack/types": "^2.2.0", | ||
| "@slack/web-api": "^6.4.0", | ||
| "@types/express": "^4.16.1", | ||
| "@types/node": ">=12", | ||
| "@types/promise.allsettled": "^1.0.3", | ||
| "@types/tsscmp": "^1.0.0", | ||
| "axios": "^0.21.2", | ||
| "axios": "^0.21.4", |
src/receivers/AwsLambdaReceiver.ts
Outdated
| @@ -168,6 +168,10 @@ export default class AwsLambdaReceiver implements Receiver { | |||
| storedResponse = response; | |||
| } | |||
| }, | |||
| retryNum: this.getHeaderValue(awsEvent.headers, 'X-Slack-Retry-Num') as number | undefined, | |||
| retryReason: this.getHeaderValue(awsEvent.headers, 'X-Slack-Retry-Reason'), | |||
| // TODO | |||
Codecov Report
@@ Coverage Diff @@
## main #1177 +/- ##
==========================================
- 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
Continue to review full report at Codecov.
|
|
After having reviews from other maintainers, I can add unit tests to improve the coverage. |
examples/custom-properties/app.js
Outdated
| let receiver; | ||
| receiver = new HTTPReceiver({ | ||
| signingSecret: process.env.SLACK_SIGNING_SECRET, | ||
| customPropertiesExtractor: (req) => { |
| ...authorizeResult, | ||
| ...event.customProperties, | ||
| retryNum: event.retryNum, | ||
| retryReason: event.retryReason, |
There was a problem hiding this comment.
What is the purpose of explicitly adding the retryNum and retryReason keys to the context object here? It looks like these two keys are part of the "built in" keys, so the check above should throw is these are reused by the custom properties, right?
There was a problem hiding this comment.
These two properties are built-in ones but they are supposed to be sent as part of the receiver event data, not from the authorize function. Thus, we set these this way.
It looks like these two keys are part of the "built in" keys, so the check above should throw is these are reused by the custom properties, right?
Yes, your understanding here is correct.
The most common use case would be propagating HTTP request headers for app tracing and so on. If you invoke another backend services from Bolt middleware/listeners, you may want to set the same x-request-id or something like that to those distributed backends. The other possible scenario would be to pass the host/app instance specific data to Bolt middleware/listeners. |
seratch
force-pushed
the
issue-759-custom-props
branch
from
0bb00a5 to
5e4db12
Compare
|
Hi there! I was wondering when will this changes be released? |
|
@ClaudioRizzo Thanks for your interest! We cannot tell when we can release the next version but it won't take long at all! If everything goes well, we will release v3.8 next week or so. |
Summary
This pull request adds custom properties to
ReceiverEventandContextobjects.Fixes #759
Fixes #1109
Fixes #1110
Checking the added example app would be the quickest way to learn the benefits by the change.
Requirements (place an
xin each[ ])