Skip to content

Commit 84fc260

Browse files
authored
Add timeout error classification (#590)
1 parent 44f500d commit 84fc260

File tree

12 files changed

+105
-35
lines changed

12 files changed

+105
-35
lines changed

codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,8 @@ def __init__(self, request: HTTPRequest):
631631
class $3L:
632632
""\"An asynchronous HTTP client solely for testing purposes.""\"
633633
634+
TIMEOUT_EXCEPTIONS = ()
635+
634636
def __init__(self, *, client_config: HTTPClientConfiguration | None = None):
635637
self._client_config = client_config
636638
@@ -644,6 +646,8 @@ async def send(
644646
class $4L:
645647
""\"An asynchronous HTTP client solely for testing purposes.""\"
646648
649+
TIMEOUT_EXCEPTIONS = ()
650+
647651
def __init__(
648652
self,
649653
*,

packages/smithy-core/src/smithy_core/aio/client.py

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from ..auth import AuthParams
1313
from ..deserializers import DeserializeableShape, ShapeDeserializer
1414
from ..endpoints import EndpointResolverParams
15-
from ..exceptions import RetryError, SmithyError
15+
from ..exceptions import ClientTimeoutError, RetryError, SmithyError
1616
from ..interceptors import (
1717
InputContext,
1818
Interceptor,
@@ -448,24 +448,31 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape](
448448

449449
_LOGGER.debug("Sending request %s", request_context.transport_request)
450450

451-
if request_future is not None:
452-
# If we have an input event stream (or duplex event stream) then we
453-
# need to let the client return ASAP so that it can start sending
454-
# events. So here we start the transport send in a background task
455-
# then set the result of the request future. It's important to sequence
456-
# it just like that so that the client gets a stream that's ready
457-
# to send.
458-
transport_task = asyncio.create_task(
459-
self.transport.send(request=request_context.transport_request)
460-
)
461-
request_future.set_result(request_context)
462-
transport_response = await transport_task
463-
else:
464-
# If we don't have an input stream, there's no point in creating a
465-
# task, so we just immediately await the coroutine.
466-
transport_response = await self.transport.send(
467-
request=request_context.transport_request
468-
)
451+
try:
452+
if request_future is not None:
453+
# If we have an input event stream (or duplex event stream) then we
454+
# need to let the client return ASAP so that it can start sending
455+
# events. So here we start the transport send in a background task
456+
# then set the result of the request future. It's important to sequence
457+
# it just like that so that the client gets a stream that's ready
458+
# to send.
459+
transport_task = asyncio.create_task(
460+
self.transport.send(request=request_context.transport_request)
461+
)
462+
request_future.set_result(request_context)
463+
transport_response = await transport_task
464+
else:
465+
# If we don't have an input stream, there's no point in creating a
466+
# task, so we just immediately await the coroutine.
467+
transport_response = await self.transport.send(
468+
request=request_context.transport_request
469+
)
470+
except Exception as e:
471+
if isinstance(e, self.transport.TIMEOUT_EXCEPTIONS):
472+
raise ClientTimeoutError(
473+
message=f"Client timeout occurred: {e}"
474+
) from e
475+
raise
469476

470477
_LOGGER.debug("Received response: %s", transport_response)
471478

packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ async def resolve_endpoint(self, params: EndpointResolverParams[Any]) -> Endpoin
8686

8787

8888
class ClientTransport[I: Request, O: Response](Protocol):
89-
"""Protocol-agnostic representation of a client tranport (e.g. an HTTP client)."""
89+
"""Protocol-agnostic representation of a client transport (e.g. an HTTP client)."""
90+
91+
TIMEOUT_EXCEPTIONS: tuple[type[Exception], ...]
9092

9193
async def send(self, request: I) -> O:
9294
"""Send a request over the transport and receive the response."""

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ class CallError(SmithyError):
5050
is_throttling_error: bool = False
5151
"""Whether the error is a throttling error."""
5252

53+
is_timeout_error: bool = False
54+
"""Whether the error represents a timeout condition."""
55+
5356
def __post_init__(self):
5457
super().__init__(self.message)
5558

@@ -61,6 +64,20 @@ class ModeledError(CallError):
6164
fault: Fault = "client"
6265

6366

67+
@dataclass(kw_only=True)
68+
class ClientTimeoutError(CallError):
69+
"""Exception raised when a client-side timeout occurs.
70+
71+
This error indicates that the client transport layer encountered a timeout while
72+
attempting to communicate with the server. This typically occurs when network
73+
requests take longer than the configured timeout period.
74+
"""
75+
76+
fault: Fault = "client"
77+
is_timeout_error: bool = True
78+
is_retry_safe: bool | None = True
79+
80+
6481
class SerializationError(SmithyError):
6582
"""Base exception type for exceptions raised during serialization."""
6683

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ class ErrorRetryInfo(Protocol):
2727
is_throttling_error: bool = False
2828
"""Whether the error is a throttling error."""
2929

30+
is_timeout_error: bool = False
31+
"""Whether the error is a timeout error."""
32+
3033

3134
class RetryBackoffStrategy(Protocol):
3235
"""Stateless strategy for computing retry delays based on retry attempt account."""

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,12 @@ def acquire(self, *, error: Exception) -> int:
268268
If there's insufficient capacity available, raise an exception.
269269
Otherwise, return the amount of capacity successfully allocated.
270270
"""
271-
capacity_amount = self.RETRY_COST
271+
272+
is_timeout = (
273+
isinstance(error, retries_interface.ErrorRetryInfo)
274+
and error.is_timeout_error
275+
)
276+
capacity_amount = self.TIMEOUT_RETRY_COST if is_timeout else self.RETRY_COST
272277

273278
with self._lock:
274279
if capacity_amount > self._available_capacity:

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,12 @@ def test_retry_quota_release_caps_at_max(
208208
# Release more than we acquired. Should cap at initial capacity.
209209
retry_quota.release(release_amount=50)
210210
assert retry_quota.available_capacity == 10
211+
212+
213+
def test_retry_quota_acquire_timeout_error(
214+
retry_quota: StandardRetryQuota,
215+
) -> None:
216+
timeout_error = CallError(is_timeout_error=True, is_retry_safe=True)
217+
acquired = retry_quota.acquire(error=timeout_error)
218+
assert acquired == StandardRetryQuota.TIMEOUT_RETRY_COST
219+
assert retry_quota.available_capacity == 0

packages/smithy-http/src/smithy_http/aio/aiohttp.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ def __post_init__(self) -> None:
5252
class AIOHTTPClient(HTTPClient):
5353
"""Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp."""
5454

55+
TIMEOUT_EXCEPTIONS = (TimeoutError,)
56+
5557
def __init__(
5658
self,
5759
*,

packages/smithy-http/src/smithy_http/aio/crt.py

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from inspect import iscoroutinefunction
99
from typing import TYPE_CHECKING, Any
1010

11+
from awscrt.exceptions import AwsCrtError
12+
1113
if TYPE_CHECKING:
1214
# pyright doesn't like optional imports. This is reasonable because if we use these
1315
# in type hints then they'd result in runtime errors.
@@ -129,9 +131,16 @@ def __post_init__(self) -> None:
129131
_assert_crt()
130132

131133

134+
class _CRTTimeoutError(Exception):
135+
"""Internal wrapper for CRT timeout errors."""
136+
137+
132138
class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient):
133139
_HTTP_PORT = 80
134140
_HTTPS_PORT = 443
141+
_TIMEOUT_ERROR_NAMES = frozenset(["AWS_IO_SOCKET_TIMEOUT", "AWS_IO_SOCKET_CLOSED"])
142+
143+
TIMEOUT_EXCEPTIONS = (_CRTTimeoutError,)
135144

136145
def __init__(
137146
self,
@@ -163,18 +172,23 @@ async def send(
163172
:param request: The request including destination URI, fields, payload.
164173
:param request_config: Configuration specific to this request.
165174
"""
166-
crt_request = self._marshal_request(request)
167-
connection = await self._get_connection(request.destination)
175+
try:
176+
crt_request = self._marshal_request(request)
177+
connection = await self._get_connection(request.destination)
168178

169-
# Convert body to async iterator for request_body_generator
170-
body_generator = self._create_body_generator(request.body)
179+
# Convert body to async iterator for request_body_generator
180+
body_generator = self._create_body_generator(request.body)
171181

172-
crt_stream = connection.request(
173-
crt_request,
174-
request_body_generator=body_generator,
175-
)
182+
crt_stream = connection.request(
183+
crt_request,
184+
request_body_generator=body_generator,
185+
)
176186

177-
return await self._await_response(crt_stream)
187+
return await self._await_response(crt_stream)
188+
except AwsCrtError as e:
189+
if e.name in self._TIMEOUT_ERROR_NAMES:
190+
raise _CRTTimeoutError() from e
191+
raise
178192

179193
async def _await_response(
180194
self, stream: "AIOHttpClientStreamUnified"

packages/smithy-http/src/smithy_http/aio/protocols.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ async def _create_error(
215215
)
216216
return error_shape.deserialize(deserializer)
217217

218-
is_throttle = response.status == 429
219218
message = (
220219
f"Unknown error for operation {operation.schema.id} "
221220
f"- status: {response.status}"
@@ -224,11 +223,17 @@ async def _create_error(
224223
message += f" - id: {error_id}"
225224
if response.reason is not None:
226225
message += f" - reason: {response.status}"
226+
227+
is_timeout = response.status == 408
228+
is_throttle = response.status == 429
229+
fault = "client" if response.status < 500 else "server"
230+
227231
return CallError(
228232
message=message,
229-
fault="client" if response.status < 500 else "server",
233+
fault=fault,
230234
is_throttling_error=is_throttle,
231-
is_retry_safe=is_throttle or None,
235+
is_timeout_error=is_timeout,
236+
is_retry_safe=is_throttle or is_timeout or None,
232237
)
233238

234239
def _matches_content_type(self, response: HTTPResponse) -> bool:

0 commit comments

Comments
 (0)