Skip to content

[8.17] Attempt to retry 403 errors when they are actually secondary throttling limit (#3358) #3383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
artem-shelkovnikov merged 1 commit into from
Apr 22, 2025
Merged

[8.17] Attempt to retry 403 errors when they are actually secondary throttling limit (#3358) #3383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions NOTICE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7432,11 +7432,11 @@ made under the terms of *both* these licenses.


soupsieve
2.6
2.7
MIT License
MIT License

Copyright (c) 2018 - 2024 Isaac Muse <[email protected]>
Copyright (c) 2018 - 2025 Isaac Muse <[email protected]>

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down
47 changes: 19 additions & 28 deletions connectors/sources/github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

import aiohttp
import fastjsonschema
from aiohttp.client_exceptions import ClientResponseError
from gidgethub import QueryError, RateLimitExceeded, sansio
import gidgethub
from gidgethub import QueryError, sansio
from gidgethub.abc import (
BadGraphQLRequest,
GraphQLAuthorizationFailure,
Expand Down Expand Up @@ -687,9 +687,6 @@ def __init__(
def set_logger(self, logger_):
self._logger = logger_

def get_rate_limit_encountered(self, status_code, rate_limit_remaining):
return status_code == FORBIDDEN and not int(rate_limit_remaining)

async def _get_retry_after(self, resource_type):
current_time = time.time()
response = await self.get_github_item("/rate_limit")
Expand Down Expand Up @@ -737,7 +734,7 @@ async def _update_installation_access_token(self):
private_key=self.private_key,
)
self._installation_access_token = access_token_response["token"]
except RateLimitExceeded:
except gidgethub.RateLimitExceeded:
await self._put_to_sleep("core")
except Exception:
self._logger.exception(
Expand All @@ -752,7 +749,7 @@ def _get_session(self):
timeout = aiohttp.ClientTimeout(total=None)
return aiohttp.ClientSession(
timeout=timeout,
raise_for_status=True,
raise_for_status=False,
connector=connector,
)

Expand Down Expand Up @@ -841,8 +838,10 @@ async def get_github_item(self, resource):
return await self._get_client.getitem(
url=resource, oauth_token=self._access_token()
)
except ClientResponseError as exception:
if exception.status == UNAUTHORIZED:
except gidgethub.RateLimitExceeded:
await self._put_to_sleep("core")
except gidgethub.HTTPException as exception:
if exception.status_code == UNAUTHORIZED:
if self.auth_method == GITHUB_APP:
self._logger.debug(
f"The access token for installation #{self._installation_id} expired, Regenerating a new token."
Expand All @@ -851,18 +850,15 @@ async def get_github_item(self, resource):
raise
msg = "Your Github token is either expired or revoked. Please check again."
raise UnauthorizedException(msg) from exception
elif self.get_rate_limit_encountered(
exception.status, exception.headers.get("X-RateLimit-Remaining")
):
await self._put_to_sleep(resource_type="core")
elif exception.status == FORBIDDEN:
elif exception.status_code == FORBIDDEN:
msg = f"Provided GitHub token does not have the necessary permissions to perform the request for the URL: {resource}."
raise ForbiddenException(msg) from exception
else:
raise
except RateLimitExceeded:
await self._put_to_sleep("core")
except Exception:
except Exception as e:
self._logger.debug(
f"An unexpected error occurred while getting GitHub item: {resource}. Error: {e}"
)
raise

async def paginated_api_call(self, query, variables, keys):
Expand Down Expand Up @@ -912,16 +908,11 @@ async def get_personal_access_token_scopes(self):
)
return set()
return {scope.strip() for scope in scopes.split(",")}
except ClientResponseError as exception:
if exception.status == FORBIDDEN:
if self.get_rate_limit_encountered(
exception.status, exception.headers.get("X-RateLimit-Remaining")
):
await self._put_to_sleep("graphql")
else:
msg = f"Provided GitHub token does not have the necessary permissions to perform the request for the URL: {self.base_url}."
raise ForbiddenException(msg) from exception
elif exception.status == UNAUTHORIZED:
except gidgethub.HTTPException as exception:
if exception.status_code == FORBIDDEN:
msg = f"Provided GitHub token does not have the necessary permissions to perform the request for the URL: {self.base_url}."
raise ForbiddenException(msg) from exception
elif exception.status_code == UNAUTHORIZED:
msg = "Your Github token is either expired or revoked. Please check again."
raise UnauthorizedException(msg) from exception
else:
Expand All @@ -944,7 +935,7 @@ async def _github_app_get(self, url):
get_jwt(app_id=self.app_id, private_key=self.private_key),
)
# we don't expect any 401 error as the jwt is freshly generated
except RateLimitExceeded:
except gidgethub.RateLimitExceeded:
await self._put_to_sleep("core")
except Exception:
raise
Expand Down
69 changes: 14 additions & 55 deletions tests/sources/test_github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
from http import HTTPStatus
from unittest.mock import ANY, AsyncMock, Mock, patch

import aiohttp
import gidgethub
import pytest
from aiohttp.client_exceptions import ClientResponseError
from gidgethub.abc import BadGraphQLRequest, GraphQLAuthorizationFailure, QueryError

from connectors.access_control import DLS_QUERY
Expand Down Expand Up @@ -960,13 +959,7 @@ async def test_get_response_with_rate_limit_exceeded():
with patch.object(
source.github_client._get_client,
"getitem",
side_effect=ClientResponseError(
status=403,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
history=None,
),
side_effect=gidgethub.HTTPException(status_code=HTTPStatus.FORBIDDEN),
):
with pytest.raises(Exception):
source.github_client._get_retry_after = AsyncMock(return_value=0)
Expand Down Expand Up @@ -2094,37 +2087,20 @@ async def test_get_personal_access_token_scopes(scopes, expected_scopes):
"exception, raises",
[
(
ClientResponseError(
status=401,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
history=None,
headers={"X-RateLimit-Remaining": 5000},
),
gidgethub.HTTPException(status_code=HTTPStatus.UNAUTHORIZED),
UnauthorizedException,
),
(
ClientResponseError(
status=403,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
history=None,
headers={"X-RateLimit-Remaining": 2500},
gidgethub.HTTPException(
status_code=HTTPStatus.FORBIDDEN,
),
ForbiddenException,
),
(
ClientResponseError(
status=404,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
history=None,
headers={"X-RateLimit-Remaining": 2500},
gidgethub.HTTPException(
status_code=HTTPStatus.NOT_FOUND,
),
ClientResponseError,
gidgethub.HTTPException,
),
],
)
Expand Down Expand Up @@ -2310,37 +2286,20 @@ async def test_update_installation_access_token_when_error_occurs():
"exceptions, raises",
[
(
ClientResponseError(
status=403,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
headers={"X-RateLimit-Remaining": 4000},
history=None,
gidgethub.HTTPException(
status_code=HTTPStatus.FORBIDDEN,
),
ForbiddenException,
),
(
ClientResponseError(
status=401,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
headers={"X-RateLimit-Remaining": 4000},
history=None,
),
gidgethub.HTTPException(status_code=HTTPStatus.UNAUTHORIZED),
UnauthorizedException,
),
(
ClientResponseError(
status=404,
request_info=aiohttp.RequestInfo(
real_url="", method=None, headers=None, url=""
),
headers={"X-RateLimit-Remaining": 4000},
history=None,
gidgethub.HTTPException(
status_code=HTTPStatus.NOT_FOUND,
),
ClientResponseError,
gidgethub.HTTPException,
),
(Exception(), Exception),
],
Expand Down