Skip to content
This repository was archived by the owner on May 27, 2025. It is now read-only.

WIP: Bot: support bubbling up rate limit info #573

Open
mistydemeo wants to merge 3 commits into
base: main
Choose a base branch
from
Open

WIP: Bot: support bubbling up rate limit info #573

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d921105
Bot: support bubbling up rate limit info
mistydemeo Aug 2, 2019
b374e16
Fix spacing
mistydemeo Aug 2, 2019
ec3b4c8
Move CI install to `install` & add verbosity
Aug 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/bot.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
SlackClient = require "./client"
pkg = require "../package"

Events = require("@slack/client").CLIENT_EVENTS

class SlackBot extends Adapter

###*
Expand Down Expand Up @@ -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
Copy link
Contributor

@seratch seratch Apr 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@client.rtm.on "authenticated", @authenticated
@client.onEvent @eventHandler

Expand Down Expand Up @@ -206,9 +209,20 @@ class SlackBot extends Adapter
error: (error) =>
@robot.logger.error "Slack RTM error: #{JSON.stringify error}"
# Assume that scripts can handle slowing themselves down, all other errors are bubbled up through Hubot
# NOTE: should rate limit errors also bubble up?
if error.code isnt -1
@robot.emit "error", error
else
Copy link
Contributor Author

@mistydemeo mistydemeo Aug 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# The actual duration of the rate limit isn't exposed. :(
@rate_limited(-1)

###*
# Bubbles up rate-limit info, allowing the bot to react
#
# @private
# @param {number seconds}
###
rate_limited: (seconds) ->
@robot.emit "slack-rate-limit", seconds

###*
# Incoming Slack event handler
Expand Down