more cleanup

Author: swaroopAkkineniWorkos

tests/test_http_client_retry.py

@@ -54,32 +54,28 @@ def mock_request(*args, **kwargs):
             assert mock_sleep.call_count == 2
 
     def test_retries_on_429_rate_limit(self, sync_http_client, retry_config, monkeypatch):
-        """Test that 429 errors trigger retry."""
+        """Test that 429 errors do NOT trigger retry (consistent with workos-node)."""
         call_count = 0
 
         def mock_request(*args, **kwargs):
             nonlocal call_count
             call_count += 1
-            if call_count < 2:
-                return httpx.Response(
-                    status_code=429,
-                    headers={"Retry-After": "0.1"},
-                    json={"error": "Rate limit exceeded"}
-                )
-            return httpx.Response(status_code=200, json={"success": True})
+            return httpx.Response(
+                status_code=429,
+                headers={"Retry-After": "0.1"},
+                json={"error": "Rate limit exceeded"}
+            )
 
         monkeypatch.setattr(sync_http_client._client, "request", MagicMock(side_effect=mock_request))
 
-        with patch("time.sleep") as mock_sleep:
-            response = sync_http_client.request("test/path", retry_config=retry_config)
+        with pytest.raises(BadRequestException):
+            sync_http_client.request("test/path", retry_config=retry_config)
 
-            assert call_count == 2
-            assert response == {"success": True}
-            # Verify Retry-After header was respected
-            mock_sleep.assert_called_once_with(0.1)
+        # Should only be called once (no retries on 429)
+        assert call_count == 1
 
     def test_no_retry_on_400_error(self, sync_http_client, monkeypatch):
-        """Test that 4xx errors (except 429) don't retry (no retry_config passed)."""
+        """Test that 4xx errors don't retry (no retry_config passed)."""
         call_count = 0
 
         def mock_request(*args, **kwargs):
@@ -345,33 +341,29 @@ async def mock_request(*args, **kwargs):
 
     @pytest.mark.asyncio
     async def test_retries_on_429_rate_limit(self, async_http_client, retry_config, monkeypatch):
-        """Test that 429 errors trigger retry."""
+        """Test that 429 errors do NOT trigger retry (consistent with workos-node)."""
         call_count = 0
 
         async def mock_request(*args, **kwargs):
             nonlocal call_count
             call_count += 1
-            if call_count < 2:
-                return httpx.Response(
-                    status_code=429,
-                    headers={"Retry-After": "0.1"},
-                    json={"error": "Rate limit exceeded"}
-                )
-            return httpx.Response(status_code=200, json={"success": True})
+            return httpx.Response(
+                status_code=429,
+                headers={"Retry-After": "0.1"},
+                json={"error": "Rate limit exceeded"}
+            )
 
         monkeypatch.setattr(async_http_client._client, "request", AsyncMock(side_effect=mock_request))
 
-        with patch("asyncio.sleep") as mock_sleep:
-            response = await async_http_client.request("test/path", retry_config=retry_config)
+        with pytest.raises(BadRequestException):
+            await async_http_client.request("test/path", retry_config=retry_config)
 
-            assert call_count == 2
-            assert response == {"success": True}
-            # Verify Retry-After header was respected
-            mock_sleep.assert_called_once_with(0.1)
+        # Should only be called once (no retries on 429)
+        assert call_count == 1
 
     @pytest.mark.asyncio
     async def test_no_retry_on_400_error(self, async_http_client, monkeypatch):
-        """Test that 4xx errors (except 429) don't retry (no retry_config passed)."""
+        """Test that 4xx errors don't retry (no retry_config passed)."""
         call_count = 0
 
         async def mock_request(*args, **kwargs):

workos/utils/_base_http_client.py

@@ -34,6 +34,8 @@
 
 DEFAULT_REQUEST_TIMEOUT = 25
 
+# Status codes that should trigger a retry (consistent with workos-node)
+RETRY_STATUS_CODES = [408, 500, 502, 504]
 
 @dataclass
 class RetryConfig:
@@ -43,13 +45,11 @@ class RetryConfig:
     max_delay: float = 30.0  # seconds
     jitter: float = 0.25  # 25% jitter
 
-
 ParamsType = Optional[Mapping[str, Any]]
 HeadersType = Optional[Dict[str, str]]
 JsonType = Optional[Union[Mapping[str, Any], Sequence[Any]]]
 ResponseJson = Mapping[Any, Any]
 
-
 class PreparedRequest(TypedDict):
     method: str
     url: str
@@ -213,30 +213,10 @@ def _handle_response(self, response: httpx.Response) -> ResponseJson:
 
     def _is_retryable_error(self, response: httpx.Response) -> bool:
         """Determine if an error should be retried."""
-        status_code = response.status_code
-        
-        # Retry on 5xx server errors
-        if 500 <= status_code < 600:
-            return True
-        
-        # Retry on 429 rate limit
-        if status_code == 429:
-            return True
-        
-        # Do NOT retry 4xx client errors (except 429)
-        return False
+        return response.status_code in RETRY_STATUS_CODES
 
     def _get_retry_delay(self, attempt: int, response: httpx.Response, retry_config: RetryConfig) -> float:
         """Calculate delay with exponential backoff and jitter."""
-        # Check for Retry-After header on 429 responses
-        if response.status_code == 429:
-            retry_after = response.headers.get("Retry-After")
-            if retry_after:
-                try:
-                    return float(retry_after)
-                except ValueError:
-                    pass  # Fall through to exponential backoff
-        
         # Exponential backoff: base_delay * 2^attempt
         delay = retry_config.base_delay * (2 ** attempt)