WIP: Bot: support bubbling up rate limit info #573
Conversation
This exposes Slack rate-limiting information to Hubot. This allows custom scripts to react to rate-limiting, for example to slow themselves down or to report to an alerting channel that a rate-limiting event has occurred.
|
|
| if error.code isnt -1 | ||
| @robot.emit "error", error | ||
| else |
There was a problem hiding this comment.
I can't find any official documentation of the rate-limiting code, but this test indicates that code -1 is reliably rate limiting: https://github.com/slackapi/hubot-slack/blob/master/test/bot.coffee#L168-L171
|
/cc @aoberoi |
Co-Authored-By: Nina Kaufman <[email protected]>
|
🤔 The Node v4.2 build is mysteriously failing while |
Codecov Report
@@ Coverage Diff @@
## master #573 +/- ##
==========================================
+ Coverage 85.03% 85.19% +0.15%
==========================================
Files 6 6
Lines 381 385 +4
Branches 85 85
==========================================
+ Hits 324 328 +4
Misses 33 33
Partials 24 24
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## main #573 +/- ##
==========================================
- Coverage 85.03% 85.00% -0.04%
==========================================
Files 6 6
Lines 381 500 +119
Branches 85 129 +44
==========================================
+ Hits 324 425 +101
- Misses 33 45 +12
- Partials 24 30 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@clavin Looks like it passed this time. ¯_(ツ)_/¯ |
|
@mistydemeo Thanks for following up. I can take a look at this today. BTW, I know you mentioned that your employer signed the CLA in another PR #567 (comment) |
|
Went ahead and signed the individual CLA. |
| @@ -39,6 +41,7 @@ class SlackBot extends Adapter | |||
| @client.rtm.on "close", @close | |||
| @client.rtm.on "disconnect", @disconnect | |||
| @client.rtm.on "error", @error | |||
| @client.web.on Events.WEB.RATE_LIMITED, @rate_limited | |||
There was a problem hiding this comment.
@mistydemeo Here is a sample I used for verifying this implementation.
module.exports = (robot) ->
web = robot.adapter.client.web
robot.hear /call a lot/i, (res) ->
for x in [1..100]
web.conversations.list().then (res) ->
robot.logger.info "#{res.ok} #{res.error}"
robot.on 'slack-rate-limit', (res) ->
robot.logger.info "Detected a rate-limited call #{res}"Does this really work for you? It doesn't work for me. The following is the error I get every time the rate_limited method is called. I haven't investigated details yet but it seems @robot is undefined in any case.
[Fri Apr 10 2020 13:05:29 GMT+0900 (Japan Standard Time)] ERROR TypeError: Cannot read property 'emit' of undefined
at WebAPIClient.SlackBot.rate_limited (/path-to-project/node_modules/hubot-slack/src/bot.coffee:217:5, <js>:276:25)
at WebAPIClient.emit (/path-to-project/node_modules/hubot-slack/node_modules/eventemitter3/index.js:116:35)
at handleRateLimitResponse (/path-to-project/node_modules/hubot-slack/node_modules/@slack/client/lib/clients/transports/call-transport.js:51:10)
at handleTransportResponse (/path-to-project/node_modules/hubot-slack/node_modules/@slack/client/lib/clients/transports/call-transport.js:148:21)
Also, to utilize this, you need to always use robot.adapter.client.web. If some of your scripts initialize WebClient separately (also, that's the way this adapter currently recommends), rate-limited errors those clients get won't be notified here.
Correct me if my understanding is wrong.
|
@mistydemeo Although I'm not sure if this is realistic for your use case yet, is it possible to consider using Events API for this purpose? Events API has an event for it: https://api.slack.com/events/app_rate_limited That'll be a much easier way to detect the errors. However, to use Events API along with RTM bot, you need to have a public endpoint to receive incoming requests from Slack API. Also, if you have been using Hubot integrations, you need to migrate those into "classic" Slack apps. (FYI: the default way is no longer the "classic" model but still you can create from https://api.slack.com/apps?new_classic_app=1 ). That said, I'm happy to help you out in the short-term by merging this PR once I verify it works. |
seratch
changed the title
|
Eek! Sorry 🙇!! I accidentally deleted the old default branch without changing the target of this PR and it automatically closed this PR! And I couldn't change it unless I cloned the old branch and pushed it again first! 😱 Sorry for the commotion! |
Summary
This exposes Slack rate-limiting information to Hubot. This allows custom scripts to react to rate-limiting, for example to slow themselves down or to report to an alerting channel that a rate-limiting event has occurred.
Requirements (place an
xin each[ ])