Retry transient GitHub API failures when fetching org DSN

src/sentry_config.py

@@ -3,10 +3,12 @@
 import base64
 import logging
 import os
+import time
 from configparser import ConfigParser
 from functools import lru_cache
 
 import requests
+from requests import exceptions as requests_exceptions
 
 LOGGING_LEVEL = os.environ.get("LOGGING_LEVEL", logging.INFO)
 logger = logging.getLogger(__name__)
@@ -15,6 +17,11 @@
 SENTRY_CONFIG_API_URL = (
     "https://api.github.com/repos/{owner}/.sentry/contents/sentry_config.ini"
 )
+GITHUB_API_TIMEOUT_SECONDS = float(os.environ.get("GITHUB_API_TIMEOUT_SECONDS", "10"))
+MAX_GITHUB_API_ATTEMPTS = int(os.environ.get("MAX_GITHUB_API_ATTEMPTS", "3"))
+GITHUB_API_RETRY_BACKOFF_SECONDS = float(
+    os.environ.get("GITHUB_API_RETRY_BACKOFF_SECONDS", "1")
+)
 
 
 def fetch_dsn_for_github_org(org: str, token: str) -> str:
@@ -23,26 +30,47 @@
         "Accept": "application/vnd.github+json",
         "Authorization": f"token {token}",
     }
-    try:
-        api_url = SENTRY_CONFIG_API_URL.replace("{owner}", org)
+    api_url = SENTRY_CONFIG_API_URL.replace("{owner}", org)
 
-        # - Get meta about sentry_config.ini file
-        resp = requests.get(api_url, headers=headers)
-        resp.raise_for_status()
-        meta = resp.json()
+    for attempt in range(1, MAX_GITHUB_API_ATTEMPTS + 1):
+        try:
+            # - Get meta about sentry_config.ini file
+            resp = requests.get(
+                api_url,
+                headers=headers,
+                timeout=GITHUB_API_TIMEOUT_SECONDS,
+            )
+            resp.raise_for_status()
+            meta = resp.json()
 
-        if meta["type"] != "file":
-            # XXX: custom error
-            raise Exception(meta["type"])
+            if meta["type"] != "file":
+                # XXX: custom error
+                raise Exception(meta["type"])
 
-        assert meta["encoding"] == "base64", meta["encoding"]
-        file_contents = base64.b64decode(meta["content"]).decode()
+            assert meta["encoding"] == "base64", meta["encoding"]
+            file_contents = base64.b64decode(meta["content"]).decode()
 
-        # - Read ini file and assertions
-        cp = ConfigParser()
-        cp.read_string(file_contents)
-        return cp.get("sentry-github-actions-app", "dsn")
+            # - Read ini file and assertions
+            cp = ConfigParser()
+            cp.read_string(file_contents)
+            return cp.get("sentry-github-actions-app", "dsn")
+        except (requests_exceptions.ConnectionError, requests_exceptions.Timeout) as e:
+            if attempt == MAX_GITHUB_API_ATTEMPTS:
+                logger.exception(
+                    "Failed to fetch sentry config from GitHub for org '%s' after %s attempts",
+                    org,
+                    MAX_GITHUB_API_ATTEMPTS,
+                )
+                raise e
 
-    except Exception as e:
-        logger.exception(e)
-        raise e
+            logger.warning(
+                "Transient GitHub API failure while fetching sentry config for org '%s' (attempt %s/%s): %s",
+                org,
+                attempt,
+                MAX_GITHUB_API_ATTEMPTS,
+                type(e).__name__,
+            )
+            time.sleep(GITHUB_API_RETRY_BACKOFF_SECONDS * attempt)
+        except Exception as e:
+            logger.exception(e)
+            raise e

tests/test_sentry_config_file.py

@@ -1,9 +1,14 @@
 from __future__ import annotations
 
 from unittest import TestCase
+from unittest.mock import Mock
+from unittest.mock import patch
 
+import requests
 import responses
 
+from src.sentry_config import GITHUB_API_TIMEOUT_SECONDS
+from src.sentry_config import MAX_GITHUB_API_ATTEMPTS
 from src.sentry_config import fetch_dsn_for_github_org
 from src.sentry_config import SENTRY_CONFIG_API_URL as api_url
 
@@ -47,11 +52,48 @@ def setUp(self) -> None:
     def test_fetch_parse_sentry_config_file(self) -> None:
         assert fetch_dsn_for_github_org(org, token) == expected_dsn
 
-    def test_fetch_private_repo(self) -> None:
-        pass
+    @patch("src.sentry_config.requests.get")
+    def test_fetch_parse_sentry_config_file_sets_timeout(self, mock_get) -> None:
+        response = Mock()
+        response.raise_for_status.return_value = None
+        response.json.return_value = sentry_config_file_meta
+        mock_get.return_value = response
 
-    def test_file_missing(self) -> None:
-        pass
+        assert fetch_dsn_for_github_org(org, token) == expected_dsn
+
+        mock_get.assert_called_once_with(
+            self.api_url,
+            headers={
+                "Accept": "application/vnd.github+json",
+                "Authorization": f"token {token}",
+            },
+            timeout=GITHUB_API_TIMEOUT_SECONDS,
+        )
+
+    @patch("src.sentry_config.time.sleep")
+    @patch("src.sentry_config.requests.get")
+    def test_fetch_private_repo_retries_transient_connection_error(
+        self, mock_get, mock_sleep
+    ) -> None:
+        response = Mock()
+        response.raise_for_status.return_value = None
+        response.json.return_value = sentry_config_file_meta
+        mock_get.side_effect = [requests.exceptions.ConnectionError("reset"), response]
+
+        assert fetch_dsn_for_github_org(org, token) == expected_dsn
+        assert mock_get.call_count == 2
+        mock_sleep.assert_called_once_with(1.0)
+
+    @patch("src.sentry_config.time.sleep")
+    @patch("src.sentry_config.requests.get")
+    def test_file_missing_raises_after_max_attempts(self, mock_get, mock_sleep) -> None:
+        mock_get.side_effect = requests.exceptions.ConnectionError("reset")
+
+        with self.assertRaises(requests.exceptions.ConnectionError):
+            fetch_dsn_for_github_org(org, token)
+
+        assert mock_get.call_count == MAX_GITHUB_API_ATTEMPTS
+        assert mock_sleep.call_count == MAX_GITHUB_API_ATTEMPTS - 1
 
     def test_bad_contents(self) -> None:
         pass