diff --git a/atlassian/rest_client.py b/atlassian/rest_client.py index 240b13aee..73a1619b2 100644 --- a/atlassian/rest_client.py +++ b/atlassian/rest_client.py @@ -3,6 +3,8 @@ import logging import random import time +from datetime import datetime, timezone +from email.utils import parsedate_to_datetime from http.cookiejar import CookieJar from json import dumps from typing import ( @@ -295,6 +297,44 @@ def _calculate_backoff_value(self, retry_count): backoff_value += random.uniform(0, self.backoff_jitter) # nosec B311 return float(max(0, min(self.max_backoff_seconds, backoff_value))) + def _parse_retry_after_header(self, header_value: Optional[str]) -> Optional[float]: + """ + Parse the Retry-After header and return a safe delay (seconds). + The Retry-After header may contain either an integer (delta-seconds) + or an HTTP-date. Values are clamped to ``self.max_backoff_seconds`` to + avoid ``time.sleep`` overflow on some platforms. + """ + if not header_value: + return None + + delay_seconds: Optional[float] + try: + delay_seconds = float(header_value) + except (TypeError, ValueError): + try: + retry_after_dt = parsedate_to_datetime(header_value) + except (TypeError, ValueError): + log.warning("Unable to parse Retry-After header value '%s'", header_value) + return None + + if retry_after_dt.tzinfo is None: + retry_after_dt = retry_after_dt.replace(tzinfo=timezone.utc) + delay_seconds = (retry_after_dt - datetime.now(timezone.utc)).total_seconds() + + if delay_seconds is None: + return None + + delay_seconds = max(0.0, delay_seconds) + if delay_seconds > self.max_backoff_seconds: + log.debug( + "Retry-After value %.2f exceeds max_backoff_seconds (%s); clamping", + delay_seconds, + self.max_backoff_seconds, + ) + delay_seconds = float(self.max_backoff_seconds) + + return delay_seconds + def _retry_handler(self): """ Creates and returns a retry handler function for managing HTTP request retries. @@ -306,13 +346,22 @@ def _retry_handler(self): returns `True` if the request should be retried, or `False` otherwise. """ retries = 0 + retry_with_header_count = 0 + max_retry_with_header_attempts = 1 # Only retry once for Retry-After header def _handle(response): - nonlocal retries - - if self.retry_with_header and "Retry-After" in response.headers and response.status_code == 429: - time.sleep(int(response.headers["Retry-After"])) - return True + nonlocal retries, retry_with_header_count + + if self.retry_with_header and response.status_code == 429: + if retry_with_header_count >= max_retry_with_header_attempts: + log.debug("Max retry attempts for Retry-After header reached, not retrying") + return False + delay = self._parse_retry_after_header(response.headers.get("Retry-After")) + if delay is not None: + retry_with_header_count += 1 + log.debug("Retrying after %s seconds (attempt %d/%d)", delay, retry_with_header_count, max_retry_with_header_attempts) + time.sleep(delay) + return True if not self.backoff_and_retry or self.use_urllib3_retry: return False diff --git a/tests/test_rest_client.py b/tests/test_rest_client.py index 03a647dde..e91d0e81c 100644 --- a/tests/test_rest_client.py +++ b/tests/test_rest_client.py @@ -3,7 +3,11 @@ Unit tests for atlassian.rest_client module """ +from datetime import datetime, timedelta +from types import SimpleNamespace + import pytest + from .mockup import mockup_server from atlassian.rest_client import AtlassianRestAPI @@ -369,3 +373,66 @@ def test_cloud_vs_server_behavior(self): # Cloud flag should be different assert cloud_api.cloud is True assert server_api.cloud is False + + def test_retry_handler_clamps_retry_after(self, monkeypatch): + """Ensure large Retry-After headers are clamped to max_backoff_seconds.""" + captured = {} + + def fake_sleep(delay): + captured["delay"] = delay + + monkeypatch.setattr("atlassian.rest_client.time.sleep", fake_sleep) + + api = AtlassianRestAPI( + url=f"{mockup_server()}/test", + retry_with_header=True, + max_backoff_seconds=5, + ) + + handler = api._retry_handler() + response = SimpleNamespace(headers={"Retry-After": "99999999999"}, status_code=429) + + assert handler(response) is True + assert captured["delay"] == 5 + + def test_retry_handler_parses_http_date(self, monkeypatch): + """Ensure HTTP-date Retry-After headers are converted to delta seconds.""" + captured = {} + + def fake_sleep(delay): + captured["delay"] = delay + + monkeypatch.setattr("atlassian.rest_client.time.sleep", fake_sleep) + + api = AtlassianRestAPI( + url=f"{mockup_server()}/test", + retry_with_header=True, + max_backoff_seconds=60, + ) + + handler = api._retry_handler() + future_delay = 10 + future_date = datetime.utcnow().replace(tzinfo=None) + timedelta(seconds=future_delay) + retry_after_value = future_date.strftime("%a, %d %b %Y %H:%M:%S GMT") + response = SimpleNamespace(headers={"Retry-After": retry_after_value}, status_code=429) + + assert handler(response) is True + assert pytest.approx(captured["delay"], rel=0.1) == future_delay + + def test_retry_handler_skips_invalid_header(self, monkeypatch): + """Ensure invalid Retry-After headers fall back to regular logic.""" + def fake_sleep(_): + raise AssertionError("sleep should not be called for invalid header") + + monkeypatch.setattr("atlassian.rest_client.time.sleep", fake_sleep) + + api = AtlassianRestAPI( + url=f"{mockup_server()}/test", + retry_with_header=True, + ) + + handler = api._retry_handler() + response = SimpleNamespace(headers={"Retry-After": "invalid-value"}, status_code=429) + + # Should return False so that other retry mechanisms can take over + assert handler(response) is False