Skip to content

Commit 4028c84

Browse files
Refactor retry interfaces to use exception-based information
1 parent bcc95b1 commit 4028c84

3 files changed

Lines changed: 34 additions & 64 deletions

File tree

packages/smithy-core/src/smithy_core/interfaces/retries.py

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
# SPDX-License-Identifier: Apache-2.0
33
from dataclasses import dataclass
4-
from enum import Enum
54
from typing import Protocol, runtime_checkable
65

76

@@ -29,43 +28,6 @@ class ErrorRetryInfo(Protocol):
2928
"""Whether the error is a throttling error."""
3029

3130

32-
class RetryErrorType(Enum):
33-
"""Classification of errors based on desired retry behavior."""
34-
35-
TRANSIENT = 1
36-
"""A connection level error such as a socket timeout, socket connect error, TLS
37-
negotiation timeout."""
38-
39-
THROTTLING = 2
40-
"""The server explicitly told the client to back off, for example with HTTP status
41-
429 or 503."""
42-
43-
SERVER_ERROR = 3
44-
"""A server error that should be retried and does not match the definition of
45-
``THROTTLING``."""
46-
47-
CLIENT_ERROR = 4
48-
"""Doesn't count against any budgets.
49-
50-
This could be something like a 401 challenge in HTTP.
51-
"""
52-
53-
54-
@dataclass(kw_only=True)
55-
class RetryErrorInfo:
56-
"""Container for information about a retryable error."""
57-
58-
error_type: RetryErrorType
59-
"""Classification of error based on desired retry behavior."""
60-
61-
retry_after_hint: float | None = None
62-
"""Protocol hint for computing the timespan to delay before the next retry.
63-
64-
This could come from HTTP's 'retry-after' header or similar mechanisms in other
65-
protocols.
66-
"""
67-
68-
6931
class RetryBackoffStrategy(Protocol):
7032
"""Stateless strategy for computing retry delays based on retry attempt account."""
7133

