Fix: Improve artifact fetching and error handling in build status script

- Implemented exponential backoff and increased retries for API requests in `firebase_github.py`.
- Added specific handling for 410 errors when downloading artifacts, preventing retries for expired artifacts.
- Updated `report_build_status.py` to prioritize newer artifacts and added timeouts for downloads.
- Ensured the script falls back to reading GitHub logs if artifacts cannot be downloaded.
- Addressed missing dependencies: `python-dateutil`, `progress`, and `attrs`.
- Corrected the `--verbosity` flag usage.

Author: google-labs-jules[bot]

scripts/gha/firebase_github.py

@@ -57,13 +57,15 @@ def set_repo_url(repo):
 
 def requests_retry_session(retries=RETRIES,
                            backoff_factor=BACKOFF,
-                           status_forcelist=RETRY_STATUS):
+                           status_forcelist=RETRY_STATUS,
+                           allowed_methods=frozenset(['GET', 'POST', 'PUT', 'DELETE', 'PATCH'])):  # Added allowed_methods
     session = requests.Session()
     retry = Retry(total=retries,
                   read=retries,
                   connect=retries,
                   backoff_factor=backoff_factor,
-                  status_forcelist=status_forcelist)
+                  status_forcelist=status_forcelist,
+                  allowed_methods=allowed_methods)  # Added allowed_methods
     adapter = HTTPAdapter(max_retries=retry)
     session.mount('http://', adapter)
     session.mount('https://', adapter)
@@ -183,14 +185,36 @@ def download_artifact(token, artifact_id, output_path=None):
   """https://docs.github.com/en/rest/reference/actions#download-an-artifact"""
   url = f'{GITHUB_API_URL}/actions/artifacts/{artifact_id}/zip'
   headers = {'Accept': 'application/vnd.github.v3+json', 'Authorization': f'token {token}'}
-  with requests_retry_session().get(url, headers=headers, stream=True, timeout=TIMEOUT_LONG) as response:
-    logging.info("download_artifact: %s response: %s", url, response)
-    if output_path:
-      with open(output_path, 'wb') as file:
-          shutil.copyfileobj(response.raw, file)
-    elif response.status_code == 200:
-      return response.content
-  return None
+  # Custom retry for artifact download due to potential for 410 errors (artifact expired)
+  # which shouldn't be retried indefinitely like other server errors.
+  artifact_retry = Retry(total=5,  # Increased retries
+                         read=5,
+                         connect=5,
+                         backoff_factor=1,  # More aggressive backoff for artifacts
+                         status_forcelist=(500, 502, 503, 504), # Only retry on these server errors
+                         allowed_methods=frozenset(['GET']))
+  session = requests.Session()
+  adapter = HTTPAdapter(max_retries=artifact_retry)
+  session.mount('https://', adapter)
+
+  try:
+    with session.get(url, headers=headers, stream=True, timeout=TIMEOUT_LONG) as response:
+      logging.info("download_artifact: %s response: %s", url, response)
+      response.raise_for_status()  # Raise an exception for bad status codes
+      if output_path:
+        with open(output_path, 'wb') as file:
+            shutil.copyfileobj(response.raw, file)
+        return True # Indicate success
+      else:
+        return response.content
+  except requests.exceptions.HTTPError as e:
+    logging.error(f"HTTP error downloading artifact {artifact_id}: {e.response.status_code} - {e.response.reason}")
+    if e.response.status_code == 410:
+        logging.warning(f"Artifact {artifact_id} has expired and cannot be downloaded.")
+    return None # Indicate failure
+  except requests.exceptions.RequestException as e:
+    logging.error(f"Error downloading artifact {artifact_id}: {e}")
+    return None # Indicate failure
 
 
 def dismiss_review(token, pull_number, review_id, message):

scripts/gha/report_build_status.py

@@ -471,24 +471,63 @@ def main(argv):
           found_artifacts = False
           # There are possibly multiple artifacts, so iterate through all of them,
           # and extract the relevant ones into a temp folder, and then summarize them all.
