Skip to content

Commit

Permalink
Merge pull request #3440 from grafana/hotfix-slack-ratelimit
Browse files Browse the repository at this point in the history
Fix slack ratelimit
  • Loading branch information
Konstantinov-Innokentii committed Nov 28, 2023
2 parents ba6844e + 62a5b21 commit 5ef675d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 20 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased
## v1.3.64 (2023-11-28)

### Fixed

- Fix excess usage of bots_info slack api call to avoid ratelimits ([#3440](https://github.com/grafana/oncall/pull/3440))

## v1.3.63 (2023-11-28)

Expand Down
48 changes: 29 additions & 19 deletions engine/apps/slack/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,21 @@ def post(self, request):

if payload_event:
if payload_event_user and slack_team_identity:
if payload_event_bot_id and payload_event_bot_id == slack_team_identity.bot_id:
"""
messages from slack apps have both user and bot_id in the payload:
{...
"bot_id":"BSVC95WJZ",
"type":"message",
"text":"HELLO",
"user":"USX7UADC7",
"ts":"1701082318.471149",
"app_id":"ASUTJU5U4",
...}
So check bot_id even if payload has a user to not to react on own bot messages.
"""
return Response(status=200)

if "id" in payload_event_user:
slack_user_id = payload_event_user["id"]
elif type(payload_event_user) is str:
Expand All @@ -219,27 +234,22 @@ def post(self, request):
elif (
payload_event_bot_id and slack_team_identity and payload_event_channel_type == EventType.MESSAGE_CHANNEL
):
response = sc.bots_info(bot=payload_event_bot_id)
bot_user_id = response.get("bot", {}).get("user_id", "")

# test if we can use user from authorizations instead of api call
payload_bot_user_id = payload_event.get("authorizations", {}).get("user_id")
logger.info(
f"checkin_bot_user_id equal={payload_bot_user_id==slack_team_identity.bot_user_id}"
f" payload_bot_user_id={payload_bot_user_id} reqiest_bot_user_id={bot_user_id}"
f" sti_bot_user_id={slack_team_identity.bot_user_id}"
)

# test if we can use bot_id instead of api call
logger.info(
f"checking_bot_id equal={payload_event_bot_id == slack_team_identity.bot_id}"
f" payload_bot_id={payload_event_bot_id} sti_bot_id={slack_team_identity.bot_id}"
)

"""
Another case of incoming messages from bots. These payloads has only bot_id, but no user field:
{..
"type":"message",
"subtype":"bot_message",
"text":"",
"ts":"1701143460.869349",
"username":"some_bot_username",
...}
It looks like it's a payload from legacy slack "Incoming Webhooks" integration
https://raintank-corp.slack.com/apps/A0F7XDUAZ-incoming-webhooks?tab=more_info
"""
# Don't react on own bot's messages.
if bot_user_id == slack_team_identity.bot_user_id:
if payload_event_bot_id == slack_team_identity.bot_id:
return Response(status=200)

elif payload_event_message_user:
slack_user_id = payload_event_message_user
# event subtype 'message_deleted'
Expand Down

0 comments on commit 5ef675d

Please sign in to comment.