Copy request for each attempt / don't retry client errors (#460)

Author: alextwoods

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

@@ -132,6 +132,7 @@ private void generateOperationExecutor(PythonWriter writer) {
         writer.addStdlibImport("typing", "Awaitable");
         writer.addStdlibImport("typing", "cast");
         writer.addStdlibImport("copy", "deepcopy");
+        writer.addStdlibImport("copy", "copy");
         writer.addStdlibImport("asyncio");
         writer.addStdlibImports("asyncio", Set.of("sleep", "Future"));
         writer.addStdlibImport("dataclasses", "replace");
@@ -395,7 +396,10 @@ def _classify_error(
                                     output_context = await self._handle_attempt(
                                         deserialize,
                                         interceptor_chain,
-                                        request_context,
+                                        replace(
+                                          request_context,
+                                          transport_request = copy(request_context.transport_request)
+                                        ),
                                         config,
                                         operation,
                                         request_future,

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

@@ -5,6 +5,8 @@
 from dataclasses import dataclass
 from enum import Enum
 
+from smithy_core.interfaces.retries import RetryErrorType
+
 from .exceptions import SmithyRetryException
 from .interfaces import retries as retries_interface
 
@@ -232,13 +234,16 @@ def refresh_retry_token_for_retry(
 
         :raises SmithyRetryException: If no further retry attempts are allowed.
         """
-        retry_count = token_to_renew.retry_count + 1
-        if retry_count >= self.max_attempts:
-            raise SmithyRetryException(
-                f"Reached maximum number of allowed attempts: {self.max_attempts}"
-            )
-        retry_delay = self.backoff_strategy.compute_next_backoff_delay(retry_count)
-        return SimpleRetryToken(retry_count=retry_count, retry_delay=retry_delay)
+        if error_info.error_type is not RetryErrorType.CLIENT_ERROR:
+            retry_count = token_to_renew.retry_count + 1
+            if retry_count >= self.max_attempts:
+                raise SmithyRetryException(
+                    f"Reached maximum number of allowed attempts: {self.max_attempts}"
+                )
+            retry_delay = self.backoff_strategy.compute_next_backoff_delay(retry_count)
+            return SimpleRetryToken(retry_count=retry_count, retry_delay=retry_delay)
+        else:
+            raise SmithyRetryException(f"Error is not retryable: {error_info}")
 
     def record_success(self, *, token: retries_interface.RetryToken) -> None:
         """Not used by this retry strategy."""

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

@@ -71,3 +71,16 @@ def test_simple_retry_strategy(max_attempts: int) -> None:
         strategy.refresh_retry_token_for_retry(
             token_to_renew=token, error_info=error_info
         )
+
+
+def test_simple_retry_strategy_does_not_retry_client_errors() -> None:
+    strategy = SimpleRetryStrategy(
+        backoff_strategy=ExponentialRetryBackoffStrategy(backoff_scale_value=5),
+        max_attempts=2,
+    )
+    error_info = RetryErrorInfo(error_type=RetryErrorType.CLIENT_ERROR)
+    token = strategy.acquire_initial_retry_token()
+    with pytest.raises(SmithyRetryException):
+        strategy.refresh_retry_token_for_retry(
+            token_to_renew=token, error_info=error_info
+        )