Fix native Smithy client retry and retry response errors (#1717)

* Fix retry for native Smithy clients
* Treat `SdkError::ResponseError` as a retryable transient failure
* Rename `ClassifyResponse` to `ClassifyRetry`
* Rename 'policy' to 'classifier' in AWS SDK public API
* Rename `AwsResponseClassifier` to `AwsResponseRetryClassifier`

Co-authored-by: Zelda Hessler <zhessler@amazon.com>

CHANGELOG.next.toml

@@ -192,3 +192,47 @@ message = "The AWS STS SDK now automatically retries `IDPCommunicationError` whe
 references = ["smithy-rs#966", "smithy-rs#1718"]
 meta = { "breaking" = false, "tada" = false, "bug" = true }
 author = "jdisanti"
+
+[[smithy-rs]]
+message = "Generated clients now retry transient errors without replacing the retry policy."
+references = ["smithy-rs#1715", "smithy-rs#1717"]
+meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }
+author = "jdisanti"
+
+[[smithy-rs]]
+message = "`ClassifyResponse` was renamed to `ClassifyRetry` and is no longer implemented for the unit type."
+references = ["smithy-rs#1715", "smithy-rs#1717"]
+meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
+author = "jdisanti"
+
+[[smithy-rs]]
+message = """
+The `with_retry_policy` and `retry_policy` functions on `aws_smithy_http::operation::Operation` have been
+renamed to `with_retry_classifier` and `retry_classifier` respectively. Public member `retry_policy` on
+`aws_smithy_http::operation::Parts` has been renamed to `retry_classifier`.
+"""
+references = ["smithy-rs#1715", "smithy-rs#1717"]
+meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
+author = "jdisanti"
+
+[[aws-sdk-rust]]
+message = "The `SdkError::ResponseError`, typically caused by a connection terminating before the full response is received, is now treated as a transient failure and retried."
+references = ["aws-sdk-rust#303", "smithy-rs#1717"]
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+author = "jdisanti"
+
+[[aws-sdk-rust]]
+message = "`ClassifyResponse` was renamed to `ClassifyRetry` and is no longer implemented for the unit type."
+references = ["smithy-rs#1715", "smithy-rs#1717"]
+meta = { "breaking" = true, "tada" = false, "bug" = false }
+author = "jdisanti"
+
+[[aws-sdk-rust]]
+message = """
+The `with_retry_policy` and `retry_policy` functions on `aws_smithy_http::operation::Operation` have been
+renamed to `with_retry_classifier` and `retry_classifier` respectively. Public member `retry_policy` on
+`aws_smithy_http::operation::Parts` has been renamed to `retry_classifier`.
+"""
+references = ["smithy-rs#1715", "smithy-rs#1717"]
+meta = { "breaking" = true, "tada" = false, "bug" = false }
+author = "jdisanti"

aws/rust-runtime/aws-config/src/http_credential_provider.rs

@@ -14,7 +14,7 @@ use aws_smithy_http::body::SdkBody;
 use aws_smithy_http::operation::{Operation, Request};
 use aws_smithy_http::response::ParseStrictResponse;
 use aws_smithy_http::result::{SdkError, SdkSuccess};
-use aws_smithy_http::retry::ClassifyResponse;
+use aws_smithy_http::retry::ClassifyRetry;
 use aws_smithy_types::retry::{ErrorKind, RetryKind};
 use aws_smithy_types::timeout;
 use aws_smithy_types::tristate::TriState;
@@ -58,7 +58,7 @@ impl HttpCredentialProvider {
     fn operation(
         &self,
         auth: Option<HeaderValue>,
-    ) -> Operation<CredentialsResponseParser, HttpCredentialRetryPolicy> {
+    ) -> Operation<CredentialsResponseParser, HttpCredentialRetryClassifier> {
         let mut http_req = http::Request::builder()
             .uri(&self.uri)
             .header(ACCEPT, "application/json");
@@ -73,7 +73,7 @@ impl HttpCredentialProvider {
                 provider_name: self.provider_name,
             },
         )
-        .with_retry_policy(HttpCredentialRetryPolicy)
+        .with_retry_classifier(HttpCredentialRetryClassifier)
     }
 }
 
@@ -165,12 +165,12 @@ impl ParseStrictResponse for CredentialsResponseParser {
 }
 
 #[derive(Clone, Debug)]
