Feature/devexp 1394 python support http 429 with without retry after header

CHANGELOG.md

@@ -20,6 +20,7 @@ All notable changes to the **Sinch Python SDK** are documented in this file.
 
 ### SDK
 
+- **[feature]** `HTTPTransport` now automatically retries rate-limited (`HTTP 429`) responses up to 3 times with backoff for endpoints that opt in via the `HTTPEndpoint.IS_RETRYABLE` flag (`False` by default). When the server sends a `Retry-After` header it is honoured; otherwise an exponential backoff with full jitter is used. If all retries are exhausted the last response is returned to the caller (#159).
 - **[dependency]** Set up minimum version for `requests` to `>=2.0.0` to prevent pulling in versions with known vulnerabilities (#152).
 - **[fix]** Fixed a race condition in OAuth token creation and renewal under concurrent requests: `TokenManagerBase` now uses a lock with double-checked locking so the initial token is fetched exactly once, and a new `refresh_auth_token(used_token)` deduplicates concurrent renewals by only fetching when the stale token still matches the cached one (#156).
 - **[refactor]** `HTTPTransport` now prepares and authenticates requests in `request()`, so the new `send_request(request_data)` receives an already-prepared `HttpRequest` and acts as a pure I/O primitive, simplifying subclassing (#156).

sinch/core/endpoint.py

@@ -4,6 +4,7 @@
 
 class HTTPEndpoint(ABC):
     ENDPOINT_URL = None
+    IS_RETRYABLE = False
 
     @property
     @abstractmethod

sinch/core/ports/http_transport.py

@@ -1,5 +1,9 @@
+import random
+import time
 import warnings
 from abc import ABC
+from datetime import datetime, timezone
+from email.utils import parsedate_to_datetime
 from platform import python_version
 from typing import Optional
 
@@ -26,6 +30,11 @@ class HTTPTransport(ABC):
         ``send`` override path will be removed in 3.0.
     """
 
+    MAX_RETRIES = 3
+    RETRYABLE_STATUS_CODES = frozenset({429})
+    BACKOFF_BASE_SECONDS = 1.0
+    BACKOFF_GROWTH = 4
+
     def __init__(self, sinch):
         self.sinch = sinch
         self._legacy_send = self._uses_legacy_send()
@@ -91,15 +100,114 @@ def request(self, endpoint: HTTPEndpoint) -> HTTPResponse:
 
         request_data = self.prepare_request(endpoint)
         request_data = self.authenticate(endpoint, request_data)
-        http_response = self.send_request(request_data)
+
+        http_response = self._send_with_retries(endpoint, request_data)
 
         if self._should_refresh_token(endpoint, http_response):
             used_token = self._get_bearer_token_from_request(request_data)
             new_token = self.sinch.configuration.token_manager.refresh_auth_token(used_token)
             self._set_bearer_token(request_data, new_token.access_token)
-            http_response = self.send_request(request_data)
+            http_response = self._send_with_retries(endpoint, request_data)
 
         return endpoint.handle_response(http_response)
+
+
+    def _send_with_retries(
+        self, endpoint: HTTPEndpoint, request_data: Optional[HttpRequest] = None
+    ) -> HTTPResponse:
+        """
+        Sends a request, retrying rate-limited responses (up to MAX_RETRIES,
+        with backoff) for endpoints that opt in via :attr:`HTTPEndpoint.IS_RETRYABLE`.
+
+        :param endpoint: The endpoint being called.
+        :type endpoint: HTTPEndpoint
+        :param request_data: The prepared request, or ``None`` on the legacy ``send`` path.
+        :type request_data: Optional[HttpRequest]
+        :returns: The HTTP response from the last attempt.
+        :rtype: HTTPResponse
+        """
+        num_retries = 0
+        while True:
+            if request_data is None:
+                http_response = self.send(endpoint)
+            else:
+                http_response = self.send_request(request_data)
+
+            if self._should_retry(endpoint, http_response, num_retries):
+                time.sleep(self._compute_backoff(http_response, num_retries))
+                num_retries += 1
+            else:
+                return http_response
+
+    def _should_retry(
+        self, endpoint: HTTPEndpoint, http_response: HTTPResponse, num_retries: int
+    ) -> bool:
+        """
+        Returns True when the endpoint opts into retries, the response is a
+        transient error and retries remain.
+
+        :param endpoint: The endpoint being called.
+        :type endpoint: HTTPEndpoint
+        :param http_response: The response received.
+        :type http_response: HTTPResponse
+        :param num_retries: Number of retries already performed.
+        :type num_retries: int
+        :returns: Whether the request should be retried.
+        :rtype: bool
+        """
+        if not endpoint.IS_RETRYABLE:
+            return False
+        if num_retries >= self.MAX_RETRIES:
+            return False
+        return http_response.status_code in self.RETRYABLE_STATUS_CODES
+
+    def _compute_backoff(self, http_response: HTTPResponse, num_retries: int) -> float:
+        """
+        Computes how long to wait before the next retry.
+
+        :param http_response: The response received.
+        :type http_response: HTTPResponse
+        :param num_retries: Number of retries already performed.
+        :type num_retries: int
+        :returns: The delay in seconds.
+        :rtype: float
+        """
+
+        headers = {key.lower(): value for key, value in http_response.headers.items()}
+        retry_after_seconds = self._parse_retry_after(headers.get("retry-after"))
+        if retry_after_seconds is not None:
+            return retry_after_seconds + random.uniform(0, 0.25)
+
+        max_delay = self.BACKOFF_BASE_SECONDS * (self.BACKOFF_GROWTH ** num_retries)
+        return random.uniform(0, max_delay)
+
+    @staticmethod
+    def _parse_retry_after(value: Optional[str]) -> Optional[float]:
+        """
+        Parses the Retry-After header into a
+        delay in seconds, or None if absent/unparseable.
+
+        :param value: The raw header value.
+        :type value: Optional[str]
+        :returns: The delay in seconds, or None if absent/invalid.
+        :rtype: Optional[float]
+        """
+        if not value:
+            return None
+
+        try:
+            seconds = float(value)
+            return seconds if seconds >= 0 else None
+        except ValueError:
+            pass
+
+        try:
+            retry_at = parsedate_to_datetime(value)
+        except (TypeError, ValueError):
+            return None
+        if retry_at.tzinfo is None:
+            retry_at = retry_at.replace(tzinfo=timezone.utc)
+        return max(0.0, (retry_at - datetime.now(timezone.utc)).total_seconds())
 
 
     def authenticate(self, endpoint: HTTPEndpoint, request_data: HttpRequest) -> HttpRequest:
@@ -215,12 +323,12 @@ def _legacy_request(self, endpoint: HTTPEndpoint) -> HTTPResponse:
         :rtype: HTTPResponse
         """
         token_before = self.sinch.configuration.token_manager.token
-        http_response = self.send(endpoint)
+        http_response = self._send_with_retries(endpoint, request_data=None)
 
         if self._should_refresh_token(endpoint, http_response):
             used_token = token_before.access_token if token_before else None
             self.sinch.configuration.token_manager.refresh_auth_token(used_token)
-            http_response = self.send(endpoint)
+            http_response = self._send_with_retries(endpoint, request_data=None)
 
         return endpoint.handle_response(http_response)
 

sinch/domains/authentication/endpoints/v1/oauth.py

@@ -9,6 +9,7 @@ class OAuthEndpoint(HTTPEndpoint):
     ENDPOINT_URL = "{origin}/oauth2/token"
     HTTP_METHOD = HTTPMethods.POST.value
     HTTP_AUTHENTICATION = HTTPAuthentication.BASIC.value
+    IS_RETRYABLE = True
 
     def __init__(self):
         pass

tests/unit/test_http_transport.py

@@ -1,4 +1,6 @@
 import json
+import random
+import time
 import pytest
 from unittest.mock import Mock
 from sinch.core.enums import HTTPAuthentication
@@ -13,7 +15,7 @@
 
 
 # Mock classes and fixtures
-def _make_mock_endpoint(auth_type, error_on_4xx=False):
+def _make_mock_endpoint(auth_type, error_on_4xx=False, is_retryable=False):
     """Create a MockEndpoint that satisfies the abstract property contract."""
 
     class _Endpoint(HTTPEndpoint):
@@ -45,6 +47,7 @@ def handle_response(self, response: HTTPResponse):
                 )
             return response
 
+    _Endpoint.IS_RETRYABLE = is_retryable
     return _Endpoint()
 
 
@@ -92,6 +95,12 @@ def mock_sinch():
     return sinch
 
 
+@pytest.fixture
+def no_sleep(mocker):
+    mocker.patch.object(random, "uniform", return_value=0.0)
+    return mocker.patch.object(time, "sleep")
+
+
 @pytest.fixture
 def base_request():
     return HttpRequest(
@@ -276,6 +285,148 @@ def test_no_refresh_on_401_without_www_authenticate(self, mock_sinch):
         token_manager.refresh_auth_token.assert_not_called()
 
 
+class TestRetryWithBackoff:
+    """Tests for the automatic retry-with-backoff on rate-limited (429) responses."""
+
+    @staticmethod
+    def _rate_limited(headers=None):
+        return _requests_response(429, body={"error": "rate limited"}, headers=headers)
+
+    def test_retries_on_429_then_succeeds(self, mock_sinch, no_sleep):
+        transport = HTTPTransportRequests(mock_sinch)
+        transport.http_session.request = Mock(side_effect=[
+            self._rate_limited(),
+            self._rate_limited(),
+            _requests_response(200, body={"ok": True}),
+        ])
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=True)
+
+        result = transport.request(endpoint)
+
+        assert result.status_code == 200
+        assert transport.http_session.request.call_count == 3
+        assert no_sleep.call_count == 2
+
+    def test_gives_up_and_returns_last_response_after_max_retries(self, mock_sinch, no_sleep):
+        transport = HTTPTransportRequests(mock_sinch)
+        transport.http_session.request = Mock(return_value=self._rate_limited())
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=True)
+
+        result = transport.request(endpoint)
+
+        assert result.status_code == 429
+        assert transport.http_session.request.call_count == HTTPTransport.MAX_RETRIES + 1
+        assert no_sleep.call_count == HTTPTransport.MAX_RETRIES
+
+    def test_no_retry_on_success(self, mock_sinch, no_sleep):
+        transport = HTTPTransportRequests(mock_sinch)
+        transport.http_session.request = Mock(return_value=_requests_response(200, body={"ok": True}))
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value)
+
+        result = transport.request(endpoint)
+
+        assert result.status_code == 200
+        assert transport.http_session.request.call_count == 1
+        no_sleep.assert_not_called()
+
+    def test_no_retry_on_non_retryable_status(self, mock_sinch, no_sleep):
+        transport = HTTPTransportRequests(mock_sinch)
+        transport.http_session.request = Mock(return_value=_requests_response(400, body={"error": "bad"}))
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value)
+
+        result = transport.request(endpoint)
+
+        assert result.status_code == 400
+        assert transport.http_session.request.call_count == 1
+        no_sleep.assert_not_called()
+
+    def test_honors_retry_after_header(self, mock_sinch, no_sleep):
+        transport = HTTPTransportRequests(mock_sinch)
+        transport.http_session.request = Mock(side_effect=[
+            self._rate_limited(headers={"Retry-After": "7"}),
+            _requests_response(200, body={"ok": True}),
+        ])
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=True)
+
+        transport.request(endpoint)
+
+        no_sleep.assert_called_once_with(7.0)
+
+    def test_no_retry_when_endpoint_not_retryable(self, mock_sinch, no_sleep):
+        transport = HTTPTransportRequests(mock_sinch)
+        transport.http_session.request = Mock(return_value=self._rate_limited())
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=False)
+
+        result = transport.request(endpoint)
+
+        assert result.status_code == 429
+        assert transport.http_session.request.call_count == 1
+        no_sleep.assert_not_called()
+
+
+class TestShouldRetry:
+    def test_retries_429_while_attempts_remain(self, mock_sinch):
+        transport = HTTPTransportRequests(mock_sinch)
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=True)
+        response = HTTPResponse(status_code=429, headers={}, body={})
+
+        assert transport._should_retry(endpoint, response, num_retries=0) is True
+
+    def test_stops_when_max_retries_reached(self, mock_sinch):
+        transport = HTTPTransportRequests(mock_sinch)
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=True)
+        response = HTTPResponse(status_code=429, headers={}, body={})
+
+        assert transport._should_retry(endpoint, response, num_retries=HTTPTransport.MAX_RETRIES) is False
+
+    def test_does_not_retry_non_retryable_status(self, mock_sinch):
+        transport = HTTPTransportRequests(mock_sinch)
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=True)
+        response = HTTPResponse(status_code=200, headers={}, body={})
+
+        assert transport._should_retry(endpoint, response, num_retries=0) is False
+
+    def test_does_not_retry_when_endpoint_not_retryable(self, mock_sinch):
+        transport = HTTPTransportRequests(mock_sinch)
+        endpoint = _make_mock_endpoint(HTTPAuthentication.BASIC.value, is_retryable=False)
+        response = HTTPResponse(status_code=429, headers={}, body={})
+
+        assert transport._should_retry(endpoint, response, num_retries=0) is False
+
+
+class TestComputeBackoff:
+    def test_uses_retry_after_header_when_present(self, mock_sinch):
+        transport = HTTPTransportRequests(mock_sinch)
+        response = HTTPResponse(status_code=429, headers={"Retry-After": "5"}, body={})
+
+        backoff = transport._compute_backoff(response, num_retries=0)
+
+        assert 5.0 <= backoff < 5.25
+
+    def test_exponential_growth_when_no_header(self, mock_sinch):
+        transport = HTTPTransportRequests(mock_sinch)
+        response = HTTPResponse(status_code=429, headers={}, body={})
+
+        assert 0.0 <= transport._compute_backoff(response, num_retries=0) <= 1.0
+        assert 0.0 <= transport._compute_backoff(response, num_retries=1) <= 4.0
+        assert 0.0 <= transport._compute_backoff(response, num_retries=2) <= 16.0
+
+
+class TestParseRetryAfter:
+    @pytest.mark.parametrize("value,expected", [
+        ("5", 5.0),
+        ("0", 0.0),
+        ("-3", None),
+        ("abc", None),
+        ("", None),
+        (None, None),
+        ("Wed, 21 Oct 2015 07:28:00 GMT", 0.0),
+        ("Wed, 21 Oct 2015 07:28:00", 0.0),
+    ])
+    def test_parse_retry_after(self, value, expected):
+        assert HTTPTransport._parse_retry_after(value) == expected
+
+
 class _LegacyTransport(HTTPTransport):
     """A pre-2.1 transport that overrides the deprecated ``send(endpoint)`` hook."""