diff --git a/collectoss/tasks/github/util/github_data_access.py b/collectoss/tasks/github/util/github_data_access.py index 6a603c3e3..9eaf634fc 100644 --- a/collectoss/tasks/github/util/github_data_access.py +++ b/collectoss/tasks/github/util/github_data_access.py @@ -169,7 +169,15 @@ def make_request(self, url, method="GET", timeout=100): raise UrlNotFoundException(f"Could not find {url}") if response.status_code == 401: - raise NotAuthorizedException(f"Could not authorize with the github api using key: {mask_key(self.key)}") + resp_content = response.json() + response_msg = resp_content.get("message") + response_status = int(resp_content.get("status")) + + if response_status == 401 and response_msg == "Bad credentials": + self.logger.warning(f"Received a 401 response from github due to bad credential: {mask_key(self.key)}") + raise NotAuthorizedException(f"Could not authorize with the github api because key was invalid: {mask_key(self.key)}") + else: + self.logger.warning(f"Received a 401 response from github unrelated to bad credentials with message {response_msg}") if response.status_code == 410: response_msg = response.json().get("message") @@ -198,7 +206,14 @@ def make_request_with_retries(self, url, method="GET", timeout=100): try: return self.__make_request_with_retries(url, method, timeout) except RetryError as e: - raise e.last_attempt.exception() + last_exception = e.last_attempt.exception() + + # https://github.com/orgs/community/discussions/101661#discussioncomment-8342211 + # this suggests we should retry 401 exceptions at least once + if isinstance(last_exception, NotAuthorizedException): + self.expired_keys_for_request = [] + self.__handle_github_not_authorized_response() + raise last_exception def _decide_retry_policy(exception: Exception) -> bool: """Defines whether or not to retry a failed request based on the exception thrown @@ -224,10 +239,6 @@ def __make_request_with_retries(self, url, method="GET", timeout=100): except RatelimitException as e: self.__handle_github_ratelimit_response(e.response) raise e - except NotAuthorizedException as e: - self.expired_keys_for_request = [] - self.__handle_github_not_authorized_response() - raise e def __handle_github_not_authorized_response(self): diff --git a/keyman/KeyClient.py b/keyman/KeyClient.py index b950229fe..ad1a14fdc 100644 --- a/keyman/KeyClient.py +++ b/keyman/KeyClient.py @@ -104,7 +104,7 @@ def request(self, platform: str | None = None) -> str: self._send("NEW", key_platform = platform or self.platform) try: msg = self._recv() - if "key" in msg: + if msg.get("key") is not None: return msg["key"] raise Exception(f"Invalid response type: {msg}") except WaitKeyTimeout as e: diff --git a/keyman/Orchestrator.py b/keyman/Orchestrator.py index 71cfae8bb..54ca064cd 100644 --- a/keyman/Orchestrator.py +++ b/keyman/Orchestrator.py @@ -156,7 +156,7 @@ def new_key(self, platform: str) -> str | None: if not len(self.fresh_keys[platform]): if not len(self.expired_keys[platform]): self.logger.warning(f"Key was requested for {platform}, but none are published") - return + raise WaitKeyTimeout(300) min_timeout = 0 for _, timeout in self.expired_keys[platform].items():