Handle GitHub app token revocation timeouts

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)

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