feat: rework GET retry logic with exponential backoff, jitter, and Retry-After support

pyproject.toml

@@ -23,7 +23,6 @@ build-backend = "hatchling.build"
 
 [dependency-groups]
 dev = [
-    "ipykernel>=6.29.5",
     "pytest>=9.0.3",
     "pytest-asyncio>=0.23.0",
     "pytest-cov>=7.1.0",

src/pyfwapi/apiconnection.py

@@ -1,4 +1,5 @@
 import asyncio
+import random
 import typing as t
 import urllib.parse
 
@@ -10,6 +11,39 @@
 from pyfwapi.log import pyfwapiLog
 from pyfwapi.model.basemodel import APIResponse
 
+# Transient HTTP statuses worth retrying on idempotent GETs.
+# Other 4xx are client errors and must raise immediately.
+RETRYABLE_STATUS_CODES = frozenset({408, 429, 500, 502, 503, 504})
+
+# Transport-level errors that may be retried.
+RETRYABLE_TRANSPORT_ERRORS = (
+    httpx.ConnectError,
+    httpx.ConnectTimeout,
+    httpx.ReadTimeout,
+    httpx.RemoteProtocolError,
+)
+
+MAX_GET_ATTEMPTS = 5
+BACKOFF_BASE_SECONDS = 1.0
+BACKOFF_MAX_SECONDS = 16.0
+
+
+def _backoff_delay(attempt: int) -> float:
+    """Exponential backoff with jitter: ~1s, 2s, 4s, 8s, 16s."""
+    delay = min(BACKOFF_BASE_SECONDS * 2**attempt, BACKOFF_MAX_SECONDS)
+    return delay + random.uniform(0, delay * 0.5)
+
+
+def _retry_after(response: httpx.Response) -> float | None:
+    """Parse a numeric `Retry-After` response header, if present."""
+    value = response.headers.get("Retry-After")
+    if value is None:
+        return None
+    try:
+        return min(float(value), BACKOFF_MAX_SECONDS)
+    except ValueError:
+        return None
+
 
 class APIConnection:
     # For implementers, this class only concerns itself with the OAuth2 token,
@@ -62,43 +96,72 @@ async def GET(
         path: str,
         /,
         *,
-        retry_attempt=0,
         headers: t.Mapping[str, str] = {},
         **kwargs,
     ) -> httpx.Response:
         """
         Perform GET request on the API and return JSON.
 
+        GETs are idempotent, so transient failures (HTTP 408/429/5xx and
+        transport-level connection errors) are retried with exponential
+        backoff and jitter. Other 4xx responses raise immediately.
+
         Raises:
             httpx.HTTPStatusError: API response if the status code is not 200.
             httpx.ConnectTimeout: Server side rate limit exceeded
             httpx.RemoteProtocolError: Server side rate limit exceeded
         """
-        try:
-            await self.ensure_token()
-            pyfwapiLog.debug(f"GET {urllib.parse.unquote(path)}")
-            async with self.rate_limit:
-                r = await self.client.get(
-                    self.HOST + path,
-                    headers={"Accept": "application/json", **headers},
-                    follow_redirects=True,
-                    **kwargs,
-                )
-                r.raise_for_status()
-                return r
-
-        except (httpx.ConnectTimeout, httpx.RemoteProtocolError, httpx.ReadTimeout):
-            # A possible effect of the server-side rate limiter
-            # Solution: retry once, after waiting about a minute.
-            if retry_attempt <= 3:
-                print(f"Server connection severed. Retrying {retry_attempt}")
+        last_error: BaseException | None = None
+
+        for attempt in range(MAX_GET_ATTEMPTS):
+            try:
+                await self.ensure_token()
+                pyfwapiLog.debug(f"GET {urllib.parse.unquote(path)}")
+                async with self.rate_limit:
+                    r = await self.client.get(
+                        self.HOST + path,
+                        headers={"Accept": "application/json", **headers},
+                        follow_redirects=True,
+                        **kwargs,
+                    )
+                    r.raise_for_status()
+                    return r
+
+            except httpx.HTTPStatusError as e:
+                # Non-retryable client errors (e.g. 401/403/404) raise at once.
+                if e.response.status_code not in RETRYABLE_STATUS_CODES:
+                    raise
+                last_error = e
+                reason = f"HTTP {e.response.status_code}"
+                delay = _retry_after(e.response) or _backoff_delay(attempt)
+
+            except RETRYABLE_TRANSPORT_ERRORS as e:
+                # A possible effect of the server-side rate limiter.
+                last_error = e
+                reason = f"{type(e).__name__}: {e}"
+                delay = _backoff_delay(attempt)
+                # Refresh the token on the next attempt; the connection
+                # reset may have invalidated the session state.
                 self.client.token = None
-                await asyncio.sleep(60)
-                return await self.GET(
-                    path, retry_attempt=retry_attempt + 1, headers=headers, **kwargs
+
+            if attempt < MAX_GET_ATTEMPTS - 1:
+                pyfwapiLog.warning(
+                    "Transient error on GET %s: %s. Retrying attempt %d/%d in %.1fs.",
+                    urllib.parse.unquote(path),
+                    reason,
+                    attempt + 2,
+                    MAX_GET_ATTEMPTS,
+                    delay,
                 )
+                await asyncio.sleep(delay)
 
-            raise
+        pyfwapiLog.error(
+            "GET %s failed after %d attempts.",
+            urllib.parse.unquote(path),
+            MAX_GET_ATTEMPTS,
+        )
+        assert last_error is not None
+        raise last_error
 
     async def PATCH(
         self,

tests/test_apiconnection.py

@@ -79,9 +79,97 @@ async def test_get_retry_on_timeout(self, mock_sleep, api_conn):
         resp = await api_conn.GET("/flakey/path")
 
         assert api_conn.client.get.call_count == 2
-        mock_sleep.assert_awaited_once_with(60)
+        mock_sleep.assert_awaited_once()
         assert resp.json() == {"retry": "success"}
 
+    @pytest.mark.asyncio
+    @patch("asyncio.sleep", new_callable=AsyncMock)
+    async def test_get_retry_on_transient_500(self, mock_sleep, api_conn):
+        """A single transient 500 must not abort the request."""
+        api_conn.client.token = {"access_token": "abc"}
+
+        request = httpx.Request("GET", "https://test.fotoware.cloud/flakey/path?p=115")
+        api_conn.client.get.side_effect = [
+            httpx.Response(500, request=request),
+            httpx.Response(200, json={"ok": True}, request=request),
+        ]
+
+        resp = await api_conn.GET("/flakey/path?p=115")
+
+        assert api_conn.client.get.call_count == 2
+        mock_sleep.assert_awaited_once()
+        assert resp.json() == {"ok": True}
+
+    @pytest.mark.asyncio
+    @patch("asyncio.sleep", new_callable=AsyncMock)
+    async def test_get_retry_burst_under_max_attempts(self, mock_sleep, api_conn):
+        """A burst of fewer than 5 transient 5xx errors still succeeds."""
+        api_conn.client.token = {"access_token": "abc"}
+
+        request = httpx.Request("GET", "https://test.fotoware.cloud/flakey/path")
+        api_conn.client.get.side_effect = [
+            httpx.Response(500, request=request),
+            httpx.Response(502, request=request),
+            httpx.Response(503, request=request),
+            httpx.Response(200, json={"ok": True}, request=request),
+        ]
+
+        resp = await api_conn.GET("/flakey/path")
+
+        assert api_conn.client.get.call_count == 4
+        assert mock_sleep.await_count == 3
+        assert resp.json() == {"ok": True}
+
+    @pytest.mark.asyncio
+    @patch("asyncio.sleep", new_callable=AsyncMock)
+    async def test_get_persistent_5xx_raises_after_max_attempts(
+        self, mock_sleep, api_conn
+    ):
+        """Persistent 5xx raises HTTPStatusError after all attempts."""
+        api_conn.client.token = {"access_token": "abc"}
+
+        request = httpx.Request("GET", "https://test.fotoware.cloud/broken/path")
+        api_conn.client.get.return_value = httpx.Response(503, request=request)
+
+        with pytest.raises(httpx.HTTPStatusError):
+            await api_conn.GET("/broken/path")
+
+        assert api_conn.client.get.call_count == 5
+        assert mock_sleep.await_count == 4
+
+    @pytest.mark.asyncio
+    @patch("asyncio.sleep", new_callable=AsyncMock)
+    async def test_get_429_honors_retry_after(self, mock_sleep, api_conn):
+        """A 429 with a Retry-After header waits the requested time."""
+        api_conn.client.token = {"access_token": "abc"}
+
+        request = httpx.Request("GET", "https://test.fotoware.cloud/limited/path")
+        api_conn.client.get.side_effect = [
+            httpx.Response(429, headers={"Retry-After": "7"}, request=request),
+            httpx.Response(200, json={"ok": True}, request=request),
+        ]
+
+        resp = await api_conn.GET("/limited/path")
+
+        assert api_conn.client.get.call_count == 2
+        mock_sleep.assert_awaited_once_with(7.0)
+        assert resp.json() == {"ok": True}
+
+    @pytest.mark.asyncio
+    @patch("asyncio.sleep", new_callable=AsyncMock)
+    async def test_get_4xx_raises_immediately(self, mock_sleep, api_conn):
+        """Client errors (other than 408/429) are not retried."""
+        api_conn.client.token = {"access_token": "abc"}
+
+        request = httpx.Request("GET", "https://test.fotoware.cloud/missing")
+        api_conn.client.get.return_value = httpx.Response(404, request=request)
+
+        with pytest.raises(httpx.HTTPStatusError):
+            await api_conn.GET("/missing")
+
+        api_conn.client.get.assert_awaited_once()
+        mock_sleep.assert_not_awaited()
+
     @pytest.mark.asyncio
     async def test_patch_success(self, api_conn):
         api_conn.client.token = {"access_token": "abc"}