diff --git a/chargebee/http_request.py b/chargebee/http_request.py index 37fdd1a..45b7699 100644 --- a/chargebee/http_request.py +++ b/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): diff --git a/tests/test_rate_limit_retries.py b/tests/test_rate_limit_retries.py new file mode 100644 index 0000000..27c420c --- /dev/null +++ b/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))