Fix github API handling and keyman behavior to correct handling of invalid or missing keys

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):
 

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:

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():