diff --git a/examples/advanced/complete_walkthrough.py b/examples/advanced/complete_walkthrough.py index 948fced..2d9a6f2 100644 --- a/examples/advanced/complete_walkthrough.py +++ b/examples/advanced/complete_walkthrough.py @@ -423,7 +423,6 @@ def _status_value_for_index(idx: int, use_french: bool): # 4) Query records via SQL (?sql parameter)) print("Query (SQL via ?sql query parameter):") try: - import time pause("Execute SQL Query") def _run_query(): diff --git a/examples/advanced/pandas_integration.py b/examples/advanced/pandas_integration.py index fdd3a86..6c6c88b 100644 --- a/examples/advanced/pandas_integration.py +++ b/examples/advanced/pandas_integration.py @@ -201,8 +201,6 @@ def backoff_retry(op, *, delays=(0, 2, 5, 10, 20), retry_http_statuses=(400, 403 # 4) Query records via SQL (Web API ?sql=) print("(Pandas) Query (SQL via Web API ?sql=):") try: - import time - def _run_query(): id_key = f"{logical}id" cols = f"{id_key}, {attr_prefix}_code, {attr_prefix}_amount, {attr_prefix}_when" diff --git a/src/PowerPlatform/Dataverse/core/config.py b/src/PowerPlatform/Dataverse/core/config.py index 2b74134..1f557a0 100644 --- a/src/PowerPlatform/Dataverse/core/config.py +++ b/src/PowerPlatform/Dataverse/core/config.py @@ -14,19 +14,28 @@ class DataverseConfig: :param language_code: LCID (Locale ID) for localized labels and messages. Default is 1033 (English - United States). :type language_code: int - :param http_retries: Optional maximum number of retry attempts for transient HTTP errors. Reserved for future use. + :param http_retries: Maximum number of retry attempts for HTTP requests (default: 5). :type http_retries: int or None - :param http_backoff: Optional backoff multiplier (in seconds) between retry attempts. Reserved for future use. + :param http_backoff: Base delay in seconds for exponential backoff (default: 0.5). :type http_backoff: float or None - :param http_timeout: Optional request timeout in seconds. Reserved for future use. + :param http_max_backoff: Maximum delay between retry attempts in seconds (default: 60.0). + :type http_max_backoff: float or None + :param http_timeout: Request timeout in seconds (default: method-dependent). :type http_timeout: float or None + :param http_jitter: Whether to add jitter to retry delays to prevent thundering herd (default: True). + :type http_jitter: bool or None + :param http_retry_transient_errors: Whether to retry transient HTTP errors like 429, 502, 503, 504 (default: True). + :type http_retry_transient_errors: bool or None """ language_code: int = 1033 - # Optional HTTP tuning (not yet wired everywhere; reserved for future use) + # HTTP retry and resilience configuration http_retries: Optional[int] = None http_backoff: Optional[float] = None + http_max_backoff: Optional[float] = None http_timeout: Optional[float] = None + http_jitter: Optional[bool] = None + http_retry_transient_errors: Optional[bool] = None @classmethod def from_env(cls) -> "DataverseConfig": @@ -36,10 +45,13 @@ def from_env(cls) -> "DataverseConfig": :return: Configuration instance with default values. :rtype: ~PowerPlatform.Dataverse.core.config.DataverseConfig """ - # Environment-free defaults + # Environment-free defaults with enhanced retry configuration return cls( language_code=1033, - http_retries=None, - http_backoff=None, - http_timeout=None, + http_retries=None, # Will default to 5 in HttpClient + http_backoff=None, # Will default to 0.5 in HttpClient + http_max_backoff=None, # Will default to 60.0 in HttpClient + http_timeout=None, # Will use method-dependent defaults in HttpClient + http_jitter=None, # Will default to True in HttpClient + http_retry_transient_errors=None, # Will default to True in HttpClient ) diff --git a/src/PowerPlatform/Dataverse/core/errors.py b/src/PowerPlatform/Dataverse/core/errors.py index 428263f..ea0181e 100644 --- a/src/PowerPlatform/Dataverse/core/errors.py +++ b/src/PowerPlatform/Dataverse/core/errors.py @@ -43,7 +43,7 @@ def __init__( self.details = details or {} self.source = source or "client" self.is_transient = is_transient - self.timestamp = _dt.datetime.utcnow().isoformat() + "Z" + self.timestamp = _dt.datetime.now(_dt.timezone.utc).isoformat() + "Z" def to_dict(self) -> Dict[str, Any]: """ diff --git a/src/PowerPlatform/Dataverse/core/http.py b/src/PowerPlatform/Dataverse/core/http.py index 286a439..40de526 100644 --- a/src/PowerPlatform/Dataverse/core/http.py +++ b/src/PowerPlatform/Dataverse/core/http.py @@ -11,6 +11,7 @@ from __future__ import annotations +import random import time from typing import Any, Optional @@ -38,17 +39,26 @@ def __init__( retries: Optional[int] = None, backoff: Optional[float] = None, timeout: Optional[float] = None, + max_backoff: Optional[float] = None, + jitter: bool = True, + retry_transient_errors: bool = True, ) -> None: self.max_attempts = retries if retries is not None else 5 self.base_delay = backoff if backoff is not None else 0.5 + self.max_backoff = max_backoff if max_backoff is not None else 60.0 self.default_timeout: Optional[float] = timeout + self.jitter = jitter + self.retry_transient_errors = retry_transient_errors + + # Transient HTTP status codes that should be retried + self.transient_status_codes = {429, 502, 503, 504} def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: """ Execute an HTTP request with automatic retry logic and timeout management. Applies default timeouts based on HTTP method (120s for POST/DELETE, 10s for others) - and retries on network errors with exponential backoff. + and retries on transient network errors and HTTP status codes with exponential backoff. :param method: HTTP method (GET, POST, PUT, DELETE, etc.). :type method: str @@ -68,13 +78,117 @@ def request(self, method: str, url: str, **kwargs: Any) -> requests.Response: m = (method or "").lower() kwargs["timeout"] = 120 if m in ("post", "delete") else 10 - # Small backoff retry on network errors only + # Enhanced retry logic with exponential backoff, jitter, and HTTP status retries for attempt in range(self.max_attempts): try: - return requests.request(method, url, **kwargs) + response = requests.request(method, url, **kwargs) + + # Check if we should retry based on HTTP status code + if (self.retry_transient_errors and + response.status_code in self.transient_status_codes and + attempt < self.max_attempts - 1): + + delay = self._calculate_retry_delay(attempt, response) + time.sleep(delay) + continue + + return response + except requests.exceptions.RequestException: if attempt == self.max_attempts - 1: raise - delay = self.base_delay * (2 ** attempt) + delay = self._calculate_retry_delay(attempt) time.sleep(delay) - continue + + # This should never be reached due to the logic above + raise RuntimeError("Unexpected end of retry loop") + + def _calculate_retry_delay(self, attempt: int, response: Optional[requests.Response] = None) -> float: + """ + Calculate the delay before the next retry attempt using exponential backoff with jitter. + + This method implements an intelligent retry delay strategy that prioritizes server-provided + guidance (Retry-After header) over client-calculated delays, uses exponential backoff to + reduce load on failing services, and applies jitter to prevent thundering herd problems + when multiple clients retry simultaneously. + + The delay calculation follows this priority order: + 1. **Retry-After header**: If present and valid, use the server-specified delay (capped at max_backoff) + 2. **Exponential backoff**: Calculate base_delay * (2^attempt), capped at max_backoff + 3. **Jitter**: If enabled, add ±25% random variation to prevent synchronized retries + + :param attempt: Zero-based retry attempt number (0 = first retry, 1 = second retry, etc.). + :type attempt: int + :param response: Optional HTTP response object containing headers. Used to extract Retry-After + header when available. If None, only exponential backoff calculation is performed. + :type response: requests.Response or None + + :return: Delay in seconds before the next retry attempt. Always >= 0, capped at max_backoff. + :rtype: float + + :raises: Does not raise exceptions. Invalid Retry-After values fall back to exponential backoff. + + .. note:: + **Retry-After Header Handling (RFC 7231):** + + - Supports integer seconds format: ``Retry-After: 120`` + - Invalid formats (non-integer, HTTP-date) fall back to exponential backoff + - Server-provided delays are capped at ``max_backoff`` to prevent excessive waits + + .. note:: + **Exponential Backoff Formula:** + + - Base calculation: ``delay = base_delay * (2^attempt)`` + - Example with ``base_delay=0.5``: 0.5s, 1.0s, 2.0s, 4.0s, 8.0s, 16.0s... + - Always capped at ``max_backoff`` (default 60s) to prevent unbounded delays + + .. note:: + **Jitter Application:** + + - When ``jitter=True``, adds random variation of ±25% of calculated delay + - Prevents thundering herd when multiple clients retry simultaneously + - Example: 4.0s delay becomes random value between 3.0s and 5.0s + - Final result is always >= 0 even with negative jitter + + Example: + Calculate delays for successive retry attempts:: + + client = HttpClient(backoff=1.0, max_backoff=30.0, jitter=True) + + # Exponential backoff examples (without jitter for predictable values) + client_no_jitter = HttpClient(backoff=0.5, jitter=False) + + # Attempt 0: 0.5s (0.5 * 2^0) + delay0 = client_no_jitter._calculate_retry_delay(0) + + # Attempt 2: 2.0s (0.5 * 2^2) + delay2 = client_no_jitter._calculate_retry_delay(2) + + # With jitter enabled, delays vary randomly within ±25% + # Attempt 1 with jitter: ~1.0s ± 0.25s = 0.75s - 1.25s + delay_with_jitter = client._calculate_retry_delay(1) + """ + + # Check for Retry-After header (RFC 7231) + if response and "Retry-After" in response.headers: + try: + retry_after = int(response.headers["Retry-After"]) + # Respect Retry-After but cap it at max_backoff + return min(retry_after, self.max_backoff) + except (ValueError, TypeError): + # If Retry-After is not a valid integer, fall back to exponential backoff + pass + + # Exponential backoff: base_delay * (2^attempt) + delay = self.base_delay * (2 ** attempt) + + # Cap the delay at max_backoff + delay = min(delay, self.max_backoff) + + # Add jitter to prevent thundering herd (±25% of delay) + if self.jitter: + jitter_range = delay * 0.25 + jitter_offset = random.uniform(-jitter_range, jitter_range) + delay = max(0, delay + jitter_offset) + + return delay diff --git a/src/PowerPlatform/Dataverse/data/odata.py b/src/PowerPlatform/Dataverse/data/odata.py index c2880bd..ed627ab 100644 --- a/src/PowerPlatform/Dataverse/data/odata.py +++ b/src/PowerPlatform/Dataverse/data/odata.py @@ -47,7 +47,10 @@ def __init__( self._http = HttpClient( retries=self.config.http_retries, backoff=self.config.http_backoff, + max_backoff=getattr(self.config, 'http_max_backoff', None), timeout=self.config.http_timeout, + jitter=getattr(self.config, 'http_jitter', True), + retry_transient_errors=getattr(self.config, 'http_retry_transient_errors', True), ) # Cache: logical name -> entity set name (plural) resolved from metadata self._logical_to_entityset_cache: dict[str, str] = {} @@ -903,6 +906,35 @@ def _normalize_picklist_label(self, label: str) -> str: norm = re.sub(r"\s+", " ", norm).strip().lower() return norm + def _request_metadata_with_retry(self, method: str, url: str, **kwargs) -> Any: + """ + Make HTTP request with metadata-specific retry logic. + + Retries 404 errors for metadata operations since attribute metadata + may not be immediately available after creation or schema changes. + + :param method: HTTP method (e.g., 'get', 'post'). + :type method: str + :param url: Target URL for the metadata request. + :type url: str + :param kwargs: Additional arguments passed to the underlying request method. + :return: HTTP response object. + :rtype: Any + :raises HttpError: If the request fails after all retry attempts. + """ + last_error = None + for attempt in range(3): # 3 attempts for metadata operations + try: + return self._request(method, url, **kwargs) + except HttpError as err: + last_error = err + if getattr(err, "status_code", None) == 404 and attempt < 2: + # Exponential backoff: 0.4s, 0.8s for metadata propagation delays + delay = 0.4 * (2 ** attempt) + time.sleep(delay) + continue + raise + def _optionset_map(self, logical_name: str, attr_logical: str) -> Optional[Dict[str, int]]: """Build or return cached mapping of normalized label -> value for a picklist attribute. @@ -930,23 +962,14 @@ def _optionset_map(self, logical_name: str, attr_logical: str) -> Optional[Dict[ f"?$filter=LogicalName eq '{attr_esc}'&$select=LogicalName,AttributeType" ) # Retry up to 3 times on 404 (new or not-yet-published attribute metadata). If still 404, raise. - r_type = None - for attempt in range(3): - try: - r_type = self._request("get", url_type) - break - except HttpError as err: - if getattr(err, "status_code", None) == 404: - if attempt < 2: - # Exponential-ish backoff: 0.4s, 0.8s - time.sleep(0.4 * (2 ** attempt)) - continue - raise RuntimeError( - f"Picklist attribute metadata not found after retries: entity='{logical_name}' attribute='{attr_logical}' (404)" - ) from err - raise - if r_type is None: - raise RuntimeError("Failed to retrieve attribute metadata due to repeated request failures.") + try: + r_type = self._request_metadata_with_retry("get", url_type) + except HttpError as err: + if getattr(err, "status_code", None) == 404: + raise RuntimeError( + f"Picklist attribute metadata not found after retries: entity='{logical_name}' attribute='{attr_logical}' (404)" + ) from err + raise body_type = r_type.json() items = body_type.get("value", []) if isinstance(body_type, dict) else [] @@ -964,22 +987,14 @@ def _optionset_map(self, logical_name: str, attr_logical: str) -> Optional[Dict[ "Microsoft.Dynamics.CRM.PicklistAttributeMetadata?$select=LogicalName&$expand=OptionSet($select=Options)" ) # Step 2 fetch with retries: expanded OptionSet (cast form first) - r_opts = None - for attempt in range(3): - try: - r_opts = self._request("get", cast_url) - break - except HttpError as err: - if getattr(err, "status_code", None) == 404: - if attempt < 2: - time.sleep(0.4 * (2 ** attempt)) # 0.4s, 0.8s - continue - raise RuntimeError( - f"Picklist OptionSet metadata not found after retries: entity='{logical_name}' attribute='{attr_logical}' (404)" - ) from err - raise - if r_opts is None: - raise RuntimeError("Failed to retrieve picklist OptionSet metadata due to repeated request failures.") + try: + r_opts = self._request_metadata_with_retry("get", cast_url) + except HttpError as err: + if getattr(err, "status_code", None) == 404: + raise RuntimeError( + f"Picklist OptionSet metadata not found after retries: entity='{logical_name}' attribute='{attr_logical}' (404)" + ) from err + raise attr_full = {} try: diff --git a/src/PowerPlatform/Dataverse/utils/pandas_adapter.py b/src/PowerPlatform/Dataverse/utils/pandas_adapter.py index fcd398d..aef7a57 100644 --- a/src/PowerPlatform/Dataverse/utils/pandas_adapter.py +++ b/src/PowerPlatform/Dataverse/utils/pandas_adapter.py @@ -84,7 +84,7 @@ def update(self, logical_name: str, record_id: str, entity_data: pd.Series) -> N raise TypeError("entity_data must be a pandas Series") payload = {k: v for k, v in entity_data.items()} if not payload: - return # nothing to send + return None # nothing to send self._c.update(logical_name, record_id, payload) # ---------------------------- Delete --------------------------------- diff --git a/tests/unit/core/test_http_errors.py b/tests/unit/core/test_http_errors.py index e1fffa9..f2379e7 100644 --- a/tests/unit/core/test_http_errors.py +++ b/tests/unit/core/test_http_errors.py @@ -8,8 +8,8 @@ class DummyAuth: def acquire_token(self, scope): - class T: access_token = "x" - return T() + class MockToken: access_token = "x" + return MockToken() class DummyHTTP: def __init__(self, responses): @@ -18,9 +18,9 @@ def request(self, method, url, **kwargs): if not self._responses: raise AssertionError("No more responses") status, headers, body = self._responses.pop(0) - class R: + class MockResponse: pass - r = R() + r = MockResponse() r.status_code = status r.headers = headers if isinstance(body, dict): @@ -34,7 +34,7 @@ def json_fail(): raise ValueError("non-json") r.json = json_fail return r -class TestClient(ODataClient): +class MockHttpClient(ODataClient): def __init__(self, responses): super().__init__(DummyAuth(), "https://org.example", None) self._http = DummyHTTP(responses) @@ -47,7 +47,7 @@ def test_http_404_subcode_and_service_code(): {"x-ms-correlation-request-id": "cid1"}, {"error": {"code": "0x800404", "message": "Not found"}}, )] - c = TestClient(responses) + c = MockHttpClient(responses) with pytest.raises(HttpError) as ei: c._request("get", c.api + "/accounts(abc)") err = ei.value.to_dict() @@ -61,7 +61,7 @@ def test_http_429_transient_and_retry_after(): {"Retry-After": "7"}, {"error": {"message": "Throttle"}}, )] - c = TestClient(responses) + c = MockHttpClient(responses) with pytest.raises(HttpError) as ei: c._request("get", c.api + "/accounts") err = ei.value.to_dict() @@ -76,7 +76,7 @@ def test_http_500_body_excerpt(): {}, "Internal failure XYZ stack truncated", )] - c = TestClient(responses) + c = MockHttpClient(responses) with pytest.raises(HttpError) as ei: c._request("get", c.api + "/accounts") err = ei.value.to_dict() @@ -90,7 +90,7 @@ def test_http_non_mapped_status_code_subcode_fallback(): {}, {"error": {"message": "Teapot"}}, )] - c = TestClient(responses) + c = MockHttpClient(responses) with pytest.raises(HttpError) as ei: c._request("get", c.api + "/accounts") err = ei.value.to_dict() diff --git a/tests/unit/core/test_http_retry_logic.py b/tests/unit/core/test_http_retry_logic.py new file mode 100644 index 0000000..fdeedd4 --- /dev/null +++ b/tests/unit/core/test_http_retry_logic.py @@ -0,0 +1,271 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +from unittest.mock import Mock, patch +import pytest +import requests + +from PowerPlatform.Dataverse.core.http import HttpClient + + +class TestHttpClientRetryLogic: + """Test comprehensive retry logic in HttpClient.""" + + def test_default_configuration(self): + """Test that HttpClient uses proper defaults.""" + client = HttpClient() + assert client.max_attempts == 5 + assert client.base_delay == 0.5 + assert client.max_backoff == 60.0 + assert client.jitter is True + assert client.retry_transient_errors is True + assert client.transient_status_codes == {429, 502, 503, 504} + + def test_custom_configuration(self): + """Test HttpClient with custom configuration.""" + client = HttpClient( + retries=3, + backoff=1.0, + max_backoff=30.0, + jitter=False, + retry_transient_errors=False + ) + assert client.max_attempts == 3 + assert client.base_delay == 1.0 + assert client.max_backoff == 30.0 + assert client.jitter is False + assert client.retry_transient_errors is False + + @patch('requests.request') + def test_successful_request_no_retry(self, mock_request): + """Test that successful requests don't trigger retries.""" + mock_response = Mock() + mock_response.status_code = 200 + mock_request.return_value = mock_response + + client = HttpClient() + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_request.call_count == 1 + + @patch('requests.request') + @patch('time.sleep') + def test_network_error_retry(self, mock_sleep, mock_request): + """Test retry on network errors (requests.exceptions.RequestException).""" + # First two calls fail, third succeeds + mock_request.side_effect = [ + requests.exceptions.ConnectionError("Network error"), + requests.exceptions.ConnectionError("Network error"), + Mock(status_code=200) + ] + + client = HttpClient(jitter=False) # Disable jitter for predictable testing + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_request.call_count == 3 + assert mock_sleep.call_count == 2 + # Check exponential backoff: 0.5, 1.0 + mock_sleep.assert_any_call(0.5) + mock_sleep.assert_any_call(1.0) + + @patch('requests.request') + @patch('time.sleep') + def test_transient_http_error_retry(self, mock_sleep, mock_request): + """Test retry on transient HTTP status codes.""" + # First call returns 429, second call succeeds + mock_429_response = Mock(status_code=429, headers={}) + mock_200_response = Mock(status_code=200, headers={}) + mock_request.side_effect = [mock_429_response, mock_200_response] + + client = HttpClient(jitter=False) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_request.call_count == 2 + assert mock_sleep.call_count == 1 + mock_sleep.assert_called_with(0.5) # Base delay + + @patch('requests.request') + @patch('time.sleep') + def test_retry_after_header_respected(self, mock_sleep, mock_request): + """Test that Retry-After header is respected for 429 responses.""" + mock_429_response = Mock(status_code=429, headers={"Retry-After": "5"}) + mock_200_response = Mock(status_code=200, headers={}) + mock_request.side_effect = [mock_429_response, mock_200_response] + + client = HttpClient(jitter=False) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_request.call_count == 2 + assert mock_sleep.call_count == 1 + mock_sleep.assert_called_with(5) # Retry-After value + + @patch('requests.request') + @patch('time.sleep') + def test_retry_after_header_capped_at_max_backoff(self, mock_sleep, mock_request): + """Test that Retry-After header is capped at max_backoff.""" + mock_429_response = Mock(status_code=429, headers={"Retry-After": "120"}) # 2 minutes + mock_200_response = Mock(status_code=200, headers={}) + mock_request.side_effect = [mock_429_response, mock_200_response] + + client = HttpClient(jitter=False, max_backoff=30.0) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_sleep.call_count == 1 + mock_sleep.assert_called_with(30.0) # Capped at max_backoff + + @patch('requests.request') + @patch('time.sleep') + def test_invalid_retry_after_header_fallback(self, mock_sleep, mock_request): + """Test fallback to exponential backoff when Retry-After is invalid.""" + mock_429_response = Mock(status_code=429, headers={"Retry-After": "invalid"}) + mock_200_response = Mock(status_code=200, headers={}) + mock_request.side_effect = [mock_429_response, mock_200_response] + + client = HttpClient(jitter=False) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_sleep.call_count == 1 + mock_sleep.assert_called_with(0.5) # Falls back to exponential backoff + + @patch('requests.request') + def test_non_transient_error_no_retry(self, mock_request): + """Test that non-transient HTTP errors are not retried.""" + mock_404_response = Mock(status_code=404, headers={}) + mock_request.return_value = mock_404_response + + client = HttpClient() + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 404 + assert mock_request.call_count == 1 + + @patch('requests.request') + @patch('time.sleep') + def test_retry_disabled_for_transient_errors(self, mock_sleep, mock_request): + """Test that transient error retry can be disabled.""" + mock_429_response = Mock(status_code=429, headers={}) + mock_request.return_value = mock_429_response + + client = HttpClient(retry_transient_errors=False) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 429 + assert mock_request.call_count == 1 + assert mock_sleep.call_count == 0 + + @patch('requests.request') + @patch('time.sleep') + def test_max_attempts_respected(self, mock_sleep, mock_request): + """Test that max_attempts is respected.""" + mock_request.side_effect = requests.exceptions.ConnectionError("Network error") + + client = HttpClient(retries=2, jitter=False) + + with pytest.raises(requests.exceptions.ConnectionError): + client.request("GET", "https://test.example.com") + + assert mock_request.call_count == 2 # max_attempts + assert mock_sleep.call_count == 1 # One retry + + @patch('requests.request') + @patch('time.sleep') + def test_exponential_backoff_capped(self, mock_sleep, mock_request): + """Test that exponential backoff is capped at max_backoff.""" + mock_request.side_effect = [ + requests.exceptions.ConnectionError("Network error"), + requests.exceptions.ConnectionError("Network error"), + requests.exceptions.ConnectionError("Network error"), + Mock(status_code=200) + ] + + client = HttpClient(retries=4, backoff=10.0, max_backoff=15.0, jitter=False) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_sleep.call_count == 3 + # Delays should be: 10.0, 15.0 (capped), 15.0 (capped) + calls = mock_sleep.call_args_list + assert calls[0][0][0] == 10.0 # 10.0 * (2^0) + assert calls[1][0][0] == 15.0 # 10.0 * (2^1) = 20.0, capped at 15.0 + assert calls[2][0][0] == 15.0 # 10.0 * (2^2) = 40.0, capped at 15.0 + + @patch('requests.request') + @patch('time.sleep') + @patch('random.uniform') + def test_jitter_applied(self, mock_uniform, mock_sleep, mock_request): + """Test that jitter is applied when enabled.""" + mock_uniform.return_value = 0.1 # Fixed jitter value for testing + mock_request.side_effect = [ + requests.exceptions.ConnectionError("Network error"), + Mock(status_code=200) + ] + + client = HttpClient(jitter=True, backoff=1.0) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_uniform.called + # Jitter should be ±25% of delay, so for 1.0s delay: ±0.25 + mock_uniform.assert_called_with(-0.25, 0.25) + # Final delay should be 1.0 + 0.1 = 1.1 + mock_sleep.assert_called_with(1.1) + + @patch('requests.request') + @patch('time.sleep') + def test_method_specific_timeouts(self, mock_sleep, mock_request): + """Test that method-specific default timeouts are applied.""" + mock_request.return_value = Mock(status_code=200) + + client = HttpClient() + + # Test GET request (should get 10s timeout) + client.request("GET", "https://test.example.com") + args, kwargs = mock_request.call_args + assert kwargs["timeout"] == 10 + + # Test POST request (should get 120s timeout) + client.request("POST", "https://test.example.com") + args, kwargs = mock_request.call_args + assert kwargs["timeout"] == 120 + + # Test DELETE request (should get 120s timeout) + client.request("DELETE", "https://test.example.com") + args, kwargs = mock_request.call_args + assert kwargs["timeout"] == 120 + + @patch('requests.request') + def test_custom_timeout_respected(self, mock_request): + """Test that custom timeout overrides defaults.""" + mock_request.return_value = Mock(status_code=200) + + client = HttpClient(timeout=30.0) + client.request("GET", "https://test.example.com") + + args, kwargs = mock_request.call_args + assert kwargs["timeout"] == 30.0 + + @patch('requests.request') + @patch('time.sleep') + def test_all_transient_status_codes_retried(self, mock_sleep, mock_request): + """Test that all transient status codes are retried.""" + for status_code in [429, 502, 503, 504]: + mock_sleep.reset_mock() + mock_request.reset_mock() + + mock_error_response = Mock(status_code=status_code, headers={}) + mock_success_response = Mock(status_code=200, headers={}) + mock_request.side_effect = [mock_error_response, mock_success_response] + + client = HttpClient(jitter=False) + response = client.request("GET", "https://test.example.com") + + assert response.status_code == 200 + assert mock_request.call_count == 2 + assert mock_sleep.call_count == 1 + mock_sleep.assert_called_with(0.5) # Base delay \ No newline at end of file diff --git a/tests/unit/data/test_enum_optionset_payload.py b/tests/unit/data/test_enum_optionset_payload.py index 09d0212..3500334 100644 --- a/tests/unit/data/test_enum_optionset_payload.py +++ b/tests/unit/data/test_enum_optionset_payload.py @@ -8,9 +8,9 @@ class DummyAuth: def acquire_token(self, scope): # pragma: no cover - simple stub - class T: + class MockToken: access_token = "token" - return T() + return MockToken() class DummyConfig: """Minimal config stub providing attributes ODataClient.__init__ expects.""" diff --git a/tests/unit/data/test_logical_crud.py b/tests/unit/data/test_logical_crud.py index 78280d0..71a9905 100644 --- a/tests/unit/data/test_logical_crud.py +++ b/tests/unit/data/test_logical_crud.py @@ -8,8 +8,8 @@ class DummyAuth: def acquire_token(self, scope): - class T: access_token = "x" - return T() + class MockToken: access_token = "x" + return MockToken() class DummyHTTPClient: def __init__(self, responses): @@ -34,7 +34,7 @@ def json_func(): resp.json = json_func return resp -class TestableClient(ODataClient): +class MockLogicalCrudClient(ODataClient): def __init__(self, responses): super().__init__(DummyAuth(), "https://org.example", None) self._http = DummyHTTPClient(responses) @@ -76,7 +76,7 @@ def test_single_create_update_delete_get(): (204, {}, {}), # update (no body) (204, {}, {}), # delete ] - c = TestableClient(responses) + c = MockLogicalCrudClient(responses) entity_set = c._entity_set_from_logical("account") rid = c._create(entity_set, "account", {"name": "Acme"}) assert rid == guid @@ -96,7 +96,7 @@ def test_bulk_create_and_update(): (204, {}, {}), # UpdateMultiple broadcast (204, {}, {}), # UpdateMultiple 1:1 ] - c = TestableClient(responses) + c = MockLogicalCrudClient(responses) entity_set = c._entity_set_from_logical("account") ids = c._create_multiple(entity_set, "account", [{"name": "A"}, {"name": "B"}]) assert ids == [g1, g2] @@ -111,7 +111,7 @@ def test_get_multiple_paging(): (200, {}, {"value": [{"accountid": "1"}], "@odata.nextLink": "https://org.example/api/data/v9.2/accounts?$skip=1"}), (200, {}, {"value": [{"accountid": "2"}]}), ] - c = TestableClient(responses) + c = MockLogicalCrudClient(responses) pages = list(c._get_multiple("account", select=["accountid"], page_size=1)) assert pages == [[{"accountid": "1"}], [{"accountid": "2"}]] @@ -120,6 +120,6 @@ def test_unknown_logical_name_raises(): responses = [ (200, {}, {"value": []}), # metadata lookup returns empty ] - c = TestableClient(responses) + c = MockLogicalCrudClient(responses) with pytest.raises(MetadataError): - c._entity_set_from_logical("nonexistent") \ No newline at end of file + c._entity_set_from_logical("nonexistent") diff --git a/tests/unit/data/test_metadata_retry_logic.py b/tests/unit/data/test_metadata_retry_logic.py new file mode 100644 index 0000000..e455a32 --- /dev/null +++ b/tests/unit/data/test_metadata_retry_logic.py @@ -0,0 +1,126 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +from unittest.mock import Mock, patch +import pytest + +from PowerPlatform.Dataverse.core.errors import HttpError +from PowerPlatform.Dataverse.data.odata import ODataClient + + +class DummyAuth: + def acquire_token(self, scope): + class MockToken: + access_token = "test_token" + return MockToken() + + +class TestMetadataRetryLogic: + """Test metadata-specific retry logic in ODataClient.""" + + def setup_method(self): + """Set up test client.""" + self.auth = DummyAuth() + self.base_url = "https://test.example.com" + self.client = ODataClient(self.auth, self.base_url) + + @patch.object(ODataClient, '_request') + def test_metadata_retry_success_first_attempt(self, mock_request): + """Test successful metadata request on first attempt.""" + mock_response = Mock() + mock_response.json.return_value = {"test": "data"} + mock_request.return_value = mock_response + + response = self.client._request_metadata_with_retry("GET", "https://test.url") + + assert response == mock_response + assert mock_request.call_count == 1 + + @patch('time.sleep') + @patch.object(ODataClient, '_request') + def test_metadata_retry_404_then_success(self, mock_request, mock_sleep): + """Test retry on 404 error then success.""" + # First call raises 404, second succeeds + http_error = HttpError("Not found", status_code=404) + mock_response = Mock() + mock_request.side_effect = [http_error, mock_response] + + response = self.client._request_metadata_with_retry("GET", "https://test.url") + + assert response == mock_response + assert mock_request.call_count == 2 + assert mock_sleep.call_count == 1 + mock_sleep.assert_called_with(0.4) # First retry delay + + @patch('time.sleep') + @patch.object(ODataClient, '_request') + def test_metadata_retry_multiple_404s_then_success(self, mock_request, mock_sleep): + """Test multiple 404 retries then success.""" + # Two 404s, then success + http_error = HttpError("Not found", status_code=404) + mock_response = Mock() + mock_request.side_effect = [http_error, http_error, mock_response] + + response = self.client._request_metadata_with_retry("GET", "https://test.url") + + assert response == mock_response + assert mock_request.call_count == 3 + assert mock_sleep.call_count == 2 + # Check exponential backoff: 0.4s, 0.8s + assert mock_sleep.call_args_list[0][0][0] == 0.4 + assert mock_sleep.call_args_list[1][0][0] == 0.8 + + @patch('time.sleep') + @patch.object(ODataClient, '_request') + def test_metadata_retry_exhausted_404s(self, mock_request, mock_sleep): + """Test that 404 retries are exhausted and error is raised.""" + # All three attempts return 404 + http_error = HttpError("Not found", status_code=404) + mock_request.side_effect = [http_error, http_error, http_error] + + with pytest.raises(HttpError) as exc_info: + self.client._request_metadata_with_retry("GET", "https://test.url") + + assert exc_info.value.status_code == 404 + assert mock_request.call_count == 3 + assert mock_sleep.call_count == 2 # Two retries + + @patch.object(ODataClient, '_request') + def test_metadata_retry_non_404_error_no_retry(self, mock_request): + """Test that non-404 errors are not retried.""" + # 500 error should not be retried in metadata operations + http_error = HttpError("Server error", status_code=500) + mock_request.side_effect = [http_error] + + with pytest.raises(HttpError) as exc_info: + self.client._request_metadata_with_retry("GET", "https://test.url") + + assert exc_info.value.status_code == 500 + assert mock_request.call_count == 1 # No retry for non-404 + + @patch.object(ODataClient, '_request_metadata_with_retry') + def test_optionset_map_uses_metadata_retry(self, mock_metadata_retry): + """Test that _optionset_map calls the metadata retry method.""" + # First call should succeed, but we're testing that the method is called + http_error = HttpError("Not found", status_code=404) + mock_metadata_retry.side_effect = [http_error] + + # This should raise RuntimeError after retries, but proves retry method was called + with pytest.raises(RuntimeError) as exc_info: + self.client._optionset_map("test_entity", "test_attribute") + + assert "Picklist attribute metadata not found after retries" in str(exc_info.value) + assert mock_metadata_retry.call_count == 1 # Should call our retry method + + @patch.object(ODataClient, '_request_metadata_with_retry') + def test_optionset_map_handles_metadata_404_gracefully(self, mock_metadata_retry): + """Test that _optionset_map handles metadata 404s gracefully.""" + # Simulate 404 after retries + http_error = HttpError("Not found", status_code=404) + mock_metadata_retry.side_effect = [http_error] + + with pytest.raises(RuntimeError) as exc_info: + self.client._optionset_map("test_entity", "test_attribute") + + assert "Picklist attribute metadata not found after retries" in str(exc_info.value) + assert mock_metadata_retry.call_count == 1 \ No newline at end of file diff --git a/tests/unit/data/test_sql_parse.py b/tests/unit/data/test_sql_parse.py index 1b3fb7d..26d8f44 100644 --- a/tests/unit/data/test_sql_parse.py +++ b/tests/unit/data/test_sql_parse.py @@ -6,8 +6,8 @@ class DummyAuth: def acquire_token(self, scope): - class T: access_token = "x" # no real token needed for parsing tests - return T() + class MockToken: access_token = "x" # no real token needed for parsing tests + return MockToken() def _client(): return ODataClient(DummyAuth(), "https://org.example", None)