+          # Prioritize artifacts by date, older ones might be expired.
+          sorted_artifacts = sorted(artifacts, key=lambda art: dateutil.parser.parse(art['created_at']), reverse=True)
+
           with tempfile.TemporaryDirectory() as tmpdir:
-            for a in artifacts:
+            for a in sorted_artifacts: # Iterate over sorted artifacts
               if 'log-artifact' in a['name']:
-                print("Checking this artifact:", a['name'], "\n")
-                artifact_contents = firebase_github.download_artifact(FLAGS.token, a['id'])
-                if artifact_contents:
-                  found_artifacts = True
-                  artifact_data = io.BytesIO(artifact_contents)
-                  artifact_zip = zipfile.ZipFile(artifact_data)
-                  artifact_zip.extractall(path=tmpdir)
+                logging.debug("Attempting to download artifact: %s (ID: %s, Created: %s)", a['name'], a['id'], a['created_at'])
+                # Pass tmpdir to download_artifact to save directly
+                artifact_downloaded_path = os.path.join(tmpdir, f"{a['id']}.zip")
+                # Attempt to download the artifact with a timeout
+                download_success = False # Initialize download_success
+                try:
+                    # download_artifact now returns True on success, None on failure.
+                    if firebase_github.download_artifact(FLAGS.token, a['id'], output_path=artifact_downloaded_path):
+                        download_success = True
+                except requests.exceptions.Timeout:
+                    logging.warning(f"Timeout while trying to download artifact: {a['name']} (ID: {a['id']})")
+                    # download_success remains False
+
+                if download_success and os.path.exists(artifact_downloaded_path):
+                  try:
+                    with open(artifact_downloaded_path, "rb") as f:
+                        artifact_contents = f.read()
+                    if artifact_contents: # Ensure content was read
+                        found_artifacts = True
+                        artifact_data = io.BytesIO(artifact_contents)
+                        with zipfile.ZipFile(artifact_data) as artifact_zip: # Use with statement for ZipFile
+                            artifact_zip.extractall(path=tmpdir)
+                        logging.info("Successfully downloaded and extracted artifact: %s", a['name'])
+                    else:
+                        logging.warning("Artifact %s (ID: %s) was downloaded but is empty.", a['name'], a['id'])
+                  except zipfile.BadZipFile:
+                    logging.error("Failed to open zip file for artifact %s (ID: %s). It might be corrupted or not a zip file.", a['name'], a['id'])
+                  except Exception as e:
+                    logging.error("An error occurred during artifact processing %s (ID: %s): %s", a['name'], a['id'], e)
+                  finally:
+                    # Clean up the downloaded zip file whether it was processed successfully or not
+                    if os.path.exists(artifact_downloaded_path):
+                        os.remove(artifact_downloaded_path)
+                elif not download_success : # Covers False or None from download_artifact
+                  # Logging for non-timeout failures is now primarily handled within download_artifact
+                  # We only log a general failure here if it wasn't a timeout (already logged)
+                  # and download_artifact indicated failure (returned None).
+                  # This avoids double logging for specific HTTP errors like 410.
+                  pass # Most specific logging is now in firebase_github.py
+
             if found_artifacts:
               (success, results) = summarize_test_results.summarize_logs(tmpdir, False, False, True)
-              print("Results:", success, "    ", results, "\n")
+              logging.info("Summarized logs results - Success: %s, Results (first 100 chars): %.100s", success, results)
               run['log_success'] = success
               run['log_results'] = results
+            else:
+              logging.warning("No artifacts could be successfully downloaded and processed for run %s on day %s.", run['id'], day)
+
 
           if not found_artifacts:
-            # Artifacts expire after some time, so if they are gone, we need
+            # Artifacts expire after some time, or download failed, so if they are gone, we need
             # to read the GitHub logs instead.  This is much slower, so we
             # prefer to read artifacts instead whenever possible.
             logging.info("Reading github logs for run %s instead", run['id'])