-struct HttpCredentialRetryPolicy;
+struct HttpCredentialRetryClassifier;
 
-impl ClassifyResponse<SdkSuccess<Credentials>, SdkError<CredentialsError>>
-    for HttpCredentialRetryPolicy
+impl ClassifyRetry<SdkSuccess<Credentials>, SdkError<CredentialsError>>
+    for HttpCredentialRetryClassifier
 {
-    fn classify(
+    fn classify_retry(
         &self,
         response: Result<&SdkSuccess<credentials::Credentials>, &SdkError<CredentialsError>>,
     ) -> RetryKind {
@@ -206,12 +206,14 @@ impl ClassifyResponse<SdkSuccess<Credentials>, SdkError<CredentialsError>>
 
 #[cfg(test)]
 mod test {
-    use crate::http_credential_provider::{CredentialsResponseParser, HttpCredentialRetryPolicy};
+    use crate::http_credential_provider::{
+        CredentialsResponseParser, HttpCredentialRetryClassifier,
+    };
     use aws_smithy_http::body::SdkBody;
     use aws_smithy_http::operation;
     use aws_smithy_http::response::ParseStrictResponse;
     use aws_smithy_http::result::{SdkError, SdkSuccess};
-    use aws_smithy_http::retry::ClassifyResponse;
+    use aws_smithy_http::retry::ClassifyRetry;
     use aws_smithy_types::retry::{ErrorKind, RetryKind};
     use aws_types::credentials::CredentialsError;
     use aws_types::Credentials;
@@ -245,7 +247,7 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            HttpCredentialRetryPolicy.classify(sdk_resp(bad_response).as_ref()),
+            HttpCredentialRetryClassifier.classify_retry(sdk_resp(bad_response).as_ref()),
             RetryKind::Error(ErrorKind::ServerError)
         );
     }
@@ -266,7 +268,7 @@ mod test {
         let sdk_result = sdk_resp(ok_response);
 
         assert_eq!(
-            HttpCredentialRetryPolicy.classify(sdk_result.as_ref()),
+            HttpCredentialRetryClassifier.classify_retry(sdk_result.as_ref()),
             RetryKind::Unnecessary
         );
 
@@ -281,7 +283,7 @@ mod test {
             .unwrap();
         let sdk_result = sdk_resp(error_response);
         assert_eq!(
-            HttpCredentialRetryPolicy.classify(sdk_result.as_ref()),
+            HttpCredentialRetryClassifier.classify_retry(sdk_result.as_ref()),
             RetryKind::UnretryableFailure
         );
         let sdk_error = sdk_result.expect_err("should be error");

aws/rust-runtime/aws-config/src/imds/client.rs

@@ -23,7 +23,7 @@ use aws_smithy_http::endpoint::Endpoint;
 use aws_smithy_http::operation;
 use aws_smithy_http::operation::{Metadata, Operation};
 use aws_smithy_http::response::ParseStrictResponse;
-use aws_smithy_http::retry::ClassifyResponse;
+use aws_smithy_http::retry::ClassifyRetry;
 use aws_smithy_http_tower::map_request::{
     AsyncMapRequestLayer, AsyncMapRequestService, MapRequestLayer, MapRequestService,
 };
@@ -232,7 +232,7 @@ impl Client {
     fn make_operation(
         &self,
         path: &str,
-    ) -> Result<Operation<ImdsGetResponseHandler, ImdsErrorPolicy>, ImdsError> {
+    ) -> Result<Operation<ImdsGetResponseHandler, ImdsResponseRetryClassifier>, ImdsError> {
         let mut base_uri: Uri = path.parse().map_err(|_| ImdsError::InvalidPath)?;
         self.inner.endpoint.set_endpoint(&mut base_uri, None);
         let request = http::Request::builder()
@@ -243,7 +243,7 @@ impl Client {
         request.properties_mut().insert(user_agent());
         Ok(Operation::new(request, ImdsGetResponseHandler)
             .with_metadata(Metadata::new("get", "imds"))
-            .with_retry_policy(ImdsErrorPolicy))
+            .with_retry_classifier(ImdsResponseRetryClassifier))
     }
 }
 
@@ -706,9 +706,9 @@ impl Display for TokenError {
 impl Error for TokenError {}
 
 #[derive(Clone)]
-struct ImdsErrorPolicy;
+struct ImdsResponseRetryClassifier;
 
-impl ImdsErrorPolicy {
+impl ImdsResponseRetryClassifier {
     fn classify(response: &operation::Response) -> RetryKind {
         let status = response.http().status();
         match status {
@@ -721,7 +721,7 @@ impl ImdsErrorPolicy {
     }
 }
 
-/// IMDS Retry Policy
+/// IMDS Response Retry Classifier
 ///
 /// Possible status codes:
 /// - 200 (OK)
@@ -730,12 +730,12 @@ impl ImdsErrorPolicy {
 /// - 403 (IMDS disabled): **Not Retryable**
 /// - 404 (Not found): **Not Retryable**
 /// - >=500 (server error): **Retryable**
-impl<T, E> ClassifyResponse<SdkSuccess<T>, SdkError<E>> for ImdsErrorPolicy {
-    fn classify(&self, response: Result<&SdkSuccess<T>, &SdkError<E>>) -> RetryKind {
+impl<T, E> ClassifyRetry<SdkSuccess<T>, SdkError<E>> for ImdsResponseRetryClassifier {
+    fn classify_retry(&self, response: Result<&SdkSuccess<T>, &SdkError<E>>) -> RetryKind {
         match response {
             Ok(_) => RetryKind::Unnecessary,
             Err(SdkError::ResponseError { raw, .. }) | Err(SdkError::ServiceError { raw, .. }) => {
-                ImdsErrorPolicy::classify(raw)
+                Self::classify(raw)
             }
             _ => RetryKind::UnretryableFailure,
         }
@@ -744,7 +744,7 @@ impl<T, E> ClassifyResponse<SdkSuccess<T>, SdkError<E>> for ImdsErrorPolicy {
 
 #[cfg(test)]
 pub(crate) mod test {
-    use crate::imds::client::{Client, EndpointMode, ImdsErrorPolicy};
+    use crate::imds::client::{Client, EndpointMode, ImdsResponseRetryClassifier};
     use crate::provider_config::ProviderConfig;
     use aws_smithy_async::rt::sleep::TokioSleep;
     use aws_smithy_client::erase::DynConnector;
@@ -1033,9 +1033,9 @@ pub(crate) mod test {
     /// Successful responses should classify as `RetryKind::Unnecessary`
     #[test]
     fn successful_response_properly_classified() {
-        use aws_smithy_http::retry::ClassifyResponse;
+        use aws_smithy_http::retry::ClassifyRetry;
 
-        let policy = ImdsErrorPolicy;
+        let classifier = ImdsResponseRetryClassifier;
         fn response_200() -> operation::Response {
             operation::Response::new(imds_response("").map(|_| SdkBody::empty()))
         }
@@ -1045,7 +1045,7 @@ pub(crate) mod test {
         };
         assert_eq!(
             RetryKind::Unnecessary,
-            policy.classify(Ok::<_, &SdkError<()>>(&success))
+            classifier.classify_retry(Ok::<_, &SdkError<()>>(&success))
         );
 
         // Emulate a failure to parse the response body (using an io error since it's easy to construct in a test)
@@ -1055,7 +1055,7 @@ pub(crate) mod test {
         };
         assert_eq!(
             RetryKind::UnretryableFailure,
-            policy.classify(Err::<&SdkSuccess<()>, _>(&failure))
+            classifier.classify_retry(Err::<&SdkSuccess<()>, _>(&failure))
         );
     }
 

aws/rust-runtime/aws-config/src/imds/client/token.rs

@@ -38,7 +38,7 @@ use aws_types::os_shim_internal::TimeSource;
 use http::{HeaderValue, Uri};
 
 use crate::cache::ExpiringCache;
-use crate::imds::client::{ImdsError, ImdsErrorPolicy, TokenError};
+use crate::imds::client::{ImdsError, ImdsResponseRetryClassifier, TokenError};
 
 /// Token Refresh Buffer
 ///
@@ -143,7 +143,7 @@ impl TokenMiddleware {
         request.properties_mut().insert(super::user_agent());
 
         let operation = Operation::new(request, self.token_parser.clone())
-            .with_retry_policy(ImdsErrorPolicy)
+            .with_retry_classifier(ImdsResponseRetryClassifier)
             .with_metadata(Metadata::new("get-token", "imds"));
         let response = self
             .client