From 836c0b8328cb18a23063da17da710070deff3e62 Mon Sep 17 00:00:00 2001 From: ddc <34492089+ddc@users.noreply.github.com> Date: Wed, 25 Feb 2026 16:26:52 -0300 Subject: [PATCH] v3.0.6 --- .env.example | 3 + src/gw2/constants/gw2_settings.py | 6 +- src/gw2/tools/gw2_client.py | 41 ++++- tests/unit/gw2/tools/test_gw2_client.py | 202 +++++++++++++++++++++++- 4 files changed, 243 insertions(+), 9 deletions(-) diff --git a/.env.example b/.env.example index a665e84c..e3ebb2f5 100644 --- a/.env.example +++ b/.env.example @@ -85,3 +85,6 @@ GW2_MISC_COOLDOWN=20 GW2_SESSION_COOLDOWN=60 GW2_WORLDS_COOLDOWN=20 GW2_WVW_COOLDOWN=20 +# GW2 API retry +GW2_API_RETRY_MAX_ATTEMPTS=5 +GW2_API_RETRY_DELAY=3.0 diff --git a/src/gw2/constants/gw2_settings.py b/src/gw2/constants/gw2_settings.py index 14c67499..9ad10a5e 100644 --- a/src/gw2/constants/gw2_settings.py +++ b/src/gw2/constants/gw2_settings.py @@ -10,6 +10,8 @@ class Gw2Settings(BaseSettings): """GW2 settings defined here with fallback to reading ENV variables""" + model_config = SettingsConfigDict(env_prefix="GW2_", env_file=".env", extra="allow") + # GW2 configuration api_version: int | None = Field(default=2) embed_color: str | None = Field(default="green") @@ -25,7 +27,9 @@ class Gw2Settings(BaseSettings): worlds_cooldown: int | None = Field(default=20) wvw_cooldown: int | None = Field(default=20) - model_config = SettingsConfigDict(env_prefix="GW2_", env_file=".env", extra="allow") + # GW2 API retry + api_retry_max_attempts: int | None = Field(default=5) + api_retry_delay: float | None = Field(default=3.0) @lru_cache(maxsize=1) diff --git a/src/gw2/tools/gw2_client.py b/src/gw2/tools/gw2_client.py index 52c1c3e9..89490e8e 100644 --- a/src/gw2/tools/gw2_client.py +++ b/src/gw2/tools/gw2_client.py @@ -1,4 +1,6 @@ +import asyncio from src.gw2.constants import gw2_messages, gw2_variables +from src.gw2.constants.gw2_settings import get_gw2_settings from src.gw2.tools.gw2_exceptions import ( APIBadRequest, APIConnectionError, @@ -9,6 +11,9 @@ APINotFound, ) +_gw2_settings = get_gw2_settings() +_RETRYABLE_STATUSES = (502, 503, 504) + class Gw2Client: def __init__(self, bot): @@ -31,13 +36,35 @@ async def call_api(self, uri: str, key=None): endpoint = f"{gw2_variables.API_URI}/{uri}" headers = self._build_headers(key) - - async with self.bot.aiosession.get(endpoint, headers=headers) as response: - if response.status in (200, 206): - return await response.json() - - await self._handle_api_error(response, endpoint) - return None + max_attempts = _gw2_settings.api_retry_max_attempts + retry_delay = _gw2_settings.api_retry_delay + + for attempt in range(1, max_attempts + 1): + try: + async with self.bot.aiosession.get(endpoint, headers=headers) as response: + if response.status in (200, 206): + return await response.json() + + if response.status not in _RETRYABLE_STATUSES or attempt == max_attempts: + await self._handle_api_error(response, endpoint) + return None + + self.bot.log.warning( + f"GW2 API returned {response.status} for {endpoint.split('?')[0]}, " + f"retrying ({attempt}/{max_attempts})..." + ) + except APIError: + raise + except Exception: + if attempt == max_attempts: + raise + self.bot.log.warning( + f"GW2 API connection error for {endpoint.split('?')[0]}, retrying ({attempt}/{max_attempts})..." + ) + + await asyncio.sleep(retry_delay) + + return None def _build_headers(self, key=None): """Build HTTP headers for API request.""" diff --git a/tests/unit/gw2/tools/test_gw2_client.py b/tests/unit/gw2/tools/test_gw2_client.py index 78294dcd..e5a91c09 100644 --- a/tests/unit/gw2/tools/test_gw2_client.py +++ b/tests/unit/gw2/tools/test_gw2_client.py @@ -11,7 +11,7 @@ APIInvalidKey, APINotFound, ) -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, call, patch class AsyncContextManager: @@ -27,6 +27,19 @@ async def __aexit__(self, *args): pass +class AsyncContextManagerError: + """Helper to simulate async context manager that raises on enter.""" + + def __init__(self, error): + self.error = error + + async def __aenter__(self): + raise self.error + + async def __aexit__(self, *args): + pass + + class TestGw2ClientInit: """Test cases for Gw2Client.__init__.""" @@ -877,3 +890,190 @@ async def test_call_api_non_200_206_returns_none_after_error_handler(self, gw2_c mock_handler.assert_called_once() assert result is None + + +class TestCallApiRetry: + """Test cases for call_api retry logic on transient 5xx errors.""" + + @pytest.fixture + def mock_bot(self): + """Create a mock bot.""" + bot = MagicMock() + bot.log = MagicMock() + bot.description = "Test Bot" + return bot + + @pytest.fixture + def gw2_client(self, mock_bot): + """Create a Gw2Client instance.""" + return Gw2Client(mock_bot) + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_retry_succeeds_after_transient_504(self, mock_sleep, mock_settings, gw2_client): + """Test successful retry after a transient 504 then 200.""" + mock_settings.api_retry_max_attempts = 5 + mock_settings.api_retry_delay = 3.0 + + mock_504 = AsyncMock() + mock_504.status = 504 + mock_504.json = AsyncMock(return_value={"text": "gateway timeout"}) + + mock_200 = AsyncMock() + mock_200.status = 200 + mock_200.json = AsyncMock(return_value={"name": "TestAccount"}) + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock( + side_effect=[AsyncContextManager(mock_504), AsyncContextManager(mock_200)] + ) + + result = await gw2_client.call_api("account") + + assert result == {"name": "TestAccount"} + assert gw2_client.bot.aiosession.get.call_count == 2 + mock_sleep.assert_called_once_with(3.0) + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_exhausts_retries_on_persistent_504(self, mock_sleep, mock_settings, gw2_client): + """Test that persistent 504 exhausts all retries then raises APIInactiveError.""" + mock_settings.api_retry_max_attempts = 3 + mock_settings.api_retry_delay = 1.0 + + mock_504 = AsyncMock() + mock_504.status = 504 + mock_504.json = AsyncMock(return_value={"text": "gateway timeout"}) + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock(side_effect=[AsyncContextManager(mock_504) for _ in range(3)]) + + with pytest.raises(APIInactiveError): + await gw2_client.call_api("account") + + assert gw2_client.bot.aiosession.get.call_count == 3 + # Sleep is called between retries, not after the last attempt + assert mock_sleep.call_count == 2 + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_no_retry_on_4xx_errors(self, mock_sleep, mock_settings, gw2_client): + """Test that 4xx errors are not retried.""" + mock_settings.api_retry_max_attempts = 5 + mock_settings.api_retry_delay = 3.0 + + for status, exception_class in [(400, APIBadRequest), (403, APIForbidden), (404, APINotFound)]: + mock_response = AsyncMock() + mock_response.status = status + mock_response.json = AsyncMock(return_value={"text": "error"}) + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock(return_value=AsyncContextManager(mock_response)) + + with pytest.raises(exception_class): + await gw2_client.call_api("account") + + assert gw2_client.bot.aiosession.get.call_count == 1 + + mock_sleep.assert_not_called() + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_retry_delay_uses_configured_value(self, mock_sleep, mock_settings, gw2_client): + """Test that retry delay is called with the configured value.""" + mock_settings.api_retry_max_attempts = 3 + mock_settings.api_retry_delay = 7.0 + + mock_503 = AsyncMock() + mock_503.status = 503 + mock_503.json = AsyncMock(return_value={"text": "service unavailable"}) + + mock_200 = AsyncMock() + mock_200.status = 200 + mock_200.json = AsyncMock(return_value={"ok": True}) + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock( + side_effect=[ + AsyncContextManager(mock_503), + AsyncContextManager(mock_503), + AsyncContextManager(mock_200), + ] + ) + + result = await gw2_client.call_api("account") + + assert result == {"ok": True} + assert mock_sleep.call_args_list == [call(7), call(7)] + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_retry_on_connection_error_then_success(self, mock_sleep, mock_settings, gw2_client): + """Test retry on aiohttp connection error then success.""" + mock_settings.api_retry_max_attempts = 5 + mock_settings.api_retry_delay = 3.0 + + mock_200 = AsyncMock() + mock_200.status = 200 + mock_200.json = AsyncMock(return_value={"name": "TestAccount"}) + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock( + side_effect=[ + AsyncContextManagerError(OSError("Connection refused")), + AsyncContextManager(mock_200), + ] + ) + + result = await gw2_client.call_api("account") + + assert result == {"name": "TestAccount"} + assert gw2_client.bot.aiosession.get.call_count == 2 + mock_sleep.assert_called_once_with(3.0) + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_connection_error_exhausts_retries(self, mock_sleep, mock_settings, gw2_client): + """Test that persistent connection errors exhaust retries and re-raise.""" + mock_settings.api_retry_max_attempts = 2 + mock_settings.api_retry_delay = 1.0 + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock( + side_effect=[ + AsyncContextManagerError(OSError("Connection refused")), + AsyncContextManagerError(OSError("Connection refused")), + ] + ) + + with pytest.raises(OSError, match="Connection refused"): + await gw2_client.call_api("account") + + assert gw2_client.bot.aiosession.get.call_count == 2 + + @pytest.mark.asyncio + @patch("src.gw2.tools.gw2_client._gw2_settings") + @patch("src.gw2.tools.gw2_client.asyncio.sleep", new_callable=AsyncMock) + async def test_api_error_not_retried(self, mock_sleep, mock_settings, gw2_client): + """Test that APIError exceptions from _handle_api_error are not caught by retry.""" + mock_settings.api_retry_max_attempts = 5 + mock_settings.api_retry_delay = 3.0 + + mock_response = AsyncMock() + mock_response.status = 429 + mock_response.json = AsyncMock(return_value={"text": "rate limited"}) + + gw2_client.bot.aiosession = MagicMock() + gw2_client.bot.aiosession.get = MagicMock(return_value=AsyncContextManager(mock_response)) + + with pytest.raises(APIConnectionError): + await gw2_client.call_api("account") + + assert gw2_client.bot.aiosession.get.call_count == 1 + mock_sleep.assert_not_called()