From 8302b0c48cb8f08b9ac8da18ad5f0297e2ffec65 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 8 May 2026 17:17:59 +0000 Subject: [PATCH] Handle transient GitHub request failures Co-authored-by: Armen Zambrano G. --- src/github_app.py | 23 +++++++++++++---- src/github_requests.py | 46 ++++++++++++++++++++++++++++++++++ src/github_sdk.py | 4 ++- src/sentry_config.py | 4 +-- tests/test_github_app.py | 54 ++++++++++++++++++++++++++++++++++++++++ tests/test_github_sdk.py | 14 +++++++++++ 6 files changed, 137 insertions(+), 8 deletions(-) create mode 100644 src/github_requests.py create mode 100644 tests/test_github_app.py diff --git a/src/github_app.py b/src/github_app.py index c18b6a9..2b4dca8 100644 --- a/src/github_app.py +++ b/src/github_app.py @@ -4,12 +4,17 @@ from __future__ import annotations import contextlib +import logging import time from typing import Generator import jwt import requests +from .github_requests import github_request + +logger = logging.getLogger(__name__) + class GithubAppToken: def __init__(self, private_key, app_id) -> None: @@ -19,7 +24,8 @@ def __init__(self, private_key, app_id) -> None: # configured by the GitHub App and expire after one hour. @contextlib.contextmanager def get_token(self, installation_id: int) -> Generator[str, None, None]: - req = requests.post( + req = github_request( + "POST", url=f"https://api.github.com/app/installations/{installation_id}/access_tokens", headers=self.headers, ) @@ -29,10 +35,17 @@ def get_token(self, installation_id: int) -> Generator[str, None, None]: # This token expires in an hour yield resp["token"] finally: - requests.delete( - "https://api.github.com/installation/token", - headers={"Authorization": f"token {resp['token']}"}, - ) + try: + github_request( + "DELETE", + "https://api.github.com/installation/token", + headers={"Authorization": f"token {resp['token']}"}, + ) + except requests.RequestException: + logger.warning( + "Failed to revoke GitHub App installation token.", + exc_info=True, + ) def get_jwt_token(self, private_key, app_id): payload = { diff --git a/src/github_requests.py b/src/github_requests.py new file mode 100644 index 0000000..c08f3dc --- /dev/null +++ b/src/github_requests.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +import logging +import time +from typing import Any + +import requests + +logger = logging.getLogger(__name__) + +DEFAULT_TIMEOUT_SECONDS = 10 +DEFAULT_RETRIES = 2 +DEFAULT_RETRY_DELAY_SECONDS = 0.5 +RETRYABLE_EXCEPTIONS = ( + requests.exceptions.ConnectionError, + requests.exceptions.Timeout, +) + + +def github_request( + method: str, + url: str, + *, + timeout: int = DEFAULT_TIMEOUT_SECONDS, + retries: int = DEFAULT_RETRIES, + retry_delay: float = DEFAULT_RETRY_DELAY_SECONDS, + **kwargs: Any, +) -> requests.Response: + """Make a GitHub API request resilient to transient network failures.""" + attempts = retries + 1 + for attempt in range(1, attempts + 1): + try: + return requests.request(method, url, timeout=timeout, **kwargs) + except RETRYABLE_EXCEPTIONS: + if attempt == attempts: + raise + logger.warning( + "GitHub API %s request failed; retrying (%s/%s).", + method.upper(), + attempt, + attempts, + exc_info=True, + ) + time.sleep(retry_delay * attempt) + + raise RuntimeError("unreachable") diff --git a/src/github_sdk.py b/src/github_sdk.py index cf28d01..53df470 100644 --- a/src/github_sdk.py +++ b/src/github_sdk.py @@ -11,6 +11,8 @@ from sentry_sdk.envelope import Envelope from sentry_sdk.utils import format_timestamp +from .github_requests import github_request + class GithubSentryError(Exception): pass @@ -42,7 +44,7 @@ def __init__(self, token, dsn, dry_run=False) -> None: def _fetch_github(self, url): headers = {"Authorization": f"token {self.token}"} - req = requests.get(url, headers=headers) + req = github_request("GET", url, headers=headers) req.raise_for_status() return req diff --git a/src/sentry_config.py b/src/sentry_config.py index 30678da..60063ad 100644 --- a/src/sentry_config.py +++ b/src/sentry_config.py @@ -6,7 +6,7 @@ from configparser import ConfigParser from functools import lru_cache -import requests +from .github_requests import github_request LOGGING_LEVEL = os.environ.get("LOGGING_LEVEL", logging.INFO) logger = logging.getLogger(__name__) @@ -27,7 +27,7 @@ def fetch_dsn_for_github_org(org: str, token: str) -> str: api_url = SENTRY_CONFIG_API_URL.replace("{owner}", org) # - Get meta about sentry_config.ini file - resp = requests.get(api_url, headers=headers) + resp = github_request("GET", api_url, headers=headers) resp.raise_for_status() meta = resp.json() diff --git a/tests/test_github_app.py b/tests/test_github_app.py new file mode 100644 index 0000000..3dcee7b --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from unittest.mock import patch + +import pytest +import requests +import responses + +from src.github_app import GithubAppToken + + +CREATE_TOKEN_URL = ( + "https://api.github.com/app/installations/123/access_tokens" +) +REVOKE_TOKEN_URL = "https://api.github.com/installation/token" + + +def github_app_token(): + app_token = object.__new__(GithubAppToken) + app_token.headers = {} + return app_token + + +def register_token_revocation_failure(): + for _ in range(3): + responses.delete( + REVOKE_TOKEN_URL, + body=requests.exceptions.ConnectionError("connection reset"), + ) + + +@responses.activate +@patch("src.github_requests.time.sleep") +def test_token_revocation_connection_error_is_ignored(mock_sleep): + responses.post(CREATE_TOKEN_URL, json={"token": "installation-token"}, status=201) + register_token_revocation_failure() + + with github_app_token().get_token(123) as token: + assert token == "installation-token" + + assert len(responses.calls) == 4 + + +@responses.activate +@patch("src.github_requests.time.sleep") +def test_token_revocation_connection_error_does_not_mask_work_error(mock_sleep): + responses.post(CREATE_TOKEN_URL, json={"token": "installation-token"}, status=201) + register_token_revocation_failure() + + with pytest.raises(requests.exceptions.ConnectionError, match="work failed"): + with github_app_token().get_token(123): + raise requests.exceptions.ConnectionError("work failed") + + assert len(responses.calls) == 4 diff --git a/tests/test_github_sdk.py b/tests/test_github_sdk.py index 7f7e401..afb5d75 100644 --- a/tests/test_github_sdk.py +++ b/tests/test_github_sdk.py @@ -59,6 +59,20 @@ def test_ensure_raise_error_on_github_api_failure(): ) +@responses.activate +@patch("src.github_requests.time.sleep") +def test_retries_github_api_connection_errors(mock_sleep): + url = "https://api.github.com/repos/getsentry/sentry/actions/runs/2104746951" + responses.get(url, body=requests.exceptions.ConnectionError("connection reset")) + responses.get(url, json={"ok": True}) + + client = GithubClient(dsn=DSN, token=TOKEN) + resp = client._fetch_github(url) + + assert resp.json() == {"ok": True} + assert len(responses.calls) == 2 + + @freeze_time() @responses.activate @patch("src.github_sdk.get_uuid")