Skip to content

Commit 67bab6b

Browse files
author
Julia Hoge
authored
fix(discord): Catch case where response from Discord takes a while (#57522)
Closes #57521
1 parent 4d7291d commit 67bab6b

File tree

6 files changed

+98
-5
lines changed

6 files changed

+98
-5
lines changed

src/sentry/incidents/logic.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
from sentry.services.hybrid_cloud.integration.model import RpcOrganizationIntegration
4848
from sentry.shared_integrations.exceptions import (
4949
ApiError,
50+
ApiTimeoutError,
5051
DuplicateDisplayNameError,
5152
IntegrationError,
5253
)
@@ -1349,6 +1350,10 @@ def get_alert_rule_trigger_action_discord_channel_id(
13491350
)
13501351
except IntegrationError:
13511352
raise InvalidTriggerActionError("Bad response from Discord channel lookup")
1353+
except ApiTimeoutError:
1354+
raise ChannelLookupTimeoutError(
1355+
"Could not find channel %s. We have timed out trying to look for it." % name
1356+
)
13521357

13531358
return name
13541359

src/sentry/integrations/discord/actions/issue_alert/form.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
from sentry.integrations.discord.utils.channel import validate_channel_id
1010
from sentry.services.hybrid_cloud.integration import integration_service
11-
from sentry.shared_integrations.exceptions import IntegrationError
11+
from sentry.shared_integrations.exceptions import ApiTimeoutError, IntegrationError
1212

1313

1414
class DiscordNotifyServiceForm(forms.Form):
@@ -61,4 +61,6 @@ def clean(self) -> dict[str, object] | None:
6161
self._format_discord_error_message("; ".join(str(e))),
6262
code="invalid",
6363
)
64+
except ApiTimeoutError:
65+
raise forms.ValidationError("Discord channel lookup timed out")
6466
return cleaned_data

src/sentry/integrations/discord/utils/channel.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
from __future__ import annotations
22

33
from django.core.exceptions import ValidationError
4+
from requests.exceptions import Timeout
45

56
from sentry.integrations.discord.client import DiscordClient
6-
from sentry.shared_integrations.exceptions import IntegrationError
7+
from sentry.shared_integrations.exceptions import ApiTimeoutError, IntegrationError
78
from sentry.shared_integrations.exceptions.base import ApiError
89

910
from . import logger
@@ -65,6 +66,16 @@ def validate_channel_id(
6566
},
6667
)
6768
raise IntegrationError("Bad response from Discord channel lookup.")
69+
except Timeout:
70+
logger.info(
71+
"rule.discord.channel_lookup_timed_out",
72+
extra={
73+
"channel_id": channel_id,
74+
"guild_name": guild_name,
75+
"reason": "channel lookup timed out",
76+
},
77+
)
78+
raise ApiTimeoutError("Discord channel lookup timed out")
6879

6980
if not isinstance(result, dict):
7081
raise IntegrationError("Bad response from Discord channel lookup.")

tests/sentry/incidents/test_logic.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
WARNING_TRIGGER_LABEL,
2222
WINDOWED_STATS_DATA_POINTS,
2323
AlertRuleTriggerLabelAlreadyUsedError,
24+
ChannelLookupTimeoutError,
2425
InvalidTriggerActionError,
2526
ProjectsNotAssociatedWithAlertRuleError,
2627
create_alert_rule,
@@ -67,7 +68,7 @@
6768
)
6869
from sentry.models import ActorTuple, Integration, OrganizationIntegration
6970
from sentry.models.actor import get_actor_id_for_user
70-
from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError
71+
from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError, ApiTimeoutError
7172
from sentry.snuba.dataset import Dataset
7273
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
7374
from sentry.testutils.cases import BaseIncidentsTest, BaseMetricsTestCase, SnubaTestCase, TestCase
@@ -1911,6 +1912,48 @@ def test_discord_no_integration(self):
19111912
integration_id=None,
19121913
)
19131914

