Skip to content

Commit e785797

Browse files
ericholscherclaude
andauthored
OAuth: don't delete repositories when rate limited by GitHub (#13240)
A rate-limited GitHub API request [fails with a 403](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api), and `update_or_create_repositories()` treats any 403 as if we lost access to the repository — deleting the `RemoteRepository` and silently disconnecting its projects. Large installations are currently hitting the rate limit during webhook-triggered syncs, so this is actively waiting to happen. A rate limit now aborts the operation without deleting anything, and without making further doomed requests. The task still fails visibly — no retries. PyGithub raises `RateLimitExceededException` (a `GithubException` subclass) for both [primary and secondary rate limits](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#about-secondary-rate-limits), so catching it before the generic 403/404 handling covers both. Reducing the API usage that triggers the rate limits is handled separately in #13241, #13242, and #13243. Likely related to #13101, since build statuses and PR comments share the same per-installation budget. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LVjkPjH3EDEQmtywJvDVau Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8acd566 commit e785797

2 files changed

Lines changed: 39 additions & 0 deletions

File tree

readthedocs/oauth/services/githubapp.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.conf import settings
88
from github import Github
99
from github import GithubException
10+
from github import RateLimitExceededException
1011
from github.Installation import Installation as GHInstallation
1112
from github.Organization import Organization as GHOrganization
1213
from github.Repository import Repository as GHRepository
@@ -247,6 +248,15 @@ def update_or_create_repositories(self, repository_ids: list[int]):
247248
# status code if the app is not installed on the repository.
248249
if not repo.private:
249250
self.gh_app_client.get_repo_installation(owner=repo.owner.login, repo=repo.name)
251+
except RateLimitExceededException:
252+
# Being rate limited doesn't mean we lost access to the repository.
253+
# Abort the operation, all remaining requests will fail as well.
254+
log.info(
255+
"Rate limit exceeded while fetching repositories from GitHub",
256+
installation_id=self.installation.installation_id,
257+
exc_info=True,
258+
)
259+
raise
250260
except GithubException as e:
251261
log.info(
252262
"Failed to fetch repository from GitHub",

readthedocs/rtd_tests/tests/test_oauth.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
import json
33
from unittest import mock
44

5+
import pytest
56
from allauth.socialaccount.providers.bitbucket_oauth2.provider import BitbucketOAuth2Provider
67
from allauth.socialaccount.providers.gitlab.provider import GitLabProvider
78
import requests_mock
9+
from github import RateLimitExceededException
810
from allauth.socialaccount.models import SocialAccount, SocialToken
911
from allauth.socialaccount.providers.github.provider import GitHubProvider
1012
from django.conf import settings
@@ -596,6 +598,33 @@ def test_update_invalid_repository(self, request):
596598
id=self.remote_repository.id
597599
).exists()
598600

601+
@requests_mock.Mocker(kw="request")
602+
def test_update_repository_rate_limited(self, request):
603+
request.post(
604+
f"{self.api_url}/app/installations/1111/access_tokens",
605+
json=self._get_access_token_json(),
606+
)
607+
request.get(
608+
f"{self.api_url}/repositories/{self.remote_repository.remote_id}",
609+
status_code=403,
610+
json={
611+
"message": f"API rate limit exceeded for installation ID {self.installation.installation_id}.",
612+
"documentation_url": "https://docs.github.com/en/rest/using-the-rest-api/getting-started-with-the-rest-api#rate-limiting",
613+
},
614+
)
615+
616+
service = self.installation.service
617+
# The second repository isn't mocked, requesting it would fail the test,
618+
# the operation should be aborted after the first rate limited response.
619+
with pytest.raises(RateLimitExceededException):
620+
service.update_or_create_repositories(
621+
[int(self.remote_repository.remote_id), 5555]
622+
)
623+
624+
# Being rate limited doesn't mean we lost access to the repository,
625+
# it shouldn't be deleted.
626+
assert RemoteRepository.objects.filter(id=self.remote_repository.id).exists()
627+
599628
@requests_mock.Mocker(kw="request")
600629
def test_sync(self, request):
601630
assert self.installation.repositories.count() == 1

0 commit comments

Comments
 (0)