Skip to content

Commit bca85bb

Browse files
wedamijacleptric
authored andcommitted
fix(repositories): Silence github errors when performing auto-sync (#114650)
This more explicitly handles some github related errors and allows us to silence them Fixes SENTRY-5NAF
1 parent a69c073 commit bca85bb

3 files changed

Lines changed: 110 additions & 6 deletions

File tree

src/sentry/integrations/github/integration.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,12 @@ def is_rate_limited_error(self, exc: ApiError) -> bool:
223223
return False
224224

225225
def is_broken_integration_error(self, exc: Exception) -> HaltReason | None:
226-
if isinstance(exc, ApiForbiddenError) and "suspended" in str(exc):
227-
return "installation_suspended"
226+
if isinstance(exc, ApiForbiddenError):
227+
if self.is_rate_limited_error(exc):
228+
return "rate_limited"
229+
if "suspended" in str(exc):
230+
return "installation_suspended"
231+
return "unauthorized"
228232
return super().is_broken_integration_error(exc)
229233

230234
def message_from_error(self, exc: Exception) -> str:

src/sentry/integrations/github_enterprise/integration.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,23 @@ def get_client(self):
219219

220220
# IntegrationInstallation methods
221221

222+
def is_rate_limited_error(self, exc: ApiError) -> bool:
223+
if (
224+
exc.json
225+
and isinstance(exc.json, dict)
226+
and RATE_LIMITED_MESSAGE in exc.json.get("message", "")
227+
):
228+
metrics.incr("github_enterprise.link_all_repos.rate_limited_error")
229+
return True
230+
return False
231+
222232
def is_broken_integration_error(self, exc: Exception) -> HaltReason | None:
223-
if isinstance(exc, ApiForbiddenError) and "suspended" in str(exc):
224-
return "installation_suspended"
233+
if isinstance(exc, ApiForbiddenError):
234+
if self.is_rate_limited_error(exc):
235+
return "rate_limited"
236+
if "suspended" in str(exc):
237+
return "installation_suspended"
238+
return "unauthorized"
225239
return super().is_broken_integration_error(exc)
226240

227241
def message_from_error(self, exc: Exception) -> str:

tests/sentry/integrations/source_code_management/test_sync_repos.py

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -930,8 +930,11 @@ def test_api_unauthorized(self) -> None:
930930
== "unauthorized"
931931
)
932932

933-
def test_api_forbidden_not_terminal(self) -> None:
934-
assert self.installation.is_broken_integration_error(ApiForbiddenError("forbidden")) is None
933+
def test_api_forbidden_returns_unauthorized(self) -> None:
934+
assert (
935+
self.installation.is_broken_integration_error(ApiForbiddenError("forbidden"))
936+
== "unauthorized"
937+
)
935938

936939
def test_api_forbidden_suspended_returns_installation_suspended(self) -> None:
937940
exc = ApiForbiddenError('{"message":"This installation has been suspended"}')
@@ -1075,6 +1078,89 @@ def test_base_class_cases_still_work(self) -> None:
10751078
assert self.installation.is_broken_integration_error(RuntimeError("boom")) is None
10761079

10771080

1081+
@control_silo_test
1082+
@patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1")
1083+
class GitHubIsBrokenIntegrationErrorTestCase(IntegrationTestCase):
1084+
"""Tests for the GitHubIntegration.is_broken_integration_error override."""
1085+
1086+
provider = GitHubIntegrationProvider
1087+
base_url = "https://api.github.com"
1088+
key = "github"
1089+
1090+
def setUp(self) -> None:
1091+
super().setUp()
1092+
self.installation = self.integration.get_installation(organization_id=self.organization.id)
1093+
1094+
def test_forbidden_ip_allow_list_returns_unauthorized(self, _: MagicMock) -> None:
1095+
exc = ApiForbiddenError(
1096+
'{"message":"the org has an IP allow list enabled, '
1097+
'and your IP address is not permitted"}'
1098+
)
1099+
assert self.installation.is_broken_integration_error(exc) == "unauthorized"
1100+
1101+
def test_forbidden_suspended_returns_installation_suspended(self, _: MagicMock) -> None:
1102+
exc = ApiForbiddenError('{"message":"This installation has been suspended"}')
1103+
assert self.installation.is_broken_integration_error(exc) == "installation_suspended"
1104+
1105+
def test_forbidden_generic_returns_unauthorized(self, _: MagicMock) -> None:
1106+
exc = ApiForbiddenError("some other 403")
1107+
assert self.installation.is_broken_integration_error(exc) == "unauthorized"
1108+
1109+
def test_rate_limited_forbidden_returns_rate_limited(self, _: MagicMock) -> None:
1110+
exc = ApiForbiddenError('{"message":"API rate limit exceeded"}')
1111+
assert self.installation.is_broken_integration_error(exc) == "rate_limited"
1112+
1113+
def test_base_class_cases_still_work(self, _: MagicMock) -> None:
1114+
assert (
1115+
self.installation.is_broken_integration_error(ApiUnauthorized("bad token"))
1116+
== "unauthorized"
1117+
)
1118+
assert self.installation.is_broken_integration_error(RuntimeError("boom")) is None
1119+
1120+
1121+
@control_silo_test
1122+
class GHEIsBrokenIntegrationErrorTestCase(TestCase):
1123+
"""Tests for the GitHubEnterpriseIntegration.is_broken_integration_error override."""
1124+
1125+
def setUp(self) -> None:
1126+
super().setUp()
1127+
GitHubEnterpriseIntegrationProvider().setup()
1128+
self.integration = self.create_integration(
1129+
organization=self.organization,
1130+
external_id="35.232.149.196:99999",
1131+
provider="github_enterprise",
1132+
metadata={
1133+
"domain_name": "35.232.149.196/testorg",
1134+
"installation_id": "99999",
1135+
"installation": {
1136+
"id": "2",
1137+
"private_key": "private_key",
1138+
"verify_ssl": True,
1139+
},
1140+
},
1141+
)
1142+
self.installation = self.integration.get_installation(organization_id=self.organization.id)
1143+
1144+
def test_forbidden_returns_unauthorized(self) -> None:
1145+
exc = ApiForbiddenError("IP allow list")
1146+
assert self.installation.is_broken_integration_error(exc) == "unauthorized"
1147+
1148+
def test_forbidden_suspended_returns_installation_suspended(self) -> None:
1149+
exc = ApiForbiddenError('{"message":"This installation has been suspended"}')
1150+
assert self.installation.is_broken_integration_error(exc) == "installation_suspended"
1151+
1152+
def test_rate_limited_forbidden_returns_rate_limited(self) -> None:
1153+
exc = ApiForbiddenError('{"message":"API rate limit exceeded"}')
1154+
assert self.installation.is_broken_integration_error(exc) == "rate_limited"
1155+
1156+
def test_base_class_cases_still_work(self) -> None:
1157+
assert (
1158+
self.installation.is_broken_integration_error(ApiUnauthorized("bad token"))
1159+
== "unauthorized"
1160+
)
1161+
assert self.installation.is_broken_integration_error(RuntimeError("boom")) is None
1162+
1163+
10781164
@control_silo_test
10791165
@patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1")
10801166
class SyncReposForOrgNewErrorHandlingTestCase(IntegrationTestCase):

0 commit comments

Comments
 (0)