Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion src/sentry/integrations/github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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")

Expand Down Expand Up @@ -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)

Expand Down
43 changes: 43 additions & 0 deletions tests/sentry/integrations/github/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading