fix(swan): add timeout to remote HTTP requests to prevent hang (#2687)

Co-authored-by: k9ert <117085+k9ert@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

src/cryptoadvance/specter/services/extension_gen.py

@@ -152,7 +152,7 @@ class ExtGen:
             shutil.copy(sourcepath, targetpath)
             print(f"    --> Created {targetpath} (copied)")
         else:
-            r = requests.get(self.env.loader.url_for_template(sourcepath))
+            r = requests.get(self.env.loader.url_for_template(sourcepath), timeout=30)
             open(targetpath, "wb").write(r.content)
             print(f"    --> Created {targetpath} (via Github)")
 
@@ -233,7 +233,7 @@ class GithubUrlLoader(BaseLoader):
     def get_source(self, environment, template):
         url = self.url_for_template(template)
         for attempt in range(3):
-            r = requests.get(url)
+            r = requests.get(url, timeout=30)
             if r.status_code == 200:
                 return r.text, url, None
             if r.status_code == 429 and attempt < 2:

src/cryptoadvance/specterext/swan/client.py

@@ -137,12 +137,25 @@ class SwanClient:
             ).decode()
             auth_header["Authorization"] = f"Basic {auth_hash}"
 
-        response = requests.post(
-            f"{self.api_url}/oidc/token",
-            data=payload,
-            headers=auth_header,
-        )
-        resp = json.loads(response.text)
+        try:
+            response = requests.post(
+                f"{self.api_url}/oidc/token",
+                data=payload,
+                headers=auth_header,
+                timeout=30,
+            )
+        except requests.exceptions.RequestException as e:
+            logger.exception(e)
+            raise SwanApiException(
+                f"Could not reach the Swan API ({self.api_url}/oidc/token): {e}"
+            ) from e
+        try:
+            resp = json.loads(response.text)
+        except ValueError as e:
+            logger.error(f"{response.status_code}: {response.text}")
+            raise SwanApiException(
+                f"Swan API returned no valid json ({response.status_code}): {response.text}"
+            ) from e
         """
             {
                 "access_token": "***************",
@@ -184,27 +197,43 @@ class SwanClient:
             "User-Agent": "Specter Desktop",
             "Authorization": f"Bearer {access_token}",
         }
+        request_context = f"endpoint: {self.api_url}{endpoint} | method: {method} | payload: {json.dumps(json_payload, indent=4)}"
+
         try:
             if method == "GET":
-                response = requests.get(self.api_url + endpoint, headers=auth_header)
+                response = requests.get(
+                    self.api_url + endpoint, headers=auth_header, timeout=30
+                )
             elif method in ["POST", "PATCH", "PUT", "DELETE"]:
                 response = requests.request(
                     method=method,
                     url=self.api_url + endpoint,
                     headers=auth_header,
                     json=json_payload,
+                    timeout=30,
                 )
-            if response.status_code != 200:
-                raise SwanApiException(f"{response.status_code}: {response.text}")
-            return response.json()
-        except Exception as e:
-            # TODO: tighten up expected Exceptions
+            else:
+                raise SwanApiException(f"Unsupported method: {method}")
+        except requests.exceptions.RequestException as e:
+            # Timeouts, connection errors, ... : no response to report about
             logger.exception(e)
-            logger.error(
-                f"endpoint: {self.api_url}{endpoint} | method: {method} | payload: {json.dumps(json_payload, indent=4)}"
-            )
+            logger.error(request_context)
+            raise SwanApiException(f"Could not reach the Swan API: {e}") from e
+
+        if response.status_code != 200:
+            logger.error(request_context)
             logger.error(f"{response.status_code}: {response.text}")
-            raise e
+            raise SwanApiException(f"{response.status_code}: {response.text}")
+
+        try:
+            return response.json()
+        except ValueError as e:
+            logger.exception(e)
+            logger.error(request_context)
+            logger.error(f"{response.status_code}: {response.text}")
+            raise SwanApiException(
+                f"Swan API returned no valid json ({response.status_code}): {response.text}"
+            ) from e
 
     def get_autowithdrawal_addresses(self, swan_wallet_id: str) -> dict:
         """

tests/test_specterext_swan_client.py

@@ -6,7 +6,10 @@ from unittest.mock import MagicMock
 import pytest
 import mock
 from mock import Mock, patch
+import requests
+
 from cryptoadvance.specterext.swan.client import (
+    SwanApiException,
     SwanApiRefreshTokenException,
     SwanClient,
 )
@@ -106,6 +109,65 @@ def test_expired_access_token():
         sc._get_access_token()
 
 
+def construct_client_with_valid_token():
+    """A client which won't need to fetch an access_token first"""
+    return SwanClient(
+        "a_hostname", "forever_valid_access_token", 5000000000, "a_refresh_token"
+    )
+
+
+def test_authenticated_request_get_timeout(app_no_node):
+    """A timeout must surface as SwanApiException, not as an UnboundLocalError"""
+    sc = construct_client_with_valid_token()
+    with app_no_node.app_context():
+        with mock.patch(
+            "requests.get", side_effect=requests.exceptions.Timeout("simulated timeout")
+        ):
+            with pytest.raises(SwanApiException) as exc_info:
+                sc.authenticated_request("/some/endpoint")
+        assert "simulated timeout" in str(exc_info.value)
+        assert isinstance(exc_info.value.__cause__, requests.exceptions.Timeout)
+
+
+def test_authenticated_request_post_timeout(app_no_node):
+    """Same for the methods going through requests.request"""
+    sc = construct_client_with_valid_token()
+    with app_no_node.app_context():
+        with mock.patch(
+            "requests.request",
+            side_effect=requests.exceptions.ConnectTimeout("simulated timeout"),
+        ):
+            with pytest.raises(SwanApiException) as exc_info:
+                sc.authenticated_request(
+                    "/some/endpoint", method="POST", json_payload={"muuh": "meeh"}
+                )
+        assert isinstance(exc_info.value.__cause__, requests.exceptions.ConnectTimeout)
+
+
+def test_authenticated_request_error_status_code(app_no_node):
+    sc = construct_client_with_valid_token()
+    fake_response = Mock()
+    fake_response.status_code = 500
+    fake_response.text = "Internal Server Error"
+    with app_no_node.app_context():
+        with mock.patch("requests.get", return_value=fake_response):
+            with pytest.raises(SwanApiException, match="500: Internal Server Error"):
+                sc.authenticated_request("/some/endpoint")
+
+
+def test_get_access_token_timeout(app_no_node):
+    """The token-endpoint is used before authenticated_request can even start"""
+    sc = SwanClient("a_hostname", "an_expired_access_token", 1000, "a_refresh_token")
+    with app_no_node.app_context():
+        with mock.patch(
+            "requests.post",
+            side_effect=requests.exceptions.Timeout("simulated timeout"),
+        ):
+            with pytest.raises(SwanApiException) as exc_info:
+                sc.authenticated_request("/some/endpoint")
+        assert isinstance(exc_info.value.__cause__, requests.exceptions.Timeout)
+
+
 @patch("requests.delete")
 @patch("requests.request")
 @patch("requests.patch")