From 68fe4fba92930687c639523f536aa55d1c3bde3e Mon Sep 17 00:00:00 2001 From: yaythomas Date: Thu, 27 Nov 2025 11:15:23 -0800 Subject: [PATCH 1/3] fix: set retry defaults and jitter Fix retry strategy to only apply default error patterns when neither retryable_errors nor retryable_error_types is specified, matching the behavior of the JS/TS SDK. Previously, specifying only one filter would still apply defaults, causing unintended retry behavior. Also fix critical jitter calculation bug where jitter was being added to base delay instead of replacing it, effectively doubling delays. - Change RetryStrategyConfig fields to None defaults - Apply default patterns only when both filters are None - Rename compute_jitter to apply_jitter for clarity - Fix apply_jitter to return final delay not additive amount - Update test expectations to match corrected jitter behavior - Add missing math import to waits.py --- .../config.py | 20 +- .../retries.py | 39 +- src/aws_durable_execution_sdk_python/waits.py | 12 +- tests/retries_test.py | 824 +++++++++++------- tests/waits_test.py | 20 +- 5 files changed, 549 insertions(+), 366 deletions(-) diff --git a/src/aws_durable_execution_sdk_python/config.py b/src/aws_durable_execution_sdk_python/config.py index 42a5d42..548b6c1 100644 --- a/src/aws_durable_execution_sdk_python/config.py +++ b/src/aws_durable_execution_sdk_python/config.py @@ -464,23 +464,35 @@ class JitterStrategy(StrEnum): Jitter is meant to be used to spread operations across time. + Based on AWS Architecture Blog: https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/ + members: :NONE: No jitter; use the exact calculated delay :FULL: Full jitter; random delay between 0 and calculated delay - :HALF: Half jitter; random delay between 0.5x and 1.0x of the calculated delay + :HALF: Equal jitter; random delay between 0.5x and 1.0x of the calculated delay """ NONE = "NONE" FULL = "FULL" HALF = "HALF" - def compute_jitter(self, delay) -> float: + def apply_jitter(self, delay: float) -> float: + """Apply jitter to a delay value and return the final delay. + + Args: + delay: The base delay value to apply jitter to + + Returns: + The final delay after applying jitter strategy + """ match self: case JitterStrategy.NONE: - return 0 + return delay case JitterStrategy.HALF: - return delay * (random.random() * 0.5 + 0.5) # noqa: S311 + # Equal jitter: delay/2 + random(0, delay/2) + return delay / 2 + random.random() * (delay / 2) # noqa: S311 case _: # default is FULL + # Full jitter: random(0, delay) return random.random() * delay # noqa: S311 diff --git a/src/aws_durable_execution_sdk_python/retries.py b/src/aws_durable_execution_sdk_python/retries.py index 643fa3e..749b6e6 100644 --- a/src/aws_durable_execution_sdk_python/retries.py +++ b/src/aws_durable_execution_sdk_python/retries.py @@ -47,10 +47,8 @@ class RetryStrategyConfig: ) # 5 minutes backoff_rate: Numeric = 2.0 jitter_strategy: JitterStrategy = field(default=JitterStrategy.FULL) - retryable_errors: list[str | re.Pattern] = field( - default_factory=lambda: [re.compile(r".*")] - ) - retryable_error_types: list[type[Exception]] = field(default_factory=list) + retryable_errors: list[str | re.Pattern] | None = None + retryable_error_types: list[type[Exception]] | None = None @property def initial_delay_seconds(self) -> int: @@ -64,42 +62,55 @@ def max_delay_seconds(self) -> int: def create_retry_strategy( - config: RetryStrategyConfig, + config: RetryStrategyConfig | None = None, ) -> Callable[[Exception, int], RetryDecision]: if config is None: config = RetryStrategyConfig() + # Apply default retryableErrors only if user didn't specify either filter + should_use_default_errors: bool = ( + config.retryable_errors is None and config.retryable_error_types is None + ) + + retryable_errors: list[str | re.Pattern] = ( + config.retryable_errors + if config.retryable_errors is not None + else ([re.compile(r".*")] if should_use_default_errors else []) + ) + retryable_error_types: list[type[Exception]] = config.retryable_error_types or [] + def retry_strategy(error: Exception, attempts_made: int) -> RetryDecision: # Check if we've exceeded max attempts if attempts_made >= config.max_attempts: return RetryDecision.no_retry() # Check if error is retryable based on error message - is_retryable_error_message = any( + is_retryable_error_message: bool = any( pattern.search(str(error)) if isinstance(pattern, re.Pattern) else pattern in str(error) - for pattern in config.retryable_errors + for pattern in retryable_errors ) # Check if error is retryable based on error type - is_retryable_error_type = any( - isinstance(error, error_type) for error_type in config.retryable_error_types + is_retryable_error_type: bool = any( + isinstance(error, error_type) for error_type in retryable_error_types ) if not is_retryable_error_message and not is_retryable_error_type: return RetryDecision.no_retry() # Calculate delay with exponential backoff - delay = min( + base_delay: float = min( config.initial_delay_seconds * (config.backoff_rate ** (attempts_made - 1)), config.max_delay_seconds, ) - delay_with_jitter = delay + config.jitter_strategy.compute_jitter(delay) - delay_with_jitter = math.ceil(delay_with_jitter) - final_delay = max(1, delay_with_jitter) + # Apply jitter to get final delay + delay_with_jitter: float = config.jitter_strategy.apply_jitter(base_delay) + # Round up and ensure minimum of 1 second + final_delay: int = max(1, math.ceil(delay_with_jitter)) - return RetryDecision.retry(Duration(seconds=round(final_delay))) + return RetryDecision.retry(Duration(seconds=final_delay)) return retry_strategy diff --git a/src/aws_durable_execution_sdk_python/waits.py b/src/aws_durable_execution_sdk_python/waits.py index e205c29..b4d740a 100644 --- a/src/aws_durable_execution_sdk_python/waits.py +++ b/src/aws_durable_execution_sdk_python/waits.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from dataclasses import dataclass, field from typing import TYPE_CHECKING, Generic @@ -81,17 +82,16 @@ def wait_strategy(result: T, attempts_made: int) -> WaitDecision: return WaitDecision.no_wait() # Calculate delay with exponential backoff - base_delay = min( + base_delay: float = min( config.initial_delay_seconds * (config.backoff_rate ** (attempts_made - 1)), config.max_delay_seconds, ) - # Apply jitter (add jitter to base delay) - jitter = config.jitter_strategy.compute_jitter(base_delay) - delay_with_jitter = base_delay + jitter + # Apply jitter to get final delay + delay_with_jitter: float = config.jitter_strategy.apply_jitter(base_delay) - # Ensure delay is an integer >= 1 - final_delay = max(1, round(delay_with_jitter)) + # Round up and ensure minimum of 1 second + final_delay: int = max(1, math.ceil(delay_with_jitter)) return WaitDecision.wait(Duration(seconds=final_delay)) diff --git a/tests/retries_test.py b/tests/retries_test.py index 9e97a45..1b58134 100644 --- a/tests/retries_test.py +++ b/tests/retries_test.py @@ -14,403 +14,563 @@ create_retry_strategy, ) +# region Jitter Strategy Tests -class TestJitterStrategy: - """Test jitter strategy implementations.""" - - def test_none_jitter_returns_zero(self): - """Test NONE jitter always returns 0.""" - strategy = JitterStrategy.NONE - assert strategy.compute_jitter(10) == 0 - assert strategy.compute_jitter(100) == 0 - - @patch("random.random") - def test_full_jitter_range(self, mock_random): - """Test FULL jitter returns value between 0 and delay.""" - mock_random.return_value = 0.5 - strategy = JitterStrategy.FULL - delay = 10 - result = strategy.compute_jitter(delay) - assert result == 5.0 # 0.5 * 10 - - @patch("random.random") - def test_half_jitter_range(self, mock_random): - """Test HALF jitter returns value between 0.5 and 1.0 (multiplier).""" - mock_random.return_value = 0.5 - strategy = JitterStrategy.HALF - result = strategy.compute_jitter(10) - assert result == 7.5 # 10 * (0.5 + 0.5*0.5) - - @patch("random.random") - def test_half_jitter_boundary_values(self, mock_random): - """Test HALF jitter boundary values.""" - strategy = JitterStrategy.HALF - - # Minimum value (random = 0) - mock_random.return_value = 0.0 - result = strategy.compute_jitter(100) - assert result == 50 - - # Maximum value (random = 1) - mock_random.return_value = 1.0 - result = strategy.compute_jitter(100) - assert result == 100 - - def test_invalid_jitter_strategy(self): - """Test behavior with invalid jitter strategy.""" - # Create an invalid enum value by bypassing normal construction - invalid_strategy = "INVALID" - - # This should raise an exception or return None - with pytest.raises((ValueError, AttributeError)): - JitterStrategy(invalid_strategy).compute_jitter(10) - - -class TestRetryDecision: - """Test RetryDecision factory methods.""" - - def test_retry_factory(self): - """Test retry factory method.""" - decision = RetryDecision.retry(Duration.from_seconds(30)) - assert decision.should_retry is True - assert decision.delay_seconds == 30 - - def test_no_retry_factory(self): - """Test no_retry factory method.""" - decision = RetryDecision.no_retry() - assert decision.should_retry is False - assert decision.delay_seconds == 0 - - -class TestRetryStrategyConfig: - """Test RetryStrategyConfig defaults and behavior.""" - - def test_default_config(self): - """Test default configuration values.""" - config = RetryStrategyConfig() - assert config.max_attempts == 3 - assert config.initial_delay_seconds == 5 - assert config.max_delay_seconds == 300 - assert config.backoff_rate == 2.0 - assert config.jitter_strategy == JitterStrategy.FULL - assert len(config.retryable_errors) == 1 - assert config.retryable_error_types == [] - - -class TestCreateRetryStrategy: - """Test retry strategy creation and behavior.""" - - def test_max_attempts_exceeded(self): - """Test strategy returns no_retry when max attempts exceeded.""" - config = RetryStrategyConfig(max_attempts=2) - strategy = create_retry_strategy(config) - error = Exception("test error") - decision = strategy(error, 2) - assert decision.should_retry is False +def test_none_jitter_returns_delay(): + """Test NONE jitter returns the original delay unchanged.""" + strategy = JitterStrategy.NONE + assert strategy.apply_jitter(10) == 10 + assert strategy.apply_jitter(100) == 100 - def test_retryable_error_message_string(self): - """Test retry based on error message string match.""" - config = RetryStrategyConfig(retryable_errors=["timeout"]) - strategy = create_retry_strategy(config) - error = Exception("connection timeout") - decision = strategy(error, 1) - assert decision.should_retry is True +@patch("random.random") +def test_full_jitter_range(mock_random): + """Test FULL jitter returns value between 0 and delay.""" + mock_random.return_value = 0.5 + strategy = JitterStrategy.FULL + delay = 10 + result = strategy.apply_jitter(delay) + assert result == 5.0 # 0.5 * 10 - def test_retryable_error_message_regex(self): - """Test retry based on error message regex match.""" - config = RetryStrategyConfig(retryable_errors=[re.compile(r"timeout|error")]) - strategy = create_retry_strategy(config) - error = Exception("network timeout occurred") - decision = strategy(error, 1) - assert decision.should_retry is True +@patch("random.random") +def test_half_jitter_range(mock_random): + """Test HALF jitter returns value between delay/2 and delay.""" + mock_random.return_value = 0.5 + strategy = JitterStrategy.HALF + result = strategy.apply_jitter(10) + assert result == 7.5 # 10/2 + 0.5 * (10/2) = 5 + 2.5 - def test_retryable_error_type(self): - """Test retry based on error type.""" - config = RetryStrategyConfig(retryable_error_types=[ValueError]) - strategy = create_retry_strategy(config) - error = ValueError("invalid value") - decision = strategy(error, 1) - assert decision.should_retry is True +@patch("random.random") +def test_half_jitter_boundary_values(mock_random): + """Test HALF jitter boundary values.""" + strategy = JitterStrategy.HALF - def test_non_retryable_error(self): - """Test no retry for non-retryable error.""" - config = RetryStrategyConfig(retryable_errors=["timeout"]) - strategy = create_retry_strategy(config) + # Minimum value (random = 0): delay/2 + 0 = delay/2 + mock_random.return_value = 0.0 + result = strategy.apply_jitter(100) + assert result == 50 - error = Exception("permission denied") - decision = strategy(error, 1) - assert decision.should_retry is False + # Maximum value (random = 1): delay/2 + delay/2 = delay + mock_random.return_value = 1.0 + result = strategy.apply_jitter(100) + assert result == 100 - @patch("random.random") - def test_exponential_backoff_calculation(self, mock_random): - """Test exponential backoff delay calculation.""" - mock_random.return_value = 0.5 - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(2), - backoff_rate=2.0, - jitter_strategy=JitterStrategy.FULL, - ) - strategy = create_retry_strategy(config) - error = Exception("test error") +def test_invalid_jitter_strategy(): + """Test behavior with invalid jitter strategy.""" + # Create an invalid enum value by bypassing normal construction + invalid_strategy = "INVALID" - # First attempt: 2 * (2^0) = 2, jitter adds 1, total = 3 - decision = strategy(error, 1) - assert decision.delay_seconds == 3 + # This should raise an exception or return None + with pytest.raises((ValueError, AttributeError)): + JitterStrategy(invalid_strategy).apply_jitter(10) - # Second attempt: 2 * (2^1) = 4, jitter adds 2, total = 6 - decision = strategy(error, 2) - assert decision.delay_seconds == 6 - def test_max_delay_cap(self): - """Test delay is capped at max_delay_seconds.""" - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(100), - max_delay=Duration.from_seconds(50), - backoff_rate=2.0, - jitter_strategy=JitterStrategy.NONE, - ) - strategy = create_retry_strategy(config) +# endregion + + +# region Retry Decision Tests + + +def test_retry_factory(): + """Test retry factory method.""" + decision = RetryDecision.retry(Duration.from_seconds(30)) + assert decision.should_retry is True + assert decision.delay_seconds == 30 + + +def test_no_retry_factory(): + """Test no_retry factory method.""" + decision = RetryDecision.no_retry() + assert decision.should_retry is False + assert decision.delay_seconds == 0 + + +# endregion + + +# region Retry Strategy Config Tests + + +def test_default_config(): + """Test default configuration values.""" + config = RetryStrategyConfig() + assert config.max_attempts == 3 + assert config.initial_delay_seconds == 5 + assert config.max_delay_seconds == 300 + assert config.backoff_rate == 2.0 + assert config.jitter_strategy == JitterStrategy.FULL + assert config.retryable_errors is None + assert config.retryable_error_types is None + + +# endregion + + +# region Create Retry Strategy Tests + + +def test_max_attempts_exceeded(): + """Test strategy returns no_retry when max attempts exceeded.""" + config = RetryStrategyConfig(max_attempts=2) + strategy = create_retry_strategy(config) + + error = Exception("test error") + decision = strategy(error, 2) + assert decision.should_retry is False + + +def test_retryable_error_message_string(): + """Test retry based on error message string match.""" + config = RetryStrategyConfig(retryable_errors=["timeout"]) + strategy = create_retry_strategy(config) + + error = Exception("connection timeout") + decision = strategy(error, 1) + assert decision.should_retry is True - error = Exception("test error") - decision = strategy(error, 2) # Would be 200 without cap - assert decision.delay_seconds == 50 - def test_minimum_delay_one_second(self): - """Test delay is at least 1 second.""" +def test_retryable_error_message_regex(): + """Test retry based on error message regex match.""" + config = RetryStrategyConfig(retryable_errors=[re.compile(r"timeout|error")]) + strategy = create_retry_strategy(config) + + error = Exception("network timeout occurred") + decision = strategy(error, 1) + assert decision.should_retry is True + + +def test_retryable_error_type(): + """Test retry based on error type.""" + config = RetryStrategyConfig(retryable_error_types=[ValueError]) + strategy = create_retry_strategy(config) + + error = ValueError("invalid value") + decision = strategy(error, 1) + assert decision.should_retry is True + + +def test_non_retryable_error(): + """Test no retry for non-retryable error.""" + config = RetryStrategyConfig(retryable_errors=["timeout"]) + strategy = create_retry_strategy(config) + + error = Exception("permission denied") + decision = strategy(error, 1) + assert decision.should_retry is False + + +@patch("random.random") +def test_exponential_backoff_calculation(mock_random): + """Test exponential backoff delay calculation with jitter.""" + mock_random.return_value = 0.5 + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(2), + backoff_rate=2.0, + jitter_strategy=JitterStrategy.FULL, + ) + strategy = create_retry_strategy(config) + + error = Exception("test error") + + # First attempt: base = 2 * (2^0) = 2, full jitter = 0.5 * 2 = 1 + decision = strategy(error, 1) + assert decision.delay_seconds == 1 + + # Second attempt: base = 2 * (2^1) = 4, full jitter = 0.5 * 4 = 2 + decision = strategy(error, 2) + assert decision.delay_seconds == 2 + + +def test_max_delay_cap(): + """Test delay is capped at max_delay_seconds.""" + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(100), + max_delay=Duration.from_seconds(50), + backoff_rate=2.0, + jitter_strategy=JitterStrategy.NONE, + ) + strategy = create_retry_strategy(config) + + error = Exception("test error") + decision = strategy(error, 2) # Would be 200 without cap + assert decision.delay_seconds == 50 + + +def test_minimum_delay_one_second(): + """Test delay is at least 1 second.""" + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(0), jitter_strategy=JitterStrategy.NONE + ) + strategy = create_retry_strategy(config) + + error = Exception("test error") + decision = strategy(error, 1) + assert decision.delay_seconds == 1 + + +def test_delay_ceiling_applied(): + """Test delay is rounded up using math.ceil.""" + with patch("random.random", return_value=0.3): config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(0), jitter_strategy=JitterStrategy.NONE + initial_delay=Duration.from_seconds(3), + jitter_strategy=JitterStrategy.FULL, ) strategy = create_retry_strategy(config) error = Exception("test error") decision = strategy(error, 1) + # base = 3, full jitter = 0.3 * 3 = 0.9, ceil(0.9) = 1 assert decision.delay_seconds == 1 - def test_delay_ceiling_applied(self): - """Test delay is rounded up using math.ceil.""" - with patch("random.random", return_value=0.3): - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(3), - jitter_strategy=JitterStrategy.FULL, - ) - strategy = create_retry_strategy(config) - error = Exception("test error") - decision = strategy(error, 1) - # 3 + (0.3 * 3) = 3.9, ceil(3.9) = 4 - assert decision.delay_seconds == 4 +# endregion -class TestRetryPresets: - """Test predefined retry presets.""" +# region Retry Presets Tests - def test_none_preset(self): - """Test none preset allows no retries.""" - strategy = RetryPresets.none() - error = Exception("test error") - decision = strategy(error, 1) - assert decision.should_retry is False +def test_none_preset(): + """Test none preset allows no retries.""" + strategy = RetryPresets.none() + error = Exception("test error") - def test_default_preset_config(self): - """Test default preset configuration.""" - strategy = RetryPresets.default() - error = Exception("test error") + decision = strategy(error, 1) + assert decision.should_retry is False - # Should retry within max attempts - decision = strategy(error, 1) - assert decision.should_retry is True - # Should not retry after max attempts - decision = strategy(error, 6) - assert decision.should_retry is False +def test_default_preset_config(): + """Test default preset configuration.""" + strategy = RetryPresets.default() + error = Exception("test error") - def test_transient_preset_config(self): - """Test transient preset configuration.""" - strategy = RetryPresets.transient() - error = Exception("test error") + # Should retry within max attempts + decision = strategy(error, 1) + assert decision.should_retry is True - # Should retry within max attempts - decision = strategy(error, 1) - assert decision.should_retry is True + # Should not retry after max attempts + decision = strategy(error, 6) + assert decision.should_retry is False - # Should not retry after max attempts - decision = strategy(error, 3) - assert decision.should_retry is False - def test_resource_availability_preset(self): - """Test resource availability preset allows longer retries.""" - strategy = RetryPresets.resource_availability() - error = Exception("test error") +def test_transient_preset_config(): + """Test transient preset configuration.""" + strategy = RetryPresets.transient() + error = Exception("test error") - # Should retry within max attempts - decision = strategy(error, 1) - assert decision.should_retry is True + # Should retry within max attempts + decision = strategy(error, 1) + assert decision.should_retry is True - # Should not retry after max attempts - decision = strategy(error, 5) - assert decision.should_retry is False + # Should not retry after max attempts + decision = strategy(error, 3) + assert decision.should_retry is False - def test_critical_preset_config(self): - """Test critical preset allows many retries.""" - strategy = RetryPresets.critical() - error = Exception("test error") - # Should retry within max attempts - decision = strategy(error, 5) - assert decision.should_retry is True +def test_resource_availability_preset(): + """Test resource availability preset allows longer retries.""" + strategy = RetryPresets.resource_availability() + error = Exception("test error") - # Should not retry after max attempts - decision = strategy(error, 10) - assert decision.should_retry is False + # Should retry within max attempts + decision = strategy(error, 1) + assert decision.should_retry is True - @patch("random.random") - def test_critical_preset_no_jitter(self, mock_random): - """Test critical preset uses no jitter.""" - mock_random.return_value = 0.5 # Should be ignored - strategy = RetryPresets.critical() - error = Exception("test error") + # Should not retry after max attempts + decision = strategy(error, 5) + assert decision.should_retry is False - decision = strategy(error, 1) - # With no jitter: 1 * (1.5^0) = 1 - assert decision.delay_seconds == 1 +def test_critical_preset_config(): + """Test critical preset allows many retries.""" + strategy = RetryPresets.critical() + error = Exception("test error") -class TestJitterIntegration: - """Test jitter integration with retry strategies.""" + # Should retry within max attempts + decision = strategy(error, 5) + assert decision.should_retry is True - @patch("random.random") - def test_full_jitter_integration(self, mock_random): - """Test full jitter integration in retry strategy.""" - mock_random.return_value = 0.8 - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.FULL - ) - strategy = create_retry_strategy(config) + # Should not retry after max attempts + decision = strategy(error, 10) + assert decision.should_retry is False - error = Exception("test error") - decision = strategy(error, 1) - # 10 + (0.8 * 10) = 18 - assert decision.delay_seconds == 18 - @patch("random.random") - def test_half_jitter_integration(self, mock_random): - """Test half jitter integration in retry strategy.""" - mock_random.return_value = 0.6 - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.HALF - ) - strategy = create_retry_strategy(config) +@patch("random.random") +def test_critical_preset_no_jitter(mock_random): + """Test critical preset uses no jitter.""" + mock_random.return_value = 0.5 # Should be ignored + strategy = RetryPresets.critical() + error = Exception("test error") - error = Exception("test error") - decision = strategy(error, 1) - # 10 + 10*(0.6 * 0.5 + 0.5) = 18 - assert decision.delay_seconds == 18 + decision = strategy(error, 1) + # With no jitter: 1 * (1.5^0) = 1 + assert decision.delay_seconds == 1 - @patch("random.random") - def test_half_jitter_integration_corrected(self, mock_random): - """Test half jitter with corrected understanding of implementation.""" - mock_random.return_value = 0.0 # Minimum jitter - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.HALF - ) - strategy = create_retry_strategy(config) - error = Exception("test error") - decision = strategy(error, 1) - # 10 + 10 * 0.5 = 15 - assert decision.delay_seconds == 15 +# endregion - def test_none_jitter_integration(self): - """Test no jitter integration in retry strategy.""" - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.NONE - ) - strategy = create_retry_strategy(config) - error = Exception("test error") - decision = strategy(error, 1) - assert decision.delay_seconds == 10 +# region Jitter Integration Tests -class TestEdgeCases: - """Test edge cases and error conditions.""" +@patch("random.random") +def test_full_jitter_integration(mock_random): + """Test full jitter integration in retry strategy.""" + mock_random.return_value = 0.8 + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.FULL + ) + strategy = create_retry_strategy(config) - def test_none_config(self): - """Test behavior when config is None.""" - strategy = create_retry_strategy(None) - error = Exception("test error") - decision = strategy(error, 1) - assert decision.should_retry is True - assert decision.delay_seconds >= 1 + error = Exception("test error") + decision = strategy(error, 1) + # base = 10, full jitter = 0.8 * 10 = 8 + assert decision.delay_seconds == 8 - def test_zero_backoff_rate(self): - """Test behavior with zero backoff rate.""" - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(5), - backoff_rate=0, - jitter_strategy=JitterStrategy.NONE, - ) - strategy = create_retry_strategy(config) - error = Exception("test error") - decision = strategy(error, 1) - # 5 * (0^0) = 5 * 1 = 5 - assert decision.delay_seconds == 5 +@patch("random.random") +def test_half_jitter_integration(mock_random): + """Test half jitter integration in retry strategy.""" + mock_random.return_value = 0.6 + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.HALF + ) + strategy = create_retry_strategy(config) - def test_fractional_backoff_rate(self): - """Test behavior with fractional backoff rate.""" - config = RetryStrategyConfig( - initial_delay=Duration.from_seconds(8), - backoff_rate=0.5, - jitter_strategy=JitterStrategy.NONE, - ) - strategy = create_retry_strategy(config) + error = Exception("test error") + decision = strategy(error, 1) + # base = 10, half jitter = 10/2 + 0.6 * (10/2) = 5 + 3 = 8 + assert decision.delay_seconds == 8 - error = Exception("test error") - decision = strategy(error, 2) - # 8 * (0.5^1) = 4 - assert decision.delay_seconds == 4 - def test_empty_retryable_errors_list(self): - """Test behavior with empty retryable errors list.""" - config = RetryStrategyConfig(retryable_errors=[]) - strategy = create_retry_strategy(config) +@patch("random.random") +def test_half_jitter_integration_corrected(mock_random): + """Test half jitter with minimum random value.""" + mock_random.return_value = 0.0 # Minimum jitter + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.HALF + ) + strategy = create_retry_strategy(config) - error = Exception("test error") - decision = strategy(error, 1) - assert decision.should_retry is False + error = Exception("test error") + decision = strategy(error, 1) + # base = 10, half jitter = 10/2 + 0.0 * (10/2) = 5 + assert decision.delay_seconds == 5 - def test_multiple_error_patterns(self): - """Test multiple error patterns matching.""" - config = RetryStrategyConfig( - retryable_errors=["timeout", re.compile(r"network.*error")] - ) - strategy = create_retry_strategy(config) - # Test string match - error1 = Exception("connection timeout") - decision1 = strategy(error1, 1) - assert decision1.should_retry is True +def test_none_jitter_integration(): + """Test no jitter integration in retry strategy.""" + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(10), jitter_strategy=JitterStrategy.NONE + ) + strategy = create_retry_strategy(config) - # Test regex match - error2 = Exception("network connection error") - decision2 = strategy(error2, 1) - assert decision2.should_retry is True + error = Exception("test error") + decision = strategy(error, 1) + assert decision.delay_seconds == 10 - def test_mixed_error_types_and_patterns(self): - """Test combination of error types and patterns.""" - config = RetryStrategyConfig( - retryable_errors=["timeout"], retryable_error_types=[ValueError] - ) - strategy = create_retry_strategy(config) - # Should retry on ValueError even without message match - error = ValueError("some value error") - decision = strategy(error, 1) - assert decision.should_retry is True +# endregion + + +# region Default Behavior Tests + + +def test_no_filters_retries_all_errors(): + """Test that when neither filter is specified, all errors are retried.""" + config = RetryStrategyConfig() + strategy = create_retry_strategy(config) + + # Should retry any error + error1 = Exception("any error message") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + error2 = ValueError("different error type") + decision2 = strategy(error2, 1) + assert decision2.should_retry is True + + +def test_only_retryable_errors_specified(): + """Test that when only retryable_errors is specified, only matching messages are retried.""" + config = RetryStrategyConfig(retryable_errors=["timeout"]) + strategy = create_retry_strategy(config) + + # Should retry matching error + error1 = Exception("connection timeout") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + # Should NOT retry non-matching error + error2 = Exception("permission denied") + decision2 = strategy(error2, 1) + assert decision2.should_retry is False + + +def test_only_retryable_error_types_specified(): + """Test that when only retryable_error_types is specified, only matching types are retried.""" + config = RetryStrategyConfig(retryable_error_types=[ValueError, TypeError]) + strategy = create_retry_strategy(config) + + # Should retry matching type + error1 = ValueError("invalid value") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + error2 = TypeError("type error") + decision2 = strategy(error2, 1) + assert decision2.should_retry is True + + # Should NOT retry non-matching type (even though message might match default pattern) + error3 = Exception("some error") + decision3 = strategy(error3, 1) + assert decision3.should_retry is False + + +def test_both_filters_specified_or_logic(): + """Test that when both filters are specified, errors matching either are retried (OR logic).""" + config = RetryStrategyConfig( + retryable_errors=["timeout"], retryable_error_types=[ValueError] + ) + strategy = create_retry_strategy(config) + + # Should retry on message match + error1 = Exception("connection timeout") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + # Should retry on type match + error2 = ValueError("some value error") + decision2 = strategy(error2, 1) + assert decision2.should_retry is True + + # Should NOT retry when neither matches + error3 = RuntimeError("runtime error") + decision3 = strategy(error3, 1) + assert decision3.should_retry is False + + +def test_empty_retryable_errors_with_types(): + """Test that empty retryable_errors list with types specified only retries matching types.""" + config = RetryStrategyConfig( + retryable_errors=[], retryable_error_types=[ValueError] + ) + strategy = create_retry_strategy(config) + + # Should retry matching type + error1 = ValueError("value error") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + # Should NOT retry non-matching type + error2 = Exception("some error") + decision2 = strategy(error2, 1) + assert decision2.should_retry is False + + +def test_empty_retryable_error_types_with_errors(): + """Test that empty retryable_error_types list with errors specified only retries matching messages.""" + config = RetryStrategyConfig(retryable_errors=["timeout"], retryable_error_types=[]) + strategy = create_retry_strategy(config) + + # Should retry matching message + error1 = Exception("connection timeout") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + # Should NOT retry non-matching message + error2 = Exception("permission denied") + decision2 = strategy(error2, 1) + assert decision2.should_retry is False + + +# endregion + + +# region Edge Cases Tests + + +def test_none_config(): + """Test behavior when config is None.""" + strategy = create_retry_strategy(None) + error = Exception("test error") + decision = strategy(error, 1) + assert decision.should_retry is True + assert decision.delay_seconds >= 1 + + +def test_zero_backoff_rate(): + """Test behavior with zero backoff rate.""" + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(5), + backoff_rate=0, + jitter_strategy=JitterStrategy.NONE, + ) + strategy = create_retry_strategy(config) + + error = Exception("test error") + decision = strategy(error, 1) + # 5 * (0^0) = 5 * 1 = 5 + assert decision.delay_seconds == 5 + + +def test_fractional_backoff_rate(): + """Test behavior with fractional backoff rate.""" + config = RetryStrategyConfig( + initial_delay=Duration.from_seconds(8), + backoff_rate=0.5, + jitter_strategy=JitterStrategy.NONE, + ) + strategy = create_retry_strategy(config) + + error = Exception("test error") + decision = strategy(error, 2) + # 8 * (0.5^1) = 4 + assert decision.delay_seconds == 4 + + +def test_empty_retryable_errors_list(): + """Test behavior with empty retryable errors list.""" + config = RetryStrategyConfig(retryable_errors=[]) + strategy = create_retry_strategy(config) + + error = Exception("test error") + decision = strategy(error, 1) + assert decision.should_retry is False + + +def test_multiple_error_patterns(): + """Test multiple error patterns matching.""" + config = RetryStrategyConfig( + retryable_errors=["timeout", re.compile(r"network.*error")] + ) + strategy = create_retry_strategy(config) + + # Test string match + error1 = Exception("connection timeout") + decision1 = strategy(error1, 1) + assert decision1.should_retry is True + + # Test regex match + error2 = Exception("network connection error") + decision2 = strategy(error2, 1) + assert decision2.should_retry is True + + +def test_mixed_error_types_and_patterns(): + """Test combination of error types and patterns.""" + config = RetryStrategyConfig( + retryable_errors=["timeout"], retryable_error_types=[ValueError] + ) + strategy = create_retry_strategy(config) + + # Should retry on ValueError even without message match + error = ValueError("some value error") + decision = strategy(error, 1) + assert decision.should_retry is True + + +# endregion diff --git a/tests/waits_test.py b/tests/waits_test.py index 4066e43..06267d8 100644 --- a/tests/waits_test.py +++ b/tests/waits_test.py @@ -105,13 +105,13 @@ def test_exponential_backoff_calculation(self, mock_random): result = "pending" - # First attempt: 2 * (2^0) = 2, jitter adds 1, total = 3 + # First attempt: 2 * (2^0) = 2, FULL jitter with 0.5 = 0.5 * 2 = 1 decision = strategy(result, 1) - assert decision.delay_seconds == 3 + assert decision.delay_seconds == 1 - # Second attempt: 2 * (2^1) = 4, jitter adds 2, total = 6 + # Second attempt: 2 * (2^1) = 4, FULL jitter with 0.5 = 0.5 * 4 = 2 decision = strategy(result, 2) - assert decision.delay_seconds == 6 + assert decision.delay_seconds == 2 def test_max_delay_cap(self): """Test delay is capped at max_delay_seconds.""" @@ -154,8 +154,8 @@ def test_full_jitter_integration(self, mock_random): result = "pending" decision = strategy(result, 1) - # 10 + (0.8 * 10) = 18 - assert decision.delay_seconds == 18 + # FULL jitter: 0.8 * 10 = 8 + assert decision.delay_seconds == 8 @patch("random.random") def test_half_jitter_integration(self, mock_random): @@ -170,8 +170,8 @@ def test_half_jitter_integration(self, mock_random): result = "pending" decision = strategy(result, 1) - # base: 10, jitter: 10 * (0.5 + 0.0 * 0.5) = 5, total: 10 + 5 = 15 - assert decision.delay_seconds == 15 + # HALF jitter: 10/2 + 0.0 * (10/2) = 5 + assert decision.delay_seconds == 5 def test_none_jitter_integration(self): """Test no jitter integration in wait strategy.""" @@ -330,8 +330,8 @@ def test_rounding_behavior(self, mock_random): result = "pending" decision = strategy(result, 1) - # 3 + (0.3 * 3) = 3.9, round(3.9) = 4 - assert decision.delay_seconds == 4 + # FULL jitter: 0.3 * 3 = 0.9, ceil(0.9) = 1 + assert decision.delay_seconds == 1 class TestWaitForConditionConfig: From 469e2868a06eaa029a8c03957c423bf0af611343 Mon Sep 17 00:00:00 2001 From: yaythomas Date: Thu, 27 Nov 2025 11:25:12 -0800 Subject: [PATCH 2/3] fix: pin hatch version Prevent ruff fmt errors since experimental is on --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32970ae..fbfb30a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install Hatch run: | - python -m pip install --upgrade hatch + python -m pip install hatch==1.15.0 - name: static analysis run: hatch fmt --check - name: type checking From 588bb443d8cf98e30a38f3982fe350124a3664ef Mon Sep 17 00:00:00 2001 From: yaythomas Date: Thu, 27 Nov 2025 11:42:15 -0800 Subject: [PATCH 3/3] feat: cache retry regex --- src/aws_durable_execution_sdk_python/retries.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/aws_durable_execution_sdk_python/retries.py b/src/aws_durable_execution_sdk_python/retries.py index 749b6e6..5a09db2 100644 --- a/src/aws_durable_execution_sdk_python/retries.py +++ b/src/aws_durable_execution_sdk_python/retries.py @@ -14,6 +14,9 @@ Numeric = int | float +# Default pattern that matches all error messages +_DEFAULT_RETRYABLE_ERROR_PATTERN = re.compile(r".*") + @dataclass class RetryDecision: @@ -75,7 +78,7 @@ def create_retry_strategy( retryable_errors: list[str | re.Pattern] = ( config.retryable_errors if config.retryable_errors is not None - else ([re.compile(r".*")] if should_use_default_errors else []) + else ([_DEFAULT_RETRYABLE_ERROR_PATTERN] if should_use_default_errors else []) ) retryable_error_types: list[type[Exception]] = config.retryable_error_types or []