Handle GitHub token cleanup connection errors without masking root cause

src/github_app.py

@@ -4,12 +4,15 @@
 from __future__ import annotations
 
 import contextlib
+import logging
 import time
 from typing import Generator
 
 import jwt
 import requests
 
+logger = logging.getLogger(__name__)
+
 
 class GithubAppToken:
     def __init__(self, private_key, app_id) -> None:
@@ -29,10 +32,19 @@ 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']}"},
-            )
+            # Token revocation is best-effort; cleanup failures should not hide
+            # the original exception from webhook processing.
+            try:
+                requests.delete(
+                    "https://api.github.com/installation/token",
+                    headers={"Authorization": f"token {resp['token']}"},
+                    timeout=5,
+                )
+            except requests.RequestException:
+                logger.warning(
+                    "Failed to revoke GitHub installation token during cleanup.",
+                    exc_info=True,
+                )
 
     def get_jwt_token(self, private_key, app_id):
         payload = {

tests/test_github_app.py

@@ -0,0 +1,71 @@
+from __future__ import annotations
+
+from unittest.mock import Mock
+from unittest.mock import patch
+
+import pytest
+import requests
+
+from src.github_app import GithubAppToken
+
+
+def _new_client():
+    client = GithubAppToken.__new__(GithubAppToken)
+    client.headers = {"Authorization": "Bearer jwt-token"}
+    return client
+
+
+def test_get_token_revokes_installation_token():
+    client = _new_client()
+    token_response = Mock()
+    token_response.raise_for_status.return_value = None
+    token_response.json.return_value = {"token": "installation-token"}
+
+    with patch("src.github_app.requests.post", return_value=token_response) as post_mock:
+        with patch("src.github_app.requests.delete") as delete_mock:
+            with client.get_token(12345) as token:
+                assert token == "installation-token"
+
+    post_mock.assert_called_once_with(
+        url="https://api.github.com/app/installations/12345/access_tokens",
+        headers=client.headers,
+    )
+    delete_mock.assert_called_once_with(
+        "https://api.github.com/installation/token",
+        headers={"Authorization": "token installation-token"},
+        timeout=5,
+    )
+
+
+def test_get_token_cleanup_network_error_is_non_fatal():
+    client = _new_client()
+    token_response = Mock()
+    token_response.raise_for_status.return_value = None
+    token_response.json.return_value = {"token": "installation-token"}
+
+    with patch("src.github_app.requests.post", return_value=token_response):
+        with patch(
+            "src.github_app.requests.delete",
+            side_effect=requests.ConnectionError("cleanup failed"),
+        ):
+            with patch("src.github_app.logger.warning") as warning_mock:
+                with client.get_token(12345) as token:
+                    assert token == "installation-token"
+
+    warning_mock.assert_called_once()
+
+
+def test_get_token_cleanup_error_does_not_mask_inner_exception():
+    client = _new_client()
+    token_response = Mock()
+    token_response.raise_for_status.return_value = None
+    token_response.json.return_value = {"token": "installation-token"}
+
+    with patch("src.github_app.requests.post", return_value=token_response):
+        with patch(
+            "src.github_app.requests.delete",
+            side_effect=requests.ConnectionError("cleanup failed"),
+        ):
+            with pytest.raises(ValueError, match="primary failure"):
+                with client.get_token(12345):
+                    raise ValueError("primary failure")