diff --git a/src/sentry/integrations/github/client.py b/src/sentry/integrations/github/client.py index 4cd9ca12f27d55..a8b7d32a930c50 100644 --- a/src/sentry/integrations/github/client.py +++ b/src/sentry/integrations/github/client.py @@ -31,7 +31,12 @@ from sentry.models.pullrequest import PullRequest, PullRequestComment from sentry.models.repository import Repository from sentry.shared_integrations.client.proxy import IntegrationProxyClient -from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError, UnknownHostError +from sentry.shared_integrations.exceptions import ( + ApiError, + ApiForbiddenError, + ApiRateLimitedError, + UnknownHostError, +) from sentry.silo.base import control_silo_function from sentry.utils import metrics @@ -41,6 +46,24 @@ # as the lower ceiling before hitting Github anymore, thus, leaving at least these # many requests left for other features that need to reach Github MINIMUM_REQUESTS = 200 +_SECONDARY_RATE_LIMIT_PHRASE = "secondary rate limit" + + +def _is_secondary_rate_limit_error(error: ApiForbiddenError) -> bool: + """ + Detect GitHub "secondary rate limit" responses which are returned as HTTP 403s. + """ + + candidates: list[str] = [] + if isinstance(getattr(error, "json", None), Mapping): + message = error.json.get("message") + if isinstance(message, str): + candidates.append(message) + if isinstance(error.text, str): + candidates.append(error.text) + combined = " ".join(candidates).lower() + return _SECONDARY_RATE_LIMIT_PHRASE in combined + JWT_AUTH_ROUTES = ("/app/installations", "access_tokens") @@ -670,6 +693,21 @@ def get_blame_for_files( except ValueError as e: logger.exception(str(e), log_info) return [] + except ApiForbiddenError as e: + if _is_secondary_rate_limit_error(e): + metrics.incr("integrations.github.get_blame_for_files.secondary_rate_limit") + logger.warning( + "sentry.integrations.github.get_blame_for_files.secondary_rate_limit", + extra=log_info, + ) + raise ApiRateLimitedError( + e.text, + code=e.code, + url=e.url, + host=e.host, + path=e.path, + ) from e + raise else: self.set_cache(cache_key, response, 60) diff --git a/tests/sentry/integrations/github/test_client.py b/tests/sentry/integrations/github/test_client.py index a2be918f5e2a0f..ded7dcaeb1b01f 100644 --- a/tests/sentry/integrations/github/test_client.py +++ b/tests/sentry/integrations/github/test_client.py @@ -1752,6 +1752,49 @@ def test_rate_limit_exceeded(self, get_jwt, mock_logger_error) -> None: }, ) + @mock.patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1") + @responses.activate + def test_secondary_rate_limit_treated_as_rate_limited(self, get_jwt) -> None: + responses.reset() + responses.add( + method=responses.POST, + url="https://api.github.com/app/installations/1/access_tokens", + body='{"token": "12345token", "expires_at": "2030-01-01T00:00:00Z"}', + status=200, + content_type="application/json", + ) + responses.add( + method=responses.GET, + url="https://api.github.com/rate_limit", + body=orjson.dumps( + { + "resources": { + "graphql": { + "limit": 5000, + "used": 100, + "remaining": 4000, + "reset": 1613064000, + } + } + } + ).decode(), + status=200, + content_type="application/json", + ) + responses.add( + method=responses.POST, + url="https://api.github.com/graphql", + json={ + "message": "You have exceeded a secondary rate limit. Please wait a few minutes before you try again.", + "documentation_url": "https://docs.github.com/en/rest/overview/rate-limits-for-the-rest-api", + }, + status=403, + content_type="application/json", + ) + + with pytest.raises(ApiRateLimitedError): + self.github_client.get_blame_for_files([self.file], extra={}) + @mock.patch("sentry.integrations.github.client.get_jwt", return_value="jwt_token_1") @responses.activate def test_no_rate_limiting(self, get_jwt) -> None: