From 90284c1066da886abed140c7ee6f4a16283d51cd Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Fri, 1 Nov 2024 09:22:16 -0500 Subject: [PATCH] fix: handle bad gh auth --- src/ape/utils/_github.py | 29 ++++++++++++- tests/functional/utils/test_github.py | 59 ++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/src/ape/utils/_github.py b/src/ape/utils/_github.py index 0c657c11ea..0b7c413b3a 100644 --- a/src/ape/utils/_github.py +++ b/src/ape/utils/_github.py @@ -8,7 +8,7 @@ from pathlib import Path from typing import Any, Optional, Union -from requests import Session +from requests import HTTPError, Session from requests.adapters import HTTPAdapter from urllib3.util.retry import Retry @@ -224,7 +224,32 @@ def _get(self, url: str, params: Optional[dict] = None) -> Any: def _request(self, method: str, url: str, **kwargs) -> Any: url = f"{self.API_URL_PREFIX}/{url}" response = self.__session.request(method, url, **kwargs) - return response.json() + + try: + response.raise_for_status() + except HTTPError as err: + if err.response.status_code == 401 and self.__session.headers.get("Authorization"): + del self.__session.headers["Authorization"] + response = self.__session.request(method, url, **kwargs) + try: + response.raise_for_status() # Raise exception if the retry also fails + except HTTPError: + # Even without the Authorization token, the request still failed. + # Raise the original error in this case. + raise err + else: + # The request failed with Authorization but succeeded without. + # Let the user know their token is likely expired. + logger.warning( + "Requests are not authorized! GITHUB_ACCESS_TOKEN is likely expired; " + "received 401 when attempted to use it. If you need GitHub authorization, " + "try resetting your token." + ) + return response.json() + + else: + # Successful response status code! + return response.json() github_client = _GithubClient() diff --git a/tests/functional/utils/test_github.py b/tests/functional/utils/test_github.py index 4418a5f72a..206a2cf5f2 100644 --- a/tests/functional/utils/test_github.py +++ b/tests/functional/utils/test_github.py @@ -1,7 +1,7 @@ from pathlib import Path import pytest -from requests.exceptions import ConnectTimeout +from requests.exceptions import ConnectTimeout, HTTPError from ape.utils._github import _GithubClient from ape.utils.os import create_tempdir @@ -98,3 +98,60 @@ def test_get_org_repos(self, github_client, mock_session): params = call.kwargs["params"] # Show we are fetching more than the default 30 per page. assert params == {"per_page": 100, "page": 1} + + def test_available_plugins(self, mocker, github_client, mock_session): + response1 = mocker.MagicMock() + response1.json.return_value = [{"name": "ape-myplugin"}] + response2 = mocker.MagicMock() + response2.json.return_value = [] + + def get_org_repos(method, url, **kwargs): + if kwargs["params"]["page"] == 1: + return response1 + else: + # End. + return response2 + + mock_session.request.side_effect = get_org_repos + actual = github_client.available_plugins + assert actual == {"ape_myplugin"} + + def test_available_plugins_handles_401(self, mocker, github_client, mock_session, ape_caplog): + """ + When you get a 401 from using a token, Ape's GitHub client should not + only warn the user but retry the request w/o authorization, as it likely + will still work. + """ + mock_session.headers = {"Authorization": "token mytoken"} + + response1 = mocker.MagicMock() + response1.json.return_value = [{"name": "ape-myplugin"}] + response2 = mocker.MagicMock() + response2.json.return_value = [] + + bad_auth_response = mocker.MagicMock() + bad_auth_response.status_code = 401 + bad_auth_response.raise_for_status.side_effect = HTTPError(response=bad_auth_response) + + def get_org_repos(method, url, **kwargs): + if mock_session.headers.get("Authorization") == "token mytoken": + return bad_auth_response + elif kwargs["params"]["page"] == 1: + return response1 + else: + # End. + return response2 + + mock_session.request.side_effect = get_org_repos + actual = github_client.available_plugins + + # Still works, even with bad auth. + assert actual == {"ape_myplugin"} + + # Show we got our log message. + expected = ( + "Requests are not authorized! GITHUB_ACCESS_TOKEN is likely " + "expired; received 401 when attempted to use it. If you need " + "GitHub authorization, try resetting your token." + ) + assert ape_caplog.head == expected