From 1c8bed7c2174a3a7caaeda43493122510d40f6b2 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Fri, 24 Oct 2025 17:35:41 -0400 Subject: [PATCH 1/9] Add error classification --- .../smithy-core/src/smithy_core/aio/client.py | 46 ++++++++------ .../smithy_core/aio/interfaces/__init__.py | 33 +++++++++- .../smithy-core/src/smithy_core/exceptions.py | 17 +++++ .../src/smithy_http/aio/aiohttp.py | 10 ++- .../smithy-http/src/smithy_http/aio/crt.py | 19 ++++++ .../src/smithy_http/aio/protocols.py | 16 ++++- .../tests/unit/aio/test_protocols.py | 36 +++++++++-- .../tests/unit/aio/test_timeout_errors.py | 63 +++++++++++++++++++ 8 files changed, 210 insertions(+), 30 deletions(-) create mode 100644 packages/smithy-http/tests/unit/aio/test_timeout_errors.py diff --git a/packages/smithy-core/src/smithy_core/aio/client.py b/packages/smithy-core/src/smithy_core/aio/client.py index bf27c440c..b6ba28948 100644 --- a/packages/smithy-core/src/smithy_core/aio/client.py +++ b/packages/smithy-core/src/smithy_core/aio/client.py @@ -12,7 +12,7 @@ from ..auth import AuthParams from ..deserializers import DeserializeableShape, ShapeDeserializer from ..endpoints import EndpointResolverParams -from ..exceptions import RetryError, SmithyError +from ..exceptions import ClientTimeoutError, RetryError, SmithyError from ..interceptors import ( InputContext, Interceptor, @@ -448,24 +448,32 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( _LOGGER.debug("Sending request %s", request_context.transport_request) - if request_future is not None: - # If we have an input event stream (or duplex event stream) then we - # need to let the client return ASAP so that it can start sending - # events. So here we start the transport send in a background task - # then set the result of the request future. It's important to sequence - # it just like that so that the client gets a stream that's ready - # to send. - transport_task = asyncio.create_task( - self.transport.send(request=request_context.transport_request) - ) - request_future.set_result(request_context) - transport_response = await transport_task - else: - # If we don't have an input stream, there's no point in creating a - # task, so we just immediately await the coroutine. - transport_response = await self.transport.send( - request=request_context.transport_request - ) + try: + if request_future is not None: + # If we have an input event stream (or duplex event stream) then we + # need to let the client return ASAP so that it can start sending + # events. So here we start the transport send in a background task + # then set the result of the request future. It's important to sequence + # it just like that so that the client gets a stream that's ready + # to send. + transport_task = asyncio.create_task( + self.transport.send(request=request_context.transport_request) + ) + request_future.set_result(request_context) + transport_response = await transport_task + else: + # If we don't have an input stream, there's no point in creating a + # task, so we just immediately await the coroutine. + transport_response = await self.transport.send( + request=request_context.transport_request + ) + except Exception as e: + error_info = self.transport.get_error_info(e) + if error_info.is_timeout_error: + raise ClientTimeoutError( + message=f"Client timeout occurred: {e}", fault=error_info.fault + ) from e + raise _LOGGER.debug("Received response: %s", transport_response) diff --git a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py index 31d772125..e3f8974be 100644 --- a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py +++ b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py @@ -1,7 +1,8 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 from collections.abc import AsyncIterable, Callable -from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any, Literal, Protocol, runtime_checkable from ...documents import TypeRegistry from ...endpoints import EndpointResolverParams @@ -10,6 +11,18 @@ from ...interfaces import StreamingBlob as SyncStreamingBlob from .eventstream import EventPublisher, EventReceiver + +@dataclass(frozen=True) +class ErrorInfo: + """Information about an error from a transport.""" + + is_timeout_error: bool + """Whether this error represents a timeout condition.""" + + fault: Literal["client", "server"] = "client" + """Whether the client or server is at fault.""" + + if TYPE_CHECKING: from typing_extensions import TypeForm @@ -86,7 +99,23 @@ async def resolve_endpoint(self, params: EndpointResolverParams[Any]) -> Endpoin class ClientTransport[I: Request, O: Response](Protocol): - """Protocol-agnostic representation of a client tranport (e.g. an HTTP client).""" + """Protocol-agnostic representation of a client transport (e.g. an HTTP client). + + Transport implementations must define the get_error_info method to determine which + exceptions represent timeout conditions for that transport. + """ + + def get_error_info(self, exception: Exception, **kwargs) -> ErrorInfo: + """Get information about an exception. + + Args: + exception: The exception to analyze + **kwargs: Additional context for analysis + + Returns: + ErrorInfo with timeout and fault information. + """ + ... async def send(self, request: I) -> O: """Send a request over the transport and receive the response.""" diff --git a/packages/smithy-core/src/smithy_core/exceptions.py b/packages/smithy-core/src/smithy_core/exceptions.py index 0e28bd530..53e4aacb3 100644 --- a/packages/smithy-core/src/smithy_core/exceptions.py +++ b/packages/smithy-core/src/smithy_core/exceptions.py @@ -50,6 +50,9 @@ class CallError(SmithyError): is_throttling_error: bool = False """Whether the error is a throttling error.""" + is_timeout_error: bool = False + """Whether the error represents a timeout condition.""" + def __post_init__(self): super().__init__(self.message) @@ -61,6 +64,20 @@ class ModeledError(CallError): fault: Fault = "client" +@dataclass(kw_only=True) +class ClientTimeoutError(CallError): + """Exception raised when a client-side timeout occurs. + + This error indicates that the client transport layer encountered a timeout while + attempting to communicate with the server. This typically occurs when network + requests take longer than the configured timeout period. + """ + + fault: Fault = "client" + is_timeout_error: bool = True + is_retry_safe: bool = True + + class SerializationError(SmithyError): """Base exception type for exceptions raised during serialization.""" diff --git a/packages/smithy-http/src/smithy_http/aio/aiohttp.py b/packages/smithy-http/src/smithy_http/aio/aiohttp.py index 83f4c191f..d1935ada6 100644 --- a/packages/smithy-http/src/smithy_http/aio/aiohttp.py +++ b/packages/smithy-http/src/smithy_http/aio/aiohttp.py @@ -20,7 +20,7 @@ except ImportError: HAS_AIOHTTP = False # type: ignore -from smithy_core.aio.interfaces import StreamingBlob +from smithy_core.aio.interfaces import ErrorInfo, StreamingBlob from smithy_core.aio.types import AsyncBytesReader from smithy_core.aio.utils import async_list from smithy_core.exceptions import MissingDependencyError @@ -52,6 +52,14 @@ def __post_init__(self) -> None: class AIOHTTPClient(HTTPClient): """Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp.""" + def get_error_info(self, exception: Exception, **kwargs) -> ErrorInfo: + """Get information about aiohttp errors.""" + + if isinstance(exception, TimeoutError): + return ErrorInfo(is_timeout_error=True) + + return ErrorInfo(is_timeout_error=False) + def __init__( self, *, diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index a450ef9c9..39d68c0df 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -8,6 +8,8 @@ from inspect import iscoroutinefunction from typing import TYPE_CHECKING, Any +from awscrt.exceptions import AwsCrtError + if TYPE_CHECKING: # pyright doesn't like optional imports. This is reasonable because if we use these # in type hints then they'd result in runtime errors. @@ -33,6 +35,7 @@ from smithy_core import interfaces as core_interfaces from smithy_core.aio import interfaces as core_aio_interfaces +from smithy_core.aio.interfaces import ErrorInfo from smithy_core.aio.types import AsyncBytesReader from smithy_core.exceptions import MissingDependencyError @@ -133,6 +136,22 @@ class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient): _HTTP_PORT = 80 _HTTPS_PORT = 443 + def get_error_info(self, exception: Exception, **kwargs) -> ErrorInfo: + """Get information about CRT errors.""" + + timeout_indicators = ( + "AWS_IO_SOCKET_TIMEOUT", + "AWS_IO_CHANNEL_ERROR_SOCKET_TIMEOUT", + "AWS_ERROR_HTTP_REQUEST_TIMEOUT", + ) + if isinstance(exception, TimeoutError): + return ErrorInfo(is_timeout_error=True, fault="client") + + if isinstance(exception, AwsCrtError) and exception.name in timeout_indicators: + return ErrorInfo(is_timeout_error=True, fault="client") + + return ErrorInfo(is_timeout_error=False) + def __init__( self, eventloop: _AWSCRTEventLoop | None = None, diff --git a/packages/smithy-http/src/smithy_http/aio/protocols.py b/packages/smithy-http/src/smithy_http/aio/protocols.py index cf25036fe..992f72d35 100644 --- a/packages/smithy-http/src/smithy_http/aio/protocols.py +++ b/packages/smithy-http/src/smithy_http/aio/protocols.py @@ -215,7 +215,6 @@ async def _create_error( ) return error_shape.deserialize(deserializer) - is_throttle = response.status == 429 message = ( f"Unknown error for operation {operation.schema.id} " f"- status: {response.status}" @@ -224,11 +223,22 @@ async def _create_error( message += f" - id: {error_id}" if response.reason is not None: message += f" - reason: {response.status}" + + if response.status == 408: + is_timeout = True + fault = "server" + else: + is_timeout = False + fault = "client" if response.status < 500 else "server" + + is_throttle = response.status == 429 + return CallError( message=message, - fault="client" if response.status < 500 else "server", + fault=fault, is_throttling_error=is_throttle, - is_retry_safe=is_throttle or None, + is_timeout_error=is_timeout, + is_retry_safe=is_throttle or is_timeout or None, ) def _matches_content_type(self, response: HTTPResponse) -> bool: diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index ecdb15cfa..665989865 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -2,23 +2,24 @@ # SPDX-License-Identifier: Apache-2.0 from typing import Any +from unittest.mock import Mock import pytest from smithy_core import URI from smithy_core.documents import TypeRegistry from smithy_core.endpoints import Endpoint -from smithy_core.interfaces import TypedProperties from smithy_core.interfaces import URI as URIInterface from smithy_core.schemas import APIOperation from smithy_core.shapes import ShapeID +from smithy_core.types import TypedProperties from smithy_http import Fields -from smithy_http.aio import HTTPRequest +from smithy_http.aio import HTTPRequest, HTTPResponse from smithy_http.aio.interfaces import HTTPRequest as HTTPRequestInterface from smithy_http.aio.interfaces import HTTPResponse as HTTPResponseInterface -from smithy_http.aio.protocols import HttpClientProtocol +from smithy_http.aio.protocols import HttpBindingClientProtocol, HttpClientProtocol -class TestProtocol(HttpClientProtocol): +class MockProtocol(HttpClientProtocol): _id = ShapeID("ns.foo#bar") @property @@ -125,7 +126,7 @@ def deserialize_response( def test_http_protocol_joins_uris( request_uri: URI, endpoint_uri: URI, expected: URI ) -> None: - protocol = TestProtocol() + protocol = MockProtocol() request = HTTPRequest( destination=request_uri, method="GET", @@ -135,3 +136,28 @@ def test_http_protocol_joins_uris( updated_request = protocol.set_service_endpoint(request=request, endpoint=endpoint) actual = updated_request.destination assert actual == expected + + +@pytest.mark.asyncio +async def test_http_408_creates_timeout_error() -> None: + """Test that HTTP 408 creates a timeout error with server fault.""" + protocol = Mock(spec=HttpBindingClientProtocol) + protocol.error_identifier = Mock() + protocol.error_identifier.identify.return_value = None + + response = HTTPResponse(status=408, fields=Fields()) + + error = await HttpBindingClientProtocol._create_error( + protocol, + operation=Mock(), + request=HTTPRequest( + destination=URI(host="example.com"), method="POST", fields=Fields() + ), + response=response, + response_body=b"", + error_registry=TypeRegistry({}), + context=TypedProperties(), + ) + + assert error.is_timeout_error is True + assert error.fault == "server" diff --git a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py new file mode 100644 index 000000000..4bec2d2be --- /dev/null +++ b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py @@ -0,0 +1,63 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +import pytest +from smithy_core.aio.interfaces import ErrorInfo + +try: + from smithy_http.aio.aiohttp import AIOHTTPClient + + HAS_AIOHTTP = True +except ImportError: + HAS_AIOHTTP = False + +try: + from smithy_http.aio.crt import AWSCRTHTTPClient + + HAS_CRT = True +except ImportError: + HAS_CRT = False + + +@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not available") +class TestAIOHTTPTimeoutErrorHandling: + """Test timeout error handling for AIOHTTPClient.""" + + @pytest.fixture + async def client(self): + return AIOHTTPClient() + + @pytest.mark.asyncio + async def test_timeout_error_detection(self, client): + """Test timeout error detection for standard TimeoutError.""" + timeout_err = TimeoutError("Connection timed out") + result = client.get_error_info(timeout_err) + assert result == ErrorInfo(is_timeout_error=True, fault="client") + + @pytest.mark.asyncio + async def test_non_timeout_error_detection(self, client): + """Test non-timeout error detection.""" + other_err = ValueError("Not a timeout") + result = client.get_error_info(other_err) + assert result == ErrorInfo(is_timeout_error=False, fault="client") + + +@pytest.mark.skipif(not HAS_CRT, reason="AWS CRT not available") +class TestAWSCRTTimeoutErrorHandling: + """Test timeout error handling for AWSCRTHTTPClient.""" + + @pytest.fixture + def client(self): + return AWSCRTHTTPClient() + + def test_timeout_error_detection(self, client): + """Test timeout error detection for standard TimeoutError.""" + timeout_err = TimeoutError("Connection timed out") + result = client.get_error_info(timeout_err) + assert result == ErrorInfo(is_timeout_error=True, fault="client") + + def test_non_timeout_error_detection(self, client): + """Test non-timeout error detection.""" + other_err = ValueError("Not a timeout") + result = client.get_error_info(other_err) + assert result == ErrorInfo(is_timeout_error=False, fault="client") From 06ece22397f9e4ad5bb3a35ee801bf71dc2ef847 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Tue, 28 Oct 2025 13:41:06 -0400 Subject: [PATCH 2/9] pyright updates --- .../smithy_core/aio/interfaces/__init__.py | 2 +- .../smithy-core/src/smithy_core/exceptions.py | 2 +- .../src/smithy_http/aio/aiohttp.py | 2 +- .../smithy-http/src/smithy_http/aio/crt.py | 2 +- .../tests/unit/aio/test_protocols.py | 2 +- .../tests/unit/aio/test_timeout_errors.py | 30 +++++++++++-------- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py index e3f8974be..9abed5734 100644 --- a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py +++ b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py @@ -105,7 +105,7 @@ class ClientTransport[I: Request, O: Response](Protocol): exceptions represent timeout conditions for that transport. """ - def get_error_info(self, exception: Exception, **kwargs) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: """Get information about an exception. Args: diff --git a/packages/smithy-core/src/smithy_core/exceptions.py b/packages/smithy-core/src/smithy_core/exceptions.py index 53e4aacb3..0a99976f9 100644 --- a/packages/smithy-core/src/smithy_core/exceptions.py +++ b/packages/smithy-core/src/smithy_core/exceptions.py @@ -75,7 +75,7 @@ class ClientTimeoutError(CallError): fault: Fault = "client" is_timeout_error: bool = True - is_retry_safe: bool = True + is_retry_safe: bool | None = True class SerializationError(SmithyError): diff --git a/packages/smithy-http/src/smithy_http/aio/aiohttp.py b/packages/smithy-http/src/smithy_http/aio/aiohttp.py index d1935ada6..a774af4ed 100644 --- a/packages/smithy-http/src/smithy_http/aio/aiohttp.py +++ b/packages/smithy-http/src/smithy_http/aio/aiohttp.py @@ -52,7 +52,7 @@ def __post_init__(self) -> None: class AIOHTTPClient(HTTPClient): """Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp.""" - def get_error_info(self, exception: Exception, **kwargs) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: """Get information about aiohttp errors.""" if isinstance(exception, TimeoutError): diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index 39d68c0df..2a997e6e8 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -136,7 +136,7 @@ class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient): _HTTP_PORT = 80 _HTTPS_PORT = 443 - def get_error_info(self, exception: Exception, **kwargs) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: """Get information about CRT errors.""" timeout_indicators = ( diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index 665989865..b43f6bcd7 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -147,7 +147,7 @@ async def test_http_408_creates_timeout_error() -> None: response = HTTPResponse(status=408, fields=Fields()) - error = await HttpBindingClientProtocol._create_error( + error = await HttpBindingClientProtocol._create_error( # type: ignore[reportPrivateUsage] protocol, operation=Mock(), request=HTTPRequest( diff --git a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py index 4bec2d2be..d4d87b999 100644 --- a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py +++ b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py @@ -1,62 +1,68 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 +from typing import TYPE_CHECKING + import pytest from smithy_core.aio.interfaces import ErrorInfo +if TYPE_CHECKING: + from smithy_http.aio.aiohttp import AIOHTTPClient + from smithy_http.aio.crt import AWSCRTHTTPClient + try: from smithy_http.aio.aiohttp import AIOHTTPClient - HAS_AIOHTTP = True + has_aiohttp = True except ImportError: - HAS_AIOHTTP = False + has_aiohttp = False try: from smithy_http.aio.crt import AWSCRTHTTPClient - HAS_CRT = True + has_crt = True except ImportError: - HAS_CRT = False + has_crt = False -@pytest.mark.skipif(not HAS_AIOHTTP, reason="aiohttp not available") +@pytest.mark.skipif(not has_aiohttp, reason="aiohttp not available") class TestAIOHTTPTimeoutErrorHandling: """Test timeout error handling for AIOHTTPClient.""" @pytest.fixture - async def client(self): + async def client(self) -> "AIOHTTPClient": return AIOHTTPClient() @pytest.mark.asyncio - async def test_timeout_error_detection(self, client): + async def test_timeout_error_detection(self, client: "AIOHTTPClient") -> None: """Test timeout error detection for standard TimeoutError.""" timeout_err = TimeoutError("Connection timed out") result = client.get_error_info(timeout_err) assert result == ErrorInfo(is_timeout_error=True, fault="client") @pytest.mark.asyncio - async def test_non_timeout_error_detection(self, client): + async def test_non_timeout_error_detection(self, client: "AIOHTTPClient") -> None: """Test non-timeout error detection.""" other_err = ValueError("Not a timeout") result = client.get_error_info(other_err) assert result == ErrorInfo(is_timeout_error=False, fault="client") -@pytest.mark.skipif(not HAS_CRT, reason="AWS CRT not available") +@pytest.mark.skipif(not has_crt, reason="AWS CRT not available") class TestAWSCRTTimeoutErrorHandling: """Test timeout error handling for AWSCRTHTTPClient.""" @pytest.fixture - def client(self): + def client(self) -> "AWSCRTHTTPClient": return AWSCRTHTTPClient() - def test_timeout_error_detection(self, client): + def test_timeout_error_detection(self, client: "AWSCRTHTTPClient") -> None: """Test timeout error detection for standard TimeoutError.""" timeout_err = TimeoutError("Connection timed out") result = client.get_error_info(timeout_err) assert result == ErrorInfo(is_timeout_error=True, fault="client") - def test_non_timeout_error_detection(self, client): + def test_non_timeout_error_detection(self, client: "AWSCRTHTTPClient") -> None: """Test non-timeout error detection.""" other_err = ValueError("Not a timeout") result = client.get_error_info(other_err) From 264f02aaf90df506dbc77fbbb0b812b47189bbda Mon Sep 17 00:00:00 2001 From: SamRemis Date: Tue, 28 Oct 2025 14:02:54 -0400 Subject: [PATCH 3/9] Add new required method to codegen --- .../python/codegen/HttpProtocolTestGenerator.java | 10 ++++++++++ packages/smithy-http/tests/unit/aio/test_protocols.py | 5 +++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java index 83f1258f4..37a1df3f3 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java @@ -619,6 +619,8 @@ private void writeUtilStubs(Symbol serviceSymbol) { writer.addImport("smithy_http", "tuples_to_fields"); writer.addImport("smithy_http.aio", "HTTPResponse", "_HTTPResponse"); writer.addImport("smithy_core.aio.utils", "async_list"); + writer.addImport("smithy_core.aio.interfaces", "ErrorInfo"); + writer.addStdlibImport("typing", "Any"); writer.write(""" class $1L($2T): @@ -634,6 +636,10 @@ class $3L: def __init__(self, *, client_config: HTTPClientConfiguration | None = None): self._client_config = client_config + def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + \"\"\"Get information about an exception.\"\"\" + return ErrorInfo(is_timeout_error=False, fault="client") + async def send( self, request: HTTPRequest, *, request_config: HTTPRequestConfiguration | None = None ) -> HTTPResponse: @@ -657,6 +663,10 @@ def __init__( self.fields = tuples_to_fields(headers or []) self.body = body + def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + \"\"\"Get information about an exception.\"\"\" + return ErrorInfo(is_timeout_error=False, fault="client") + async def send( self, request: HTTPRequest, *, request_config: HTTPRequestConfiguration | None = None ) -> _HTTPResponse: diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index b43f6bcd7..88feb98a8 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -8,10 +8,11 @@ from smithy_core import URI from smithy_core.documents import TypeRegistry from smithy_core.endpoints import Endpoint +from smithy_core.interfaces import TypedProperties from smithy_core.interfaces import URI as URIInterface from smithy_core.schemas import APIOperation from smithy_core.shapes import ShapeID -from smithy_core.types import TypedProperties +from smithy_core.types import TypedProperties as ConcreteTypedProperties from smithy_http import Fields from smithy_http.aio import HTTPRequest, HTTPResponse from smithy_http.aio.interfaces import HTTPRequest as HTTPRequestInterface @@ -156,7 +157,7 @@ async def test_http_408_creates_timeout_error() -> None: response=response, response_body=b"", error_registry=TypeRegistry({}), - context=TypedProperties(), + context=ConcreteTypedProperties(), ) assert error.is_timeout_error is True From f35ddd11de5ec5e6c20077658be197f7609a69ae Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 5 Nov 2025 12:09:26 -0500 Subject: [PATCH 4/9] Updates from feedback --- packages/smithy-http/src/smithy_http/aio/crt.py | 3 +-- packages/smithy-http/src/smithy_http/aio/protocols.py | 9 ++------- packages/smithy-http/tests/unit/aio/test_protocols.py | 2 +- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index 2a997e6e8..cd4d4f7e0 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -141,8 +141,7 @@ def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: timeout_indicators = ( "AWS_IO_SOCKET_TIMEOUT", - "AWS_IO_CHANNEL_ERROR_SOCKET_TIMEOUT", - "AWS_ERROR_HTTP_REQUEST_TIMEOUT", + "AWS_IO_SOCKET_CLOSED", ) if isinstance(exception, TimeoutError): return ErrorInfo(is_timeout_error=True, fault="client") diff --git a/packages/smithy-http/src/smithy_http/aio/protocols.py b/packages/smithy-http/src/smithy_http/aio/protocols.py index 992f72d35..af32cee16 100644 --- a/packages/smithy-http/src/smithy_http/aio/protocols.py +++ b/packages/smithy-http/src/smithy_http/aio/protocols.py @@ -224,14 +224,9 @@ async def _create_error( if response.reason is not None: message += f" - reason: {response.status}" - if response.status == 408: - is_timeout = True - fault = "server" - else: - is_timeout = False - fault = "client" if response.status < 500 else "server" - + is_timeout = response.status == 408 is_throttle = response.status == 429 + fault = "client" if response.status < 500 else "server" return CallError( message=message, diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index 88feb98a8..f56df85ca 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -161,4 +161,4 @@ async def test_http_408_creates_timeout_error() -> None: ) assert error.is_timeout_error is True - assert error.fault == "server" + assert error.fault == "client" From 92dd38fdc8a90a28eff326c8dff091c46b0780be Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 5 Nov 2025 12:51:43 -0500 Subject: [PATCH 5/9] Remove fault from ClientErrorInfo --- .../smithy-core/src/smithy_core/aio/client.py | 2 +- .../smithy_core/aio/interfaces/__init__.py | 11 +++--- .../src/smithy_http/aio/aiohttp.py | 8 ++--- .../smithy-http/src/smithy_http/aio/crt.py | 11 +++--- .../tests/unit/aio/test_timeout_errors.py | 36 ++++++++++++------- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/packages/smithy-core/src/smithy_core/aio/client.py b/packages/smithy-core/src/smithy_core/aio/client.py index b6ba28948..a1d09723e 100644 --- a/packages/smithy-core/src/smithy_core/aio/client.py +++ b/packages/smithy-core/src/smithy_core/aio/client.py @@ -471,7 +471,7 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( error_info = self.transport.get_error_info(e) if error_info.is_timeout_error: raise ClientTimeoutError( - message=f"Client timeout occurred: {e}", fault=error_info.fault + message=f"Client timeout occurred: {e}" ) from e raise diff --git a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py index 9abed5734..21d6911ad 100644 --- a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py +++ b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 from collections.abc import AsyncIterable, Callable from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Literal, Protocol, runtime_checkable +from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable from ...documents import TypeRegistry from ...endpoints import EndpointResolverParams @@ -13,15 +13,12 @@ @dataclass(frozen=True) -class ErrorInfo: +class ClientErrorInfo: """Information about an error from a transport.""" is_timeout_error: bool """Whether this error represents a timeout condition.""" - fault: Literal["client", "server"] = "client" - """Whether the client or server is at fault.""" - if TYPE_CHECKING: from typing_extensions import TypeForm @@ -105,7 +102,7 @@ class ClientTransport[I: Request, O: Response](Protocol): exceptions represent timeout conditions for that transport. """ - def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: """Get information about an exception. Args: @@ -113,7 +110,7 @@ def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: **kwargs: Additional context for analysis Returns: - ErrorInfo with timeout and fault information. + ClientErrorInfo with timeout information. """ ... diff --git a/packages/smithy-http/src/smithy_http/aio/aiohttp.py b/packages/smithy-http/src/smithy_http/aio/aiohttp.py index a774af4ed..dab82da54 100644 --- a/packages/smithy-http/src/smithy_http/aio/aiohttp.py +++ b/packages/smithy-http/src/smithy_http/aio/aiohttp.py @@ -20,7 +20,7 @@ except ImportError: HAS_AIOHTTP = False # type: ignore -from smithy_core.aio.interfaces import ErrorInfo, StreamingBlob +from smithy_core.aio.interfaces import ClientErrorInfo, StreamingBlob from smithy_core.aio.types import AsyncBytesReader from smithy_core.aio.utils import async_list from smithy_core.exceptions import MissingDependencyError @@ -52,13 +52,13 @@ def __post_init__(self) -> None: class AIOHTTPClient(HTTPClient): """Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp.""" - def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: """Get information about aiohttp errors.""" if isinstance(exception, TimeoutError): - return ErrorInfo(is_timeout_error=True) + return ClientErrorInfo(is_timeout_error=True) - return ErrorInfo(is_timeout_error=False) + return ClientErrorInfo(is_timeout_error=False) def __init__( self, diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index cd4d4f7e0..1156aca4d 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -35,7 +35,7 @@ from smithy_core import interfaces as core_interfaces from smithy_core.aio import interfaces as core_aio_interfaces -from smithy_core.aio.interfaces import ErrorInfo +from smithy_core.aio.interfaces import ClientErrorInfo from smithy_core.aio.types import AsyncBytesReader from smithy_core.exceptions import MissingDependencyError @@ -136,20 +136,17 @@ class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient): _HTTP_PORT = 80 _HTTPS_PORT = 443 - def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: """Get information about CRT errors.""" timeout_indicators = ( "AWS_IO_SOCKET_TIMEOUT", "AWS_IO_SOCKET_CLOSED", ) - if isinstance(exception, TimeoutError): - return ErrorInfo(is_timeout_error=True, fault="client") - if isinstance(exception, AwsCrtError) and exception.name in timeout_indicators: - return ErrorInfo(is_timeout_error=True, fault="client") + return ClientErrorInfo(is_timeout_error=True) - return ErrorInfo(is_timeout_error=False) + return ClientErrorInfo(is_timeout_error=False) def __init__( self, diff --git a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py index d4d87b999..f3ab7c148 100644 --- a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py +++ b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING import pytest -from smithy_core.aio.interfaces import ErrorInfo +from smithy_core.aio.interfaces import ClientErrorInfo if TYPE_CHECKING: from smithy_http.aio.aiohttp import AIOHTTPClient @@ -19,6 +19,7 @@ try: from smithy_http.aio.crt import AWSCRTHTTPClient + from awscrt.exceptions import AwsCrtError has_crt = True except ImportError: @@ -38,14 +39,14 @@ async def test_timeout_error_detection(self, client: "AIOHTTPClient") -> None: """Test timeout error detection for standard TimeoutError.""" timeout_err = TimeoutError("Connection timed out") result = client.get_error_info(timeout_err) - assert result == ErrorInfo(is_timeout_error=True, fault="client") + assert result == ClientErrorInfo(is_timeout_error=True) @pytest.mark.asyncio async def test_non_timeout_error_detection(self, client: "AIOHTTPClient") -> None: """Test non-timeout error detection.""" other_err = ValueError("Not a timeout") result = client.get_error_info(other_err) - assert result == ErrorInfo(is_timeout_error=False, fault="client") + assert result == ClientErrorInfo(is_timeout_error=False) @pytest.mark.skipif(not has_crt, reason="AWS CRT not available") @@ -56,14 +57,25 @@ class TestAWSCRTTimeoutErrorHandling: def client(self) -> "AWSCRTHTTPClient": return AWSCRTHTTPClient() - def test_timeout_error_detection(self, client: "AWSCRTHTTPClient") -> None: - """Test timeout error detection for standard TimeoutError.""" - timeout_err = TimeoutError("Connection timed out") - result = client.get_error_info(timeout_err) - assert result == ErrorInfo(is_timeout_error=True, fault="client") - - def test_non_timeout_error_detection(self, client: "AWSCRTHTTPClient") -> None: - """Test non-timeout error detection.""" + @pytest.mark.parametrize( + "error_name,expected_timeout", + [ + ("AWS_IO_SOCKET_TIMEOUT", True), + ("AWS_IO_SOCKET_CLOSED", True), + ("AWS_IO_SOCKET_CONNECTION_REFUSED", False), + ], + ) + def test_crt_error_detection(self, client: "AWSCRTHTTPClient", error_name: str, expected_timeout: bool) -> None: + """Test CRT error detection for various error types.""" + if not has_crt: + pytest.skip("AWS CRT not available") + + crt_err = AwsCrtError(code=0, name=error_name, message=f"CRT error: {error_name}") + result = client.get_error_info(crt_err) + assert result == ClientErrorInfo(is_timeout_error=expected_timeout) + + def test_non_crt_error_detection(self, client: "AWSCRTHTTPClient") -> None: + """Test non-CRT error detection.""" other_err = ValueError("Not a timeout") result = client.get_error_info(other_err) - assert result == ErrorInfo(is_timeout_error=False, fault="client") + assert result == ClientErrorInfo(is_timeout_error=False) From ecb05aa04649d915a69ce32f93dc439011bd1d5c Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 5 Nov 2025 12:58:19 -0500 Subject: [PATCH 6/9] Remove outdated/incorrect docstrings --- packages/smithy-http/src/smithy_http/aio/aiohttp.py | 1 - packages/smithy-http/src/smithy_http/aio/crt.py | 1 - packages/smithy-http/tests/unit/aio/test_protocols.py | 1 - 3 files changed, 3 deletions(-) diff --git a/packages/smithy-http/src/smithy_http/aio/aiohttp.py b/packages/smithy-http/src/smithy_http/aio/aiohttp.py index dab82da54..f0505de56 100644 --- a/packages/smithy-http/src/smithy_http/aio/aiohttp.py +++ b/packages/smithy-http/src/smithy_http/aio/aiohttp.py @@ -53,7 +53,6 @@ class AIOHTTPClient(HTTPClient): """Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp.""" def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - """Get information about aiohttp errors.""" if isinstance(exception, TimeoutError): return ClientErrorInfo(is_timeout_error=True) diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index 1156aca4d..1d4f8e123 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -137,7 +137,6 @@ class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient): _HTTPS_PORT = 443 def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - """Get information about CRT errors.""" timeout_indicators = ( "AWS_IO_SOCKET_TIMEOUT", diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index f56df85ca..7d668cb53 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -141,7 +141,6 @@ def test_http_protocol_joins_uris( @pytest.mark.asyncio async def test_http_408_creates_timeout_error() -> None: - """Test that HTTP 408 creates a timeout error with server fault.""" protocol = Mock(spec=HttpBindingClientProtocol) protocol.error_identifier = Mock() protocol.error_identifier.identify.return_value = None From 9e37c52619cd3c46ad003620a1bdae2560cfd8cf Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 5 Nov 2025 13:07:28 -0500 Subject: [PATCH 7/9] CI fixes --- .../python/codegen/HttpProtocolTestGenerator.java | 10 +++++----- packages/smithy-http/src/smithy_http/aio/aiohttp.py | 1 - packages/smithy-http/src/smithy_http/aio/crt.py | 1 - .../tests/unit/aio/test_timeout_errors.py | 12 ++++++++---- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java index 37a1df3f3..ae0f098b0 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java @@ -619,7 +619,7 @@ private void writeUtilStubs(Symbol serviceSymbol) { writer.addImport("smithy_http", "tuples_to_fields"); writer.addImport("smithy_http.aio", "HTTPResponse", "_HTTPResponse"); writer.addImport("smithy_core.aio.utils", "async_list"); - writer.addImport("smithy_core.aio.interfaces", "ErrorInfo"); + writer.addImport("smithy_core.aio.interfaces", "ClientErrorInfo"); writer.addStdlibImport("typing", "Any"); writer.write(""" @@ -636,9 +636,9 @@ class $3L: def __init__(self, *, client_config: HTTPClientConfiguration | None = None): self._client_config = client_config - def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: \"\"\"Get information about an exception.\"\"\" - return ErrorInfo(is_timeout_error=False, fault="client") + return ClientErrorInfo(is_timeout_error=False) async def send( self, request: HTTPRequest, *, request_config: HTTPRequestConfiguration | None = None @@ -663,9 +663,9 @@ def __init__( self.fields = tuples_to_fields(headers or []) self.body = body - def get_error_info(self, exception: Exception, **kwargs: Any) -> ErrorInfo: + def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: \"\"\"Get information about an exception.\"\"\" - return ErrorInfo(is_timeout_error=False, fault="client") + return ClientErrorInfo(is_timeout_error=False) async def send( self, request: HTTPRequest, *, request_config: HTTPRequestConfiguration | None = None diff --git a/packages/smithy-http/src/smithy_http/aio/aiohttp.py b/packages/smithy-http/src/smithy_http/aio/aiohttp.py index f0505de56..5d80791f9 100644 --- a/packages/smithy-http/src/smithy_http/aio/aiohttp.py +++ b/packages/smithy-http/src/smithy_http/aio/aiohttp.py @@ -53,7 +53,6 @@ class AIOHTTPClient(HTTPClient): """Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp.""" def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - if isinstance(exception, TimeoutError): return ClientErrorInfo(is_timeout_error=True) diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index 1d4f8e123..fdee231e9 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -137,7 +137,6 @@ class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient): _HTTPS_PORT = 443 def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - timeout_indicators = ( "AWS_IO_SOCKET_TIMEOUT", "AWS_IO_SOCKET_CLOSED", diff --git a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py index f3ab7c148..01e96d19f 100644 --- a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py +++ b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py @@ -18,8 +18,8 @@ has_aiohttp = False try: + from awscrt.exceptions import AwsCrtError # type: ignore from smithy_http.aio.crt import AWSCRTHTTPClient - from awscrt.exceptions import AwsCrtError has_crt = True except ImportError: @@ -65,12 +65,16 @@ def client(self) -> "AWSCRTHTTPClient": ("AWS_IO_SOCKET_CONNECTION_REFUSED", False), ], ) - def test_crt_error_detection(self, client: "AWSCRTHTTPClient", error_name: str, expected_timeout: bool) -> None: + def test_crt_error_detection( + self, client: "AWSCRTHTTPClient", error_name: str, expected_timeout: bool + ) -> None: """Test CRT error detection for various error types.""" if not has_crt: pytest.skip("AWS CRT not available") - - crt_err = AwsCrtError(code=0, name=error_name, message=f"CRT error: {error_name}") + + crt_err = AwsCrtError( # type: ignore + code=0, name=error_name, message=f"CRT error: {error_name}" + ) result = client.get_error_info(crt_err) assert result == ClientErrorInfo(is_timeout_error=expected_timeout) From 70100eee10a94f8eb7dce8b68504b67f4048e764 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 12 Nov 2025 13:36:22 -0500 Subject: [PATCH 8/9] Make get_error_info optional --- .../smithy-core/src/smithy_core/aio/client.py | 17 +++++++++----- .../smithy_core/aio/interfaces/__init__.py | 22 ++++++++++++------- .../tests/unit/aio/test_protocols.py | 1 - 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/packages/smithy-core/src/smithy_core/aio/client.py b/packages/smithy-core/src/smithy_core/aio/client.py index a1d09723e..57432c589 100644 --- a/packages/smithy-core/src/smithy_core/aio/client.py +++ b/packages/smithy-core/src/smithy_core/aio/client.py @@ -6,7 +6,7 @@ from collections.abc import Awaitable, Callable, Sequence from copy import copy from dataclasses import dataclass, field, replace -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast from .. import URI from ..auth import AuthParams @@ -32,6 +32,7 @@ ClientProtocol, ClientTransport, EndpointResolver, + ErrorClassifyingTransport, Request, Response, ) @@ -468,11 +469,15 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( request=request_context.transport_request ) except Exception as e: - error_info = self.transport.get_error_info(e) - if error_info.is_timeout_error: - raise ClientTimeoutError( - message=f"Client timeout occurred: {e}" - ) from e + if hasattr(self.transport, "get_error_info"): + classifying_transport = cast( + ErrorClassifyingTransport[TRequest, TResponse], self.transport + ) + error_info = classifying_transport.get_error_info(e) + if error_info.is_timeout_error: + raise ClientTimeoutError( + message=f"Client timeout occurred: {e}" + ) from e raise _LOGGER.debug("Received response: %s", transport_response) diff --git a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py index 21d6911ad..a7a583e34 100644 --- a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py +++ b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py @@ -96,10 +96,20 @@ async def resolve_endpoint(self, params: EndpointResolverParams[Any]) -> Endpoin class ClientTransport[I: Request, O: Response](Protocol): - """Protocol-agnostic representation of a client transport (e.g. an HTTP client). + """Protocol-agnostic representation of a client transport (e.g. an HTTP client).""" - Transport implementations must define the get_error_info method to determine which - exceptions represent timeout conditions for that transport. + async def send(self, request: I) -> O: + """Send a request over the transport and receive the response.""" + ... + + +class ErrorClassifyingTransport[I: Request, O: Response]( + ClientTransport[I, O], Protocol +): + """A client transport that can classify errors for retry and timeout detection. + + Transport implementations should implement this protocol if they can determine + which exceptions represent timeout conditions or other classifiable error types. """ def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: @@ -110,14 +120,10 @@ def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo **kwargs: Additional context for analysis Returns: - ClientErrorInfo with timeout information. + ClientErrorInfo with error classification details. """ ... - async def send(self, request: I) -> O: - """Send a request over the transport and receive the response.""" - ... - class ClientProtocol[I: Request, O: Response](Protocol): """A protocol used by a client to communicate with a server.""" diff --git a/packages/smithy-http/tests/unit/aio/test_protocols.py b/packages/smithy-http/tests/unit/aio/test_protocols.py index 7d668cb53..7eb578d75 100644 --- a/packages/smithy-http/tests/unit/aio/test_protocols.py +++ b/packages/smithy-http/tests/unit/aio/test_protocols.py @@ -139,7 +139,6 @@ def test_http_protocol_joins_uris( assert actual == expected -@pytest.mark.asyncio async def test_http_408_creates_timeout_error() -> None: protocol = Mock(spec=HttpBindingClientProtocol) protocol.error_identifier = Mock() From 39dfa0f460f455efadf01cee7b8c223965967828 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 12 Nov 2025 19:06:19 -0500 Subject: [PATCH 9/9] Updates based on feedback --- .../codegen/HttpProtocolTestGenerator.java | 10 +-- .../smithy-core/src/smithy_core/aio/client.py | 16 ++-- .../smithy_core/aio/interfaces/__init__.py | 38 ++------- .../src/smithy_http/aio/aiohttp.py | 8 +- .../smithy-http/src/smithy_http/aio/crt.py | 39 ++++----- .../tests/unit/aio/test_timeout_errors.py | 85 ------------------- 6 files changed, 35 insertions(+), 161 deletions(-) delete mode 100644 packages/smithy-http/tests/unit/aio/test_timeout_errors.py diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java index ae0f098b0..2df706fa7 100644 --- a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java +++ b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/HttpProtocolTestGenerator.java @@ -619,8 +619,6 @@ private void writeUtilStubs(Symbol serviceSymbol) { writer.addImport("smithy_http", "tuples_to_fields"); writer.addImport("smithy_http.aio", "HTTPResponse", "_HTTPResponse"); writer.addImport("smithy_core.aio.utils", "async_list"); - writer.addImport("smithy_core.aio.interfaces", "ClientErrorInfo"); - writer.addStdlibImport("typing", "Any"); writer.write(""" class $1L($2T): @@ -636,9 +634,7 @@ class $3L: def __init__(self, *, client_config: HTTPClientConfiguration | None = None): self._client_config = client_config - def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - \"\"\"Get information about an exception.\"\"\" - return ClientErrorInfo(is_timeout_error=False) + TIMEOUT_EXCEPTIONS = () async def send( self, request: HTTPRequest, *, request_config: HTTPRequestConfiguration | None = None @@ -663,9 +659,7 @@ def __init__( self.fields = tuples_to_fields(headers or []) self.body = body - def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - \"\"\"Get information about an exception.\"\"\" - return ClientErrorInfo(is_timeout_error=False) + TIMEOUT_EXCEPTIONS = () async def send( self, request: HTTPRequest, *, request_config: HTTPRequestConfiguration | None = None diff --git a/packages/smithy-core/src/smithy_core/aio/client.py b/packages/smithy-core/src/smithy_core/aio/client.py index 57432c589..3fc2b31e1 100644 --- a/packages/smithy-core/src/smithy_core/aio/client.py +++ b/packages/smithy-core/src/smithy_core/aio/client.py @@ -6,7 +6,7 @@ from collections.abc import Awaitable, Callable, Sequence from copy import copy from dataclasses import dataclass, field, replace -from typing import TYPE_CHECKING, Any, cast +from typing import TYPE_CHECKING, Any from .. import URI from ..auth import AuthParams @@ -32,7 +32,6 @@ ClientProtocol, ClientTransport, EndpointResolver, - ErrorClassifyingTransport, Request, Response, ) @@ -469,15 +468,10 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( request=request_context.transport_request ) except Exception as e: - if hasattr(self.transport, "get_error_info"): - classifying_transport = cast( - ErrorClassifyingTransport[TRequest, TResponse], self.transport - ) - error_info = classifying_transport.get_error_info(e) - if error_info.is_timeout_error: - raise ClientTimeoutError( - message=f"Client timeout occurred: {e}" - ) from e + if isinstance(e, self.transport.TIMEOUT_EXCEPTIONS): + raise ClientTimeoutError( + message=f"Client timeout occurred: {e}" + ) from e raise _LOGGER.debug("Received response: %s", transport_response) diff --git a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py index a7a583e34..03ffec96c 100644 --- a/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py +++ b/packages/smithy-core/src/smithy_core/aio/interfaces/__init__.py @@ -1,7 +1,6 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 from collections.abc import AsyncIterable, Callable -from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable from ...documents import TypeRegistry @@ -11,15 +10,6 @@ from ...interfaces import StreamingBlob as SyncStreamingBlob from .eventstream import EventPublisher, EventReceiver - -@dataclass(frozen=True) -class ClientErrorInfo: - """Information about an error from a transport.""" - - is_timeout_error: bool - """Whether this error represents a timeout condition.""" - - if TYPE_CHECKING: from typing_extensions import TypeForm @@ -96,32 +86,16 @@ async def resolve_endpoint(self, params: EndpointResolverParams[Any]) -> Endpoin class ClientTransport[I: Request, O: Response](Protocol): - """Protocol-agnostic representation of a client transport (e.g. an HTTP client).""" - - async def send(self, request: I) -> O: - """Send a request over the transport and receive the response.""" - ... - + """Protocol-agnostic representation of a client transport (e.g. an HTTP client). -class ErrorClassifyingTransport[I: Request, O: Response]( - ClientTransport[I, O], Protocol -): - """A client transport that can classify errors for retry and timeout detection. - - Transport implementations should implement this protocol if they can determine - which exceptions represent timeout conditions or other classifiable error types. + Transports must define TIMEOUT_EXCEPTIONS as a tuple of exception types that + are raised when a timeout occurs. """ - def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - """Get information about an exception. - - Args: - exception: The exception to analyze - **kwargs: Additional context for analysis + TIMEOUT_EXCEPTIONS: tuple[type[Exception], ...] - Returns: - ClientErrorInfo with error classification details. - """ + async def send(self, request: I) -> O: + """Send a request over the transport and receive the response.""" ... diff --git a/packages/smithy-http/src/smithy_http/aio/aiohttp.py b/packages/smithy-http/src/smithy_http/aio/aiohttp.py index 5d80791f9..2d5114726 100644 --- a/packages/smithy-http/src/smithy_http/aio/aiohttp.py +++ b/packages/smithy-http/src/smithy_http/aio/aiohttp.py @@ -20,7 +20,7 @@ except ImportError: HAS_AIOHTTP = False # type: ignore -from smithy_core.aio.interfaces import ClientErrorInfo, StreamingBlob +from smithy_core.aio.interfaces import StreamingBlob from smithy_core.aio.types import AsyncBytesReader from smithy_core.aio.utils import async_list from smithy_core.exceptions import MissingDependencyError @@ -52,11 +52,7 @@ def __post_init__(self) -> None: class AIOHTTPClient(HTTPClient): """Implementation of :py:class:`.interfaces.HTTPClient` using aiohttp.""" - def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - if isinstance(exception, TimeoutError): - return ClientErrorInfo(is_timeout_error=True) - - return ClientErrorInfo(is_timeout_error=False) + TIMEOUT_EXCEPTIONS = (TimeoutError,) def __init__( self, diff --git a/packages/smithy-http/src/smithy_http/aio/crt.py b/packages/smithy-http/src/smithy_http/aio/crt.py index fdee231e9..db13162e1 100644 --- a/packages/smithy-http/src/smithy_http/aio/crt.py +++ b/packages/smithy-http/src/smithy_http/aio/crt.py @@ -35,7 +35,6 @@ from smithy_core import interfaces as core_interfaces from smithy_core.aio import interfaces as core_aio_interfaces -from smithy_core.aio.interfaces import ClientErrorInfo from smithy_core.aio.types import AsyncBytesReader from smithy_core.exceptions import MissingDependencyError @@ -132,19 +131,16 @@ def __post_init__(self) -> None: _assert_crt() +class _CRTTimeoutError(Exception): + """Internal wrapper for CRT timeout errors.""" + + class AWSCRTHTTPClient(http_aio_interfaces.HTTPClient): _HTTP_PORT = 80 _HTTPS_PORT = 443 + _TIMEOUT_ERROR_NAMES = frozenset(["AWS_IO_SOCKET_TIMEOUT", "AWS_IO_SOCKET_CLOSED"]) - def get_error_info(self, exception: Exception, **kwargs: Any) -> ClientErrorInfo: - timeout_indicators = ( - "AWS_IO_SOCKET_TIMEOUT", - "AWS_IO_SOCKET_CLOSED", - ) - if isinstance(exception, AwsCrtError) and exception.name in timeout_indicators: - return ClientErrorInfo(is_timeout_error=True) - - return ClientErrorInfo(is_timeout_error=False) + TIMEOUT_EXCEPTIONS = (_CRTTimeoutError,) def __init__( self, @@ -176,18 +172,23 @@ async def send( :param request: The request including destination URI, fields, payload. :param request_config: Configuration specific to this request. """ - crt_request = self._marshal_request(request) - connection = await self._get_connection(request.destination) + try: + crt_request = self._marshal_request(request) + connection = await self._get_connection(request.destination) - # Convert body to async iterator for request_body_generator - body_generator = self._create_body_generator(request.body) + # Convert body to async iterator for request_body_generator + body_generator = self._create_body_generator(request.body) - crt_stream = connection.request( - crt_request, - request_body_generator=body_generator, - ) + crt_stream = connection.request( + crt_request, + request_body_generator=body_generator, + ) - return await self._await_response(crt_stream) + return await self._await_response(crt_stream) + except AwsCrtError as e: + if e.name in self._TIMEOUT_ERROR_NAMES: + raise _CRTTimeoutError() from e + raise async def _await_response( self, stream: "AIOHttpClientStreamUnified" diff --git a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py b/packages/smithy-http/tests/unit/aio/test_timeout_errors.py deleted file mode 100644 index 01e96d19f..000000000 --- a/packages/smithy-http/tests/unit/aio/test_timeout_errors.py +++ /dev/null @@ -1,85 +0,0 @@ -# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -from typing import TYPE_CHECKING - -import pytest -from smithy_core.aio.interfaces import ClientErrorInfo - -if TYPE_CHECKING: - from smithy_http.aio.aiohttp import AIOHTTPClient - from smithy_http.aio.crt import AWSCRTHTTPClient - -try: - from smithy_http.aio.aiohttp import AIOHTTPClient - - has_aiohttp = True -except ImportError: - has_aiohttp = False - -try: - from awscrt.exceptions import AwsCrtError # type: ignore - from smithy_http.aio.crt import AWSCRTHTTPClient - - has_crt = True -except ImportError: - has_crt = False - - -@pytest.mark.skipif(not has_aiohttp, reason="aiohttp not available") -class TestAIOHTTPTimeoutErrorHandling: - """Test timeout error handling for AIOHTTPClient.""" - - @pytest.fixture - async def client(self) -> "AIOHTTPClient": - return AIOHTTPClient() - - @pytest.mark.asyncio - async def test_timeout_error_detection(self, client: "AIOHTTPClient") -> None: - """Test timeout error detection for standard TimeoutError.""" - timeout_err = TimeoutError("Connection timed out") - result = client.get_error_info(timeout_err) - assert result == ClientErrorInfo(is_timeout_error=True) - - @pytest.mark.asyncio - async def test_non_timeout_error_detection(self, client: "AIOHTTPClient") -> None: - """Test non-timeout error detection.""" - other_err = ValueError("Not a timeout") - result = client.get_error_info(other_err) - assert result == ClientErrorInfo(is_timeout_error=False) - - -@pytest.mark.skipif(not has_crt, reason="AWS CRT not available") -class TestAWSCRTTimeoutErrorHandling: - """Test timeout error handling for AWSCRTHTTPClient.""" - - @pytest.fixture - def client(self) -> "AWSCRTHTTPClient": - return AWSCRTHTTPClient() - - @pytest.mark.parametrize( - "error_name,expected_timeout", - [ - ("AWS_IO_SOCKET_TIMEOUT", True), - ("AWS_IO_SOCKET_CLOSED", True), - ("AWS_IO_SOCKET_CONNECTION_REFUSED", False), - ], - ) - def test_crt_error_detection( - self, client: "AWSCRTHTTPClient", error_name: str, expected_timeout: bool - ) -> None: - """Test CRT error detection for various error types.""" - if not has_crt: - pytest.skip("AWS CRT not available") - - crt_err = AwsCrtError( # type: ignore - code=0, name=error_name, message=f"CRT error: {error_name}" - ) - result = client.get_error_info(crt_err) - assert result == ClientErrorInfo(is_timeout_error=expected_timeout) - - def test_non_crt_error_detection(self, client: "AWSCRTHTTPClient") -> None: - """Test non-CRT error detection.""" - other_err = ValueError("Not a timeout") - result = client.get_error_info(other_err) - assert result == ClientErrorInfo(is_timeout_error=False)