diff --git a/src/github_app.py b/src/github_app.py index c18b6a9..56c35f7 100644 --- a/src/github_app.py +++ b/src/github_app.py @@ -4,12 +4,23 @@ from __future__ import annotations import contextlib +import logging import time from typing import Generator import jwt import requests +logger = logging.getLogger(__name__) + +GITHUB_API_TIMEOUT_SECONDS = 5 +GITHUB_API_MAX_ATTEMPTS = 3 +GITHUB_API_RETRYABLE_ERRORS = ( + requests.ConnectionError, + requests.Timeout, + requests.exceptions.SSLError, +) + class GithubAppToken: def __init__(self, private_key, app_id) -> None: @@ -19,8 +30,9 @@ 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( - url=f"https://api.github.com/app/installations/{installation_id}/access_tokens", + req = _request_github_with_retries( + requests.post, + f"https://api.github.com/app/installations/{installation_id}/access_tokens", headers=self.headers, ) req.raise_for_status() @@ -29,10 +41,14 @@ 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: + _request_github_with_retries( + requests.delete, + "https://api.github.com/installation/token", + headers={"Authorization": f"token {resp['token']}"}, + ) + except GITHUB_API_RETRYABLE_ERRORS as e: + logger.warning("Failed to revoke GitHub installation token: %s", e) def get_jwt_token(self, private_key, app_id): payload = { @@ -51,3 +67,14 @@ def get_authentication_header(self, private_key, app_id): "Accept": "application/vnd.github.v3+json", "Authorization": f"Bearer {jwt_token}", } + + +def _request_github_with_retries(method, url, **kwargs): + kwargs.setdefault("timeout", GITHUB_API_TIMEOUT_SECONDS) + for attempt in range(GITHUB_API_MAX_ATTEMPTS): + try: + return method(url, **kwargs) + except GITHUB_API_RETRYABLE_ERRORS: + if attempt == GITHUB_API_MAX_ATTEMPTS - 1: + raise + time.sleep(2**attempt) diff --git a/tests/test_github_app.py b/tests/test_github_app.py new file mode 100644 index 0000000..6d6a7f1 --- /dev/null +++ b/tests/test_github_app.py @@ -0,0 +1,95 @@ +from __future__ import annotations + +import logging +from unittest.mock import Mock, patch + +import pytest +import requests + +from src.github_app import ( + GITHUB_API_MAX_ATTEMPTS, + GITHUB_API_TIMEOUT_SECONDS, + GithubAppToken, +) + + +class FakeResponse: + def __init__(self, body): + self.body = body + + def json(self): + return self.body + + def raise_for_status(self): + return None + + +def make_github_app_token(): + github_app_token = GithubAppToken.__new__(GithubAppToken) + github_app_token.headers = {"Authorization": "Bearer app-token"} + return github_app_token + + +@patch("src.github_app.time.sleep") +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_retries_installation_token_creation( + mock_post, + mock_delete, + mock_sleep, +): + mock_post.side_effect = [ + requests.ConnectTimeout("connection timed out"), + FakeResponse({"token": "installation-token"}), + ] + mock_delete.return_value = FakeResponse({}) + + with make_github_app_token().get_token(123) as token: + assert token == "installation-token" + + assert mock_post.call_count == 2 + for call in mock_post.call_args_list: + assert call.kwargs["timeout"] == GITHUB_API_TIMEOUT_SECONDS + mock_sleep.assert_called_once_with(1) + + +@patch("src.github_app.time.sleep") +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_logs_and_swallows_revocation_timeout( + mock_post, + mock_delete, + mock_sleep, + caplog, +): + mock_post.return_value = FakeResponse({"token": "installation-token"}) + mock_delete.side_effect = requests.ConnectTimeout("connection timed out") + + with caplog.at_level(logging.WARNING): + with make_github_app_token().get_token(123) as token: + assert token == "installation-token" + + assert mock_delete.call_count == GITHUB_API_MAX_ATTEMPTS + for call in mock_delete.call_args_list: + assert call.kwargs["timeout"] == GITHUB_API_TIMEOUT_SECONDS + assert mock_sleep.call_count == GITHUB_API_MAX_ATTEMPTS - 1 + assert "Failed to revoke GitHub installation token" in caplog.text + + +@patch("src.github_app.time.sleep") +@patch("src.github_app.requests.delete") +@patch("src.github_app.requests.post") +def test_get_token_preserves_context_errors_when_revocation_times_out( + mock_post, + mock_delete, + mock_sleep, +): + mock_post.return_value = FakeResponse({"token": "installation-token"}) + mock_delete.side_effect = requests.ConnectTimeout("connection timed out") + + with pytest.raises(RuntimeError, match="processing failed"): + with make_github_app_token().get_token(123): + raise RuntimeError("processing failed") + + assert mock_delete.call_count == GITHUB_API_MAX_ATTEMPTS + assert mock_sleep.call_count == GITHUB_API_MAX_ATTEMPTS - 1