Skip to content

Commit 4e28a5e

Browse files
authored
fix: retries optional not set
* 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 * fix: pin hatch version Prevent ruff fmt errors since experimental is on * feat: cache retry regex
1 parent 94b57cc commit 4e28a5e

File tree

6 files changed

+553
-367
lines changed

6 files changed

+553
-367
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ jobs:
4242
python-version: ${{ matrix.python-version }}
4343
- name: Install Hatch
4444
run: |
45-
python -m pip install --upgrade hatch
45+
python -m pip install hatch==1.15.0
4646
- name: static analysis
4747
run: hatch fmt --check
4848
- name: type checking

src/aws_durable_execution_sdk_python/config.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -464,23 +464,35 @@ class JitterStrategy(StrEnum):
464464
465465
Jitter is meant to be used to spread operations across time.
466466
467+
Based on AWS Architecture Blog: https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/
468+
467469
members:
468470
:NONE: No jitter; use the exact calculated delay
469471
:FULL: Full jitter; random delay between 0 and calculated delay
470-
:HALF: Half jitter; random delay between 0.5x and 1.0x of the calculated delay
472+
:HALF: Equal jitter; random delay between 0.5x and 1.0x of the calculated delay
471473
"""
472474

473475
NONE = "NONE"
474476
FULL = "FULL"
475477
HALF = "HALF"
476478

477-
def compute_jitter(self, delay) -> float:
479+
def apply_jitter(self, delay: float) -> float:
480+
"""Apply jitter to a delay value and return the final delay.
481+
482+
Args:
483+
delay: The base delay value to apply jitter to
484+
485+
Returns:
486+
The final delay after applying jitter strategy
487+
"""
478488
match self:
479489
case JitterStrategy.NONE:
480-
return 0
490+
return delay
481491
case JitterStrategy.HALF:
482-
return delay * (random.random() * 0.5 + 0.5) # noqa: S311
492+
# Equal jitter: delay/2 + random(0, delay/2)
493+
return delay / 2 + random.random() * (delay / 2) # noqa: S311
483494
case _: # default is FULL
495+
# Full jitter: random(0, delay)
484496
return random.random() * delay # noqa: S311
485497

486498

src/aws_durable_execution_sdk_python/retries.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
Numeric = int | float
1616

17+
# Default pattern that matches all error messages
18+
_DEFAULT_RETRYABLE_ERROR_PATTERN = re.compile(r".*")
19+
1720

1821
@dataclass
1922
class RetryDecision:
@@ -47,10 +50,8 @@ class RetryStrategyConfig:
4750
) # 5 minutes
4851
backoff_rate: Numeric = 2.0
4952
jitter_strategy: JitterStrategy = field(default=JitterStrategy.FULL)
50-
retryable_errors: list[str | re.Pattern] = field(
51-
default_factory=lambda: [re.compile(r".*")]
52-
)
53-
retryable_error_types: list[type[Exception]] = field(default_factory=list)
53+
retryable_errors: list[str | re.Pattern] | None = None
54+
retryable_error_types: list[type[Exception]] | None = None
5455

5556
@property
5657
def initial_delay_seconds(self) -> int:
@@ -64,42 +65,55 @@ def max_delay_seconds(self) -> int:
6465

6566

6667
def create_retry_strategy(
67-
config: RetryStrategyConfig,
68+
config: RetryStrategyConfig | None = None,
6869
) -> Callable[[Exception, int], RetryDecision]:
6970
if config is None:
7071
config = RetryStrategyConfig()
7172

73+
# Apply default retryableErrors only if user didn't specify either filter
74+
should_use_default_errors: bool = (
75+
config.retryable_errors is None and config.retryable_error_types is None
76+
)
77+
78+
retryable_errors: list[str | re.Pattern] = (
79+
config.retryable_errors
80+
if config.retryable_errors is not None
81+
else ([_DEFAULT_RETRYABLE_ERROR_PATTERN] if should_use_default_errors else [])
82+
)
83+
retryable_error_types: list[type[Exception]] = config.retryable_error_types or []
84+
7285
def retry_strategy(error: Exception, attempts_made: int) -> RetryDecision:
7386
# Check if we've exceeded max attempts
7487
if attempts_made >= config.max_attempts:
7588
return RetryDecision.no_retry()
7689

7790
# Check if error is retryable based on error message
78-
is_retryable_error_message = any(
91+
is_retryable_error_message: bool = any(
7992
pattern.search(str(error))
8093
if isinstance(pattern, re.Pattern)
8194
else pattern in str(error)
82-
for pattern in config.retryable_errors
95+
for pattern in retryable_errors
8396
)
8497

8598
# Check if error is retryable based on error type
86-
is_retryable_error_type = any(
87-
isinstance(error, error_type) for error_type in config.retryable_error_types
99+
is_retryable_error_type: bool = any(
100+
isinstance(error, error_type) for error_type in retryable_error_types
88101
)
89102

90103
if not is_retryable_error_message and not is_retryable_error_type:
91104
return RetryDecision.no_retry()
92105

93106
# Calculate delay with exponential backoff
94-
delay = min(
107+
base_delay: float = min(
95108
config.initial_delay_seconds * (config.backoff_rate ** (attempts_made - 1)),
96109
config.max_delay_seconds,
97110
)
98-
delay_with_jitter = delay + config.jitter_strategy.compute_jitter(delay)
99-
delay_with_jitter = math.ceil(delay_with_jitter)
100-
final_delay = max(1, delay_with_jitter)
111+
# Apply jitter to get final delay
112+
delay_with_jitter: float = config.jitter_strategy.apply_jitter(base_delay)
113+
# Round up and ensure minimum of 1 second
114+
final_delay: int = max(1, math.ceil(delay_with_jitter))
101115

102-
return RetryDecision.retry(Duration(seconds=round(final_delay)))
116+
return RetryDecision.retry(Duration(seconds=final_delay))
103117

104118
return retry_strategy
105119

src/aws_durable_execution_sdk_python/waits.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import math
56
from dataclasses import dataclass, field
67
from typing import TYPE_CHECKING, Generic
78

@@ -81,17 +82,16 @@ def wait_strategy(result: T, attempts_made: int) -> WaitDecision:
8182
return WaitDecision.no_wait()
8283

8384
# Calculate delay with exponential backoff
84-
base_delay = min(
85+
base_delay: float = min(
8586
config.initial_delay_seconds * (config.backoff_rate ** (attempts_made - 1)),
8687
config.max_delay_seconds,
8788
)
8889

89-
# Apply jitter (add jitter to base delay)
90-
jitter = config.jitter_strategy.compute_jitter(base_delay)
91-
delay_with_jitter = base_delay + jitter
90+
# Apply jitter to get final delay
91+
delay_with_jitter: float = config.jitter_strategy.apply_jitter(base_delay)
9292

93-
# Ensure delay is an integer >= 1
94-
final_delay = max(1, round(delay_with_jitter))
93+
# Round up and ensure minimum of 1 second
94+
final_delay: int = max(1, math.ceil(delay_with_jitter))
9595

9696
return WaitDecision.wait(Duration(seconds=final_delay))
9797

0 commit comments

Comments
 (0)