Fix #586 hubot-slack calling bots.info a suspicious number of times

[seratch]

Summary

This pull request fixes #586 by changing the internals of SlackClient a bit.

The latest RTM event payloads from Slack has both user and bot_id even for events generated by a bot user as below:

{
  "bot_id": "B12345678",
  "suppress_notification": false,
  "type": "message",
  "text": "Badgers? BADGERS? WE DON'T NEED NO STINKIN BADGERS",
  "user": "U12345678",
  "team": "T12345678",
  "bot_profile": {
    "id": "B12345678",
    "deleted": false,
    "name": "hubot",
    "updated": 1568973776,
    "app_id": "A12345678",
    "icons": {
      "image_36": "https://a.slack-edge.com/80588/img/services/hubot_36.png",
      "image_48": "https://a.slack-edge.com/80588/img/services/hubot_48.png",
      "image_72": "https://a.slack-edge.com/80588/img/services/hubot_72.png"
    },
    "team_id": "T12345678"
  },
  "source_team": "T12345678",
  "user_team": "T12345678",
  "channel": "C12345678",
  "event_ts": "1585723843.003300",
  "ts": "1585723843.003300"
}

In such cases, SlackClient's @botUserIdMap (the internal memory cache of bot user information) is not updated. That's the reason why the cache doesn't work as expected and a large number of bots.info calls may occur when receiving bot user events.

Requirements (place an x in each [ ])

• I've read and understood the Contributing Guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.

[codecov]

Codecov Report

Merging #588 into master will decrease coverage by 0.10%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master     #588      +/-   ##
==========================================
- Coverage   85.03%   84.93%   -0.11%     
==========================================
  Files           6        6              
  Lines         381      385       +4     
  Branches       85       85              
==========================================
+ Hits          324      327       +3     
- Misses         33       34       +1     
  Partials       24       24              

Impacted Files	Coverage Δ
src/client.coffee	91.15% <71.42%> (-0.60%)	⬇️

---

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 933bafc...f6b3e7f. Read the comment docs.

[aoberoi]

with this change, when the event has a bot_id we will not make a call to Web API method users.info, and therefore there won't be a value for fetched.user later.

this seems safe because there's no place in the branch of code where fetched.bot is assigned, that the value of fetched.user is used. 👍

but i am wondering, how will this reduce the number of calls to bots.info?

[seratch]

@aoberoi

but i am wondering, how will this reduce the number of calls to bots.info?

The root cause of the increase of bots.info calls is lack of the timing to run the following lines with the latest payloads.

• https://github.com/seratch/hubot-slack/blob/0a063513c4a8c7b53d6481d1b41d35f58622e208/src/client.coffee#L367
• https://github.com/seratch/hubot-slack/blob/0a063513c4a8c7b53d6481d1b41d35f58622e208/src/client.coffee#L372

The combination of the latest payloads and current SlackClient implementation keeps the @botUserIdMap empty in any case. So, fetchBotUser method calls bots.info API every time receiving an event with both bot_id and user.

[aoberoi]

ohhh i see! So now that event.user is in every bot message, fetched.user always has a value, and then botUserIdMap is never updated. This change makes it so fetched.user will not have a value when both event.bot and event.user are present. 👍

[mistydemeo]

Let me know when you'd like me to test this!

[seratch]

@mistydemeo Thanks a lot for your help on this! I just released version 4.7.2. Could you try it out when you have a chance?

[mistydemeo]

I've deployed 4.7.2, and it's behaving normally. We deployed at 11:15 PDT; can you please take a look at our API usage and see if it's reduced since then?

[seratch]

@mistydemeo Thank you! After your deployment, we're seeing the significant decrease in bots.info API calls in data. In the case where you need more help on related matters, I'll be closely working with the support team!