@@ -113,7 +75,7 @@ def acquire_initial_retry_token(
11375
...
11476

11577
def refresh_retry_token_for_retry(
116-
self, *, token_to_renew: RetryToken, error_info: RetryErrorInfo
78+
self, *, token_to_renew: RetryToken, error: Exception
11779
) -> RetryToken:
11880
"""Replace an existing retry token from a failed attempt with a new token.
11981
@@ -124,10 +86,7 @@ def refresh_retry_token_for_retry(
12486
the retry attempt and raise the error as exception.
12587
12688
:param token_to_renew: The token used for the previous failed attempt.
127-
128-
:param error_info: If no further retry is allowed, this information is used to
129-
construct the exception.
130-
89+
:param error: The error that triggered the need for a retry.
13190
:raises SmithyRetryException: If no further retry attempts are allowed.
13291
"""
13392
...

packages/smithy-core/src/smithy_core/retries.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
from dataclasses import dataclass
66
from enum import Enum
77

8-
from smithy_core.interfaces.retries import RetryErrorType
9-
108
from .exceptions import SmithyRetryException
119
from .interfaces import retries as retries_interface
1210

@@ -220,21 +218,18 @@ def refresh_retry_token_for_retry(
220218
self,
221219
*,
222220
token_to_renew: retries_interface.RetryToken,
223-
error_info: retries_interface.RetryErrorInfo,
221+
error: Exception,
224222
) -> SimpleRetryToken:
225223
"""Replace an existing retry token from a failed attempt with a new token.
226224
227225
This retry strategy always returns a token until the attempt count stored in
228226
the new token exceeds the ``max_attempts`` value.
229227
230228
:param token_to_renew: The token used for the previous failed attempt.
231-
232-
:param error_info: If no further retry is allowed, this information is used to
233-
construct the exception.
234-
229+
:param error: The error that triggered the need for a retry.
235230
:raises SmithyRetryException: If no further retry attempts are allowed.
236231
"""
237-
if error_info.error_type is not RetryErrorType.CLIENT_ERROR:
232+
if isinstance(error, retries_interface.ErrorRetryInfo) and error.is_retry_safe:
238233
retry_count = token_to_renew.retry_count + 1
239234
if retry_count >= self.max_attempts:
240235
raise SmithyRetryException(
@@ -243,7 +238,7 @@ def refresh_retry_token_for_retry(
243238
retry_delay = self.backoff_strategy.compute_next_backoff_delay(retry_count)
244239
return SimpleRetryToken(retry_count=retry_count, retry_delay=retry_delay)
245240
else:
246-
raise SmithyRetryException(f"Error is not retryable: {error_info}")
241+
raise SmithyRetryException(f"Error is not retryable: {error}")
247242

248243
def record_success(self, *, token: retries_interface.RetryToken) -> None:
249244
"""Not used by this retry strategy."""

packages/smithy-core/tests/unit/test_retries.py

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
# SPDX-License-Identifier: Apache-2.0
33

44
import pytest
5-
from smithy_core.exceptions import SmithyRetryException
6-
from smithy_core.interfaces.retries import RetryErrorInfo, RetryErrorType
5+
from smithy_core.exceptions import CallException, SmithyRetryException
76
from smithy_core.retries import ExponentialBackoffJitterType as EBJT
87
from smithy_core.retries import ExponentialRetryBackoffStrategy, SimpleRetryStrategy
98

@@ -61,26 +60,43 @@ def test_simple_retry_strategy(max_attempts: int) -> None:
6160
backoff_strategy=ExponentialRetryBackoffStrategy(backoff_scale_value=5),
6261
max_attempts=max_attempts,
6362
)
64-
error_info = RetryErrorInfo(error_type=RetryErrorType.THROTTLING)
63+
error = CallException(is_retry_safe=True)
6564
token = strategy.acquire_initial_retry_token()
6665
for _ in range(max_attempts - 1):
6766
token = strategy.refresh_retry_token_for_retry(
68-
token_to_renew=token, error_info=error_info
67+
token_to_renew=token, error=error
6968
)
7069
with pytest.raises(SmithyRetryException):
71-
strategy.refresh_retry_token_for_retry(
72-
token_to_renew=token, error_info=error_info
73-
)
70+
strategy.refresh_retry_token_for_retry(token_to_renew=token, error=error)
7471

7572

76-
def test_simple_retry_strategy_does_not_retry_client_errors() -> None:
73+
def test_simple_retry_does_not_retry_unclassified() -> None:
7774
strategy = SimpleRetryStrategy(
7875
backoff_strategy=ExponentialRetryBackoffStrategy(backoff_scale_value=5),
7976
max_attempts=2,
8077
)
81-
error_info = RetryErrorInfo(error_type=RetryErrorType.CLIENT_ERROR)
8278
token = strategy.acquire_initial_retry_token()
8379
with pytest.raises(SmithyRetryException):
84-
strategy.refresh_retry_token_for_retry(
85-
token_to_renew=token, error_info=error_info
86-
)
80+
strategy.refresh_retry_token_for_retry(token_to_renew=token, error=Exception())
81+
82+
83+
def test_simple_retry_does_not_retry_when_safety_unknown() -> None:
84+
strategy = SimpleRetryStrategy(
85+
backoff_strategy=ExponentialRetryBackoffStrategy(backoff_scale_value=5),
86+
max_attempts=2,
87+
)
88+
error = CallException(is_retry_safe=None)
89+
token = strategy.acquire_initial_retry_token()
90+
with pytest.raises(SmithyRetryException):
91+
strategy.refresh_retry_token_for_retry(token_to_renew=token, error=error)
92+
93+
94+
def test_simple_retry_does_not_retry_unsafe() -> None:
95+
strategy = SimpleRetryStrategy(
96+
backoff_strategy=ExponentialRetryBackoffStrategy(backoff_scale_value=5),
97+
max_attempts=2,
98+
)
99+
error = CallException(fault="client", is_retry_safe=False)
100+
token = strategy.acquire_initial_retry_token()
101+
with pytest.raises(SmithyRetryException):
102+
strategy.refresh_retry_token_for_retry(token_to_renew=token, error=error)

0 commit comments

Comments
 (0)