diff --git a/src/cryptoadvance/specter/services/extension_gen.py b/src/cryptoadvance/specter/services/extension_gen.py index a0304d49d..e47a1a6aa 100644 --- a/src/cryptoadvance/specter/services/extension_gen.py +++ b/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: diff --git a/src/cryptoadvance/specterext/swan/client.py b/src/cryptoadvance/specterext/swan/client.py index 70b44bbfd..c9695fbcc 100644 --- a/src/cryptoadvance/specterext/swan/client.py +++ b/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: """ diff --git a/tests/test_specterext_swan_client.py b/tests/test_specterext_swan_client.py index c4475773e..6b9a41fbb 100644 --- a/tests/test_specterext_swan_client.py +++ b/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")