Skip to content

Commit a85b104

Browse files
iamrajjoshiandrewshie-sentry
authored andcommitted
🔧 chore: make retry errors for gitlab record as halt take 2 (#87785)
take 2 of #87578, `base_url` is on client.
1 parent b82e064 commit a85b104

File tree

4 files changed

+79
-2
lines changed

4 files changed

+79
-2
lines changed

src/sentry/integrations/gitlab/client.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,5 +369,9 @@ def get_blame_for_files(
369369
return fetch_file_blames(
370370
self,
371371
files,
372-
extra={**extra, "provider": "gitlab", "org_integration_id": self.org_integration_id},
372+
extra={
373+
**extra,
374+
"provider": "gitlab",
375+
"org_integration_id": self.org_integration_id,
376+
},
373377
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
GITLAB_CLOUD_BASE_URL = "https://gitlab.com"

src/sentry/integrations/source_code_management/commit_context.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
from sentry import analytics
1414
from sentry.auth.exceptions import IdentityNotValid
15+
from sentry.integrations.gitlab.constants import GITLAB_CLOUD_BASE_URL
1516
from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig
1617
from sentry.integrations.source_code_management.metrics import (
1718
CommitContextHaltReason,
@@ -32,7 +33,11 @@
3233
PullRequestCommit,
3334
)
3435
from sentry.models.repository import Repository
35-
from sentry.shared_integrations.exceptions import ApiInvalidRequestError, ApiRateLimitedError
36+
from sentry.shared_integrations.exceptions import (
37+
ApiInvalidRequestError,
38+
ApiRateLimitedError,
39+
ApiRetryError,
40+
)
3641
from sentry.users.models.identity import Identity
3742
from sentry.utils import metrics
3843
from sentry.utils.cache import cache
@@ -131,6 +136,17 @@ def get_blame_for_files(
131136
return []
132137
else:
133138
raise
139+
except ApiRetryError as e:
140+
# Ignore retry errors for GitLab
141+
# TODO(ecosystem): Remove this once we have a better way to handle this
142+
if (
143+
self.integration_name == ExternalProviderEnum.GITLAB.value
144+
and client.base_url != GITLAB_CLOUD_BASE_URL
145+
):
146+
lifecycle.record_halt(e)
147+
return []
148+
else:
149+
raise
134150
return response
135151

136152
def get_commit_context_all_frames(
@@ -377,6 +393,8 @@ def create_or_update_comment(
377393

378394

379395
class CommitContextClient(ABC):
396+
base_url: str
397+
380398
@abstractmethod
381399
def get_blame_for_files(
382400
self, files: Sequence[SourceLineInfo], extra: Mapping[str, Any]

tests/sentry/integrations/source_code_management/test_commit_context_slo.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import pytest
44

5+
from sentry.integrations.gitlab.constants import GITLAB_CLOUD_BASE_URL
56
from sentry.integrations.source_code_management.commit_context import (
67
CommitContextIntegration,
78
SourceLineInfo,
@@ -20,6 +21,7 @@ class MockCommitContextIntegration(CommitContextIntegration):
2021

2122
def __init__(self):
2223
self.client = Mock()
24+
self.client.base_url = "https://example.com"
2325

2426
def get_client(self):
2527
return self.client
@@ -122,6 +124,58 @@ class MockGitlabIntegration(MockCommitContextIntegration):
122124
assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)
123125
assert_halt_metric(mock_record, ApiInvalidRequestError(text="Invalid request"))
124126

127+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
128+
def test_get_blame_for_files_retry_error(self, mock_record):
129+
"""Test retry error for Gitlab Self-hosted records halt"""
130+
from sentry.shared_integrations.exceptions import ApiRetryError
131+
132+
# Because this is Gitlab Self-hosted, this should be halt
133+
class MockGitlabIntegration(MockCommitContextIntegration):
134+
integration_name = "gitlab"
135+
base_url = "https://bufo-bot.gitlab.com"
136+
137+
def __init__(self):
138+
super().__init__()
139+
self.client.base_url = self.base_url
140+
141+
self.integration = MockGitlabIntegration()
142+
143+
self.integration.client.get_blame_for_files = Mock(
144+
side_effect=ApiRetryError(text="Host error")
145+
)
146+
147+
result = self.integration.get_blame_for_files([self.source_line], {})
148+
149+
assert result == []
150+
assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)
151+
assert_halt_metric(mock_record, ApiRetryError(text="Host error"))
152+
153+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
154+
def test_get_blame_for_files_retry_error_gitlab(self, mock_record):
155+
"""Test retry error for GitLab saas records failure"""
156+
from sentry.shared_integrations.exceptions import ApiRetryError
157+
158+
# Because this is Gitlab SAAS, this should be failure
159+
class MockGitlabIntegration(MockCommitContextIntegration):
160+
integration_name = "gitlab"
161+
base_url = GITLAB_CLOUD_BASE_URL
162+
163+
def __init__(self):
164+
super().__init__()
165+
self.client.base_url = self.base_url
166+
167+
self.integration = MockGitlabIntegration()
168+
169+
self.integration.client.get_blame_for_files = Mock(
170+
side_effect=ApiRetryError(text="Host error")
171+
)
172+
173+
with pytest.raises(ApiRetryError):
174+
self.integration.get_blame_for_files([self.source_line], {})
175+
176+
assert_slo_metric(mock_record, EventLifecycleOutcome.FAILURE)
177+
assert_failure_metric(mock_record, ApiRetryError(text="Host error"))
178+
125179
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
126180
def test_get_commit_context_all_frames(self, mock_record):
127181
"""Test get_commit_context_all_frames records correct lifecycle events"""

0 commit comments

Comments
 (0)