1915+
@responses.activate
1916+
@mock.patch("sentry.integrations.discord.utils.channel.validate_channel_id")
1917+
def test_discord_timeout(self, mock_validate_channel_id):
1918+
mock_validate_channel_id.side_effect = ApiTimeoutError("Discord channel lookup timed out")
1919+
1920+
base_url: str = "https://discord.com/api/v10"
1921+
channel_id = "channel-id"
1922+
guild_id = "example-discord-server"
1923+
guild_name = "Server Name"
1924+
1925+
integration = Integration.objects.create(
1926+
provider="discord",
1927+
name="Example Discord",
1928+
external_id=f"{guild_id}",
1929+
metadata={
1930+
"guild_id": f"{guild_id}",
1931+
"name": f"{guild_name}",
1932+
},
1933+
)
1934+
1935+
integration.add_organization(self.organization, self.user)
1936+
type = AlertRuleTriggerAction.Type.DISCORD
1937+
target_type = AlertRuleTriggerAction.TargetType.SPECIFIC
1938+
responses.add(
1939+
method=responses.GET,
1940+
url=f"{base_url}/channels/{channel_id}",
1941+
json={
1942+
"guild_id": f"{guild_id}",
1943+
"name": f"{guild_name}",
1944+
},
1945+
)
1946+
1947+
with self.feature("organizations:integrations-discord-metric-alerts"):
1948+
with pytest.raises(ChannelLookupTimeoutError):
1949+
update_alert_rule_trigger_action(
1950+
self.action,
1951+
type,
1952+
target_type,
1953+
target_identifier=channel_id,
1954+
integration_id=integration.id,
1955+
)
1956+
19141957

19151958
class DeleteAlertRuleTriggerAction(BaseAlertRuleTriggerActionTest, TestCase):
19161959
@cached_property

tests/sentry/integrations/discord/test_issue_alert.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from sentry.models.group import GroupStatus
1414
from sentry.models.release import Release
1515
from sentry.services.hybrid_cloud.integration import integration_service
16+
from sentry.shared_integrations.exceptions import ApiTimeoutError
1617
from sentry.testutils.cases import RuleTestCase, TestCase
1718
from sentry.testutils.helpers.datetime import before_now, iso_format
1819
from sentry.testutils.skips import requires_snuba
@@ -230,14 +231,18 @@ def test_integration_removed(self):
230231
assert len(results) == 0
231232

232233
@responses.activate
233-
@mock.patch("sentry.integrations.discord.actions.issue_alert.form.validate_channel_id")
234+
@mock.patch(
235+
"sentry.integrations.discord.actions.issue_alert.form.validate_channel_id",
236+
return_value=None,
237+
)
234238
def test_get_form_instance(self, mock_validate_channel_id):
235239
form = self.rule.get_form_instance()
236240
form.full_clean()
237241
assert form.is_valid()
238242
assert int(form.cleaned_data["server"]) == self.discord_integration.id
239243
assert form.cleaned_data["channel_id"] == self.channel_id
240244
assert form.cleaned_data["tags"] == self.tags
245+
assert mock_validate_channel_id.call_count == 1
241246

242247
@responses.activate
243248
def test_label(self):
@@ -337,3 +342,21 @@ def test_invalid_channel_id(self, mock_validate_channel_id):
337342
form.full_clean()
338343
assert not form.is_valid()
339344
assert mock_validate_channel_id.call_count == 1
345+
346+
@mock.patch(
347+
"sentry.integrations.discord.actions.issue_alert.form.validate_channel_id",
348+
side_effect=ApiTimeoutError("Discord channel lookup timed out"),
349+
)
350+
def test_channel_id_lookup_timeout(self, mock_validate_channel_id):
351+
form = DiscordNotifyServiceForm(
352+
data={
353+
"server": self.discord_integration.id,
354+
"channel_id": self.channel_id,
355+
"tags": "environment",
356+
},
357+
integrations=self.integrations,
358+
)
359+
360+
form.full_clean()
361+
assert not form.is_valid()
362+
assert mock_validate_channel_id.call_count == 1

tests/sentry/integrations/discord/test_utils.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
from django.core.exceptions import ValidationError
44
from pytest import raises
5+
from requests.exceptions import Timeout
56

67
from sentry.integrations.discord.utils.auth import verify_signature
78
from sentry.integrations.discord.utils.channel import validate_channel_id
8-
from sentry.shared_integrations.exceptions import IntegrationError
9+
from sentry.shared_integrations.exceptions import ApiTimeoutError, IntegrationError
910
from sentry.shared_integrations.exceptions.base import ApiError
1011
from sentry.testutils.cases import TestCase
1112

@@ -92,3 +93,11 @@ def test_not_guild_member(self, mock_get_channel):
9293
validate_channel_id(
9394
self.channel_id, self.guild_id, self.integration_id, self.guild_name
9495
)
96+
97+
@mock.patch("sentry.integrations.discord.utils.channel.DiscordClient.get_channel")
98+
def test_timeout(self, mock_get_channel):
99+
mock_get_channel.side_effect = Timeout("foo")
100+
with raises(ApiTimeoutError):
101+
validate_channel_id(
102+
self.channel_id, self.guild_id, self.integration_id, self.guild_name
103+
)

0 commit comments

Comments
 (0)