fix: bound rate-limit retries and honor Retry-After

chargebee/http_request.py

@@ -20,6 +20,9 @@
 from chargebee.main import Chargebee
 from chargebee.version import VERSION
 
+# Longest Retry-After delay the client will wait for; longer values use the default delay.
+MAX_RETRY_AFTER_MS = 60_000
+
 _logger = logging.getLogger(__name__)
 
 
@@ -138,11 +141,17 @@ def _process_response(url, request_args, retry_config, enable_debug_logs):
         except Exception as err:
             status_code = extract_status_code(err)
 
-            if not retry_config or not retry_config.is_enabled():
+            if (
+                not retry_config
+                or not retry_config.is_enabled()
+                or retry_count >= retry_config.get_max_retries()
+            ):
                 raise err
 
             if status_code == 429:
-                delay_ms = parse_retry_after(err) or retry_config.get_delay_ms()
+                delay_ms = parse_retry_after(err)
+                if delay_ms is None:
+                    delay_ms = retry_config.get_delay_ms()
                 log(
                     f"Rate limit hit. Retrying in {delay_ms}ms",
                     "INFO",
@@ -198,11 +207,17 @@ async def _process_response_async(url, request_args, retry_config, enable_debug_
         except Exception as err:
             status_code = extract_status_code(err)
 
-            if not retry_config or not retry_config.is_enabled():
+            if (
+                not retry_config
+                or not retry_config.is_enabled()
+                or retry_count >= retry_config.get_max_retries()
+            ):
                 raise err
 
             if status_code == 429:
-                delay_ms = parse_retry_after(err) or retry_config.get_delay_ms()
+                delay_ms = parse_retry_after(err)
+                if delay_ms is None:
+                    delay_ms = retry_config.get_delay_ms()
                 log(
                     f"Rate limit hit. Retrying in {delay_ms}ms",
                     "INFO",
@@ -309,22 +324,38 @@ def extract_status_code(err):
 
 
 def parse_retry_after(err):
-    headers = getattr(err, "response_headers", {}) or {}
+    """Return the Retry-After delay in milliseconds, or None to use the default delay.
+
+    Delays longer than MAX_RETRY_AFTER_MS are ignored, so a server cannot make the
+    client sleep for an unreasonable time (for example an HTTP-date far in the future).
+    """
+    headers = getattr(err, "http_headers", {}) or {}
     retry_after = headers.get("retry-after") or headers.get("Retry-After")
     if not retry_after:
         return None
     try:
-        return int(retry_after) * 1000
+        delay_ms = max(0, int(retry_after) * 1000)
     except ValueError:
         try:
             from email.utils import parsedate_to_datetime
 
             retry_time = parsedate_to_datetime(retry_after)
-            return max(
-                0, int((retry_time - datetime.datetime.utcnow()).total_seconds() * 1000)
+            if retry_time.tzinfo is None:
+                retry_time = retry_time.replace(tzinfo=datetime.timezone.utc)
+            delay_ms = max(
+                0,
+                int(
+                    (
+                        retry_time - datetime.datetime.now(datetime.timezone.utc)
+                    ).total_seconds()
+                    * 1000
+                ),
             )
         except Exception:
             return None
+    if delay_ms > MAX_RETRY_AFTER_MS:
+        return None
+    return delay_ms
 
 
 def calculate_backoff_delay(retry_count, base_delay_ms):

tests/test_rate_limit_retries.py

@@ -0,0 +1,88 @@
+import asyncio
+import datetime
+import unittest
+from unittest.mock import AsyncMock, Mock, patch
+
+from chargebee.api_error import APIError
+from chargebee.http_request import (
+    _process_response,
+    MAX_RETRY_AFTER_MS,
+    _process_response_async,
+    parse_retry_after,
+)
+from chargebee.retry_config import RetryConfig
+
+
+class RateLimitRetryTests(unittest.TestCase):
+    def test_rate_limit_respects_retry_limit(self):
+        for asynchronous in (False, True):
+            for retries in (0, 2):
+                with self.subTest(asynchronous=asynchronous, retries=retries):
+                    error = APIError(429, {"message": "limited"}, {"Retry-After": "2"})
+                    # A final successful response bounds the regression on broken code.
+                    effects = [error] * (retries + 1) + [({}, {}, 200)]
+                    request = (
+                        AsyncMock(side_effect=effects)
+                        if asynchronous
+                        else Mock(side_effect=effects)
+                    )
+                    sleep = AsyncMock() if asynchronous else Mock()
+                    suffix = "_async" if asynchronous else ""
+                    with (
+                        patch("chargebee.http_request._make_request" + suffix, request),
+                        patch("chargebee.http_request.sleep" + suffix, sleep),
+                    ):
+                        with self.assertRaises(APIError) as caught:
+                            args = (
+                                "https://example.com",
+                                {"method": "GET", "headers": {}},
+                                RetryConfig(enabled=True, max_retries=retries),
+                                False,
+                            )
+                            if asynchronous:
+                                asyncio.run(_process_response_async(*args))
+                            else:
+                                _process_response(*args)
+                        self.assertIs(caught.exception, error)
+                        self.assertEqual(request.call_count, retries + 1)
+                        self.assertEqual(sleep.call_count, retries)
+                        for call in sleep.call_args_list:
+                            self.assertEqual(call.args, (2000,))
+
+    def test_retry_after_from_api_error_headers(self):
+        for value, expected in (
+            ("2", 2000),
+            ("0", 0),
+            ("-5", 0),
+            ("invalid", None),
+            ("60", MAX_RETRY_AFTER_MS),
+            ("61", None),
+            ("86400", None),
+        ):
+            with self.subTest(value=value):
+                error = APIError(429, {}, {"Retry-After": value})
+                self.assertEqual(parse_retry_after(error), expected)
+
+    def test_retry_after_http_date(self):
+        error = APIError(429, {}, {"Retry-After": "Thu, 01 Oct 2026 12:00:02 GMT"})
+        now = datetime.datetime(2026, 10, 1, 12, tzinfo=datetime.timezone.utc)
+
+        class FrozenDatetime(datetime.datetime):
+            @classmethod
+            def now(cls, tz=None):
+                return now
+
+        with patch("chargebee.http_request.datetime.datetime", FrozenDatetime):
+            self.assertEqual(parse_retry_after(error), 2000)
+
+    def test_retry_after_http_date_too_far_uses_default_delay(self):
+        error = APIError(429, {}, {"Retry-After": "Fri, 01 Oct 2027 12:00:00 GMT"})
+        now = datetime.datetime(2026, 10, 1, 12, tzinfo=datetime.timezone.utc)
+
+        class FrozenDatetime(datetime.datetime):
+            @classmethod
+            def now(cls, tz=None):
+                return now
+
+        with patch("chargebee.http_request.datetime.datetime", FrozenDatetime):
+            self.assertIsNone(parse_retry_after(error))