Implement and test retry classifier sorting

CHANGELOG.next.toml

@@ -46,3 +46,43 @@ message = "Added impl `Display` to Enums."
 references = ["smithy-rs#3336", "smithy-rs#3391"]
 meta = { "breaking" = false, "tada" = false, "bug" = false}
 author = "iampkmone"
+
+[[aws-sdk-rust]]
+message = """
+Retry classifiers will now be sorted by priority. This change only affects requests
+that are retried. Some requests that were previously been classified as transient
+errors may now be classified as throttling errors.
+
+If you were
+
+- configuring multiple custom retry classifiers
+- that would disagree on how to classify a response
+- that have differing priorities
+
+you may see a behavior change in that classification for the same response is now
+dependent on the classifier priority instead of the order in which the classifier
+was added.
+"""
+references = ["smithy-rs#3322"]
+meta = { "breaking" = false, "bug" = true, "tada" = false }
+author = "Velfi"
+
+[[smithy-rs]]
+message = """
+Retry classifiers will now be sorted by priority. This change only affects requests
+that are retried. Some requests that were previously been classified as transient
+errors may now be classified as throttling errors.
+
+If you were
+
+- configuring multiple custom retry classifiers
+- that would disagree on how to classify a response
+- that have differing priorities
+
+you may see a behavior change in that classification for the same response is now
+dependent on the classifier priority instead of the order in which the classifier
+was added.
+"""
+references = ["smithy-rs#3322"]
+meta = { "breaking" = false, "bug" = true, "tada" = false }
+author = "Velfi"

aws/rust-runtime/aws-runtime/src/retries/classifiers.rs

@@ -100,7 +100,7 @@ where
     }
 
     fn priority(&self) -> RetryClassifierPriority {
-        RetryClassifierPriority::with_lower_priority_than(
+        RetryClassifierPriority::run_before(
             RetryClassifierPriority::modeled_as_retryable_classifier(),
         )
     }

aws/sdk/integration-tests/Cargo.toml

@@ -1,6 +1,7 @@
 # Note: This workspace exists so that these tests can be run without having to build the entire SDK. When you run
 # `./gradlew -Paws.fullsdk=true :aws:sdk:assemble` these tests are copied into their respective Service crates.
 [workspace]
+resolver = "2"
 members = [
     "dynamodb",
     "ec2",

aws/sdk/integration-tests/s3/tests/retry-classifier-customization.rs

@@ -8,6 +8,7 @@ use aws_sdk_s3::config::retry::{ClassifyRetry, RetryAction, RetryConfig};
 use aws_sdk_s3::config::SharedAsyncSleep;
 use aws_smithy_async::rt::sleep::TokioSleep;
 use aws_smithy_runtime::client::http::test_util::{ReplayEvent, StaticReplayClient};
+use aws_smithy_runtime_api::client::retries::classifiers::RetryClassifierPriority;
 use aws_smithy_types::body::SdkBody;
 use std::sync::{Arc, Mutex};
 
@@ -133,3 +134,141 @@ async fn test_retry_classifier_customization_for_operation() {
     // ensure our custom retry classifier was called at least once.
     assert_ne!(customization_test_classifier.counter(), 0);
 }
+
+#[derive(Debug, Clone)]
+struct OrderingTestClassifier {
+    counter: Arc<Mutex<u8>>,
+    name: &'static str,
+    priority: RetryClassifierPriority,
+}
+
+impl OrderingTestClassifier {
+    pub fn new(name: &'static str, priority: RetryClassifierPriority) -> Self {
+        Self {
+            counter: Arc::new(Mutex::new(0u8)),
+            name,
+            priority,
+        }
+    }
+
+    pub fn counter(&self) -> u8 {
+        *self.counter.lock().unwrap()
+    }
+}
+
+impl ClassifyRetry for OrderingTestClassifier {
+    fn classify_retry(&self, _ctx: &InterceptorContext) -> RetryAction {
+        tracing::debug!("Running classifier {}", self.name);
+        *self.counter.lock().unwrap() += 1;
+        RetryAction::NoActionIndicated
+    }
+
+    fn name(&self) -> &'static str {
+        "Ordering Test Retry Classifier"
+    }
+
+    fn priority(&self) -> RetryClassifierPriority {
+        self.priority.clone()
+    }
+}
+
+#[tracing_test::traced_test]
+#[tokio::test]
+async fn test_retry_classifier_customization_ordering() {
+    let http_client = StaticReplayClient::new(vec![
+        ReplayEvent::new(req(), err()),
+        ReplayEvent::new(req(), ok()),
+    ]);
+
+    let classifier_a = OrderingTestClassifier::new("6", RetryClassifierPriority::default());
+    let classifier_b = OrderingTestClassifier::new(
+        "5",
+        RetryClassifierPriority::run_before(classifier_a.priority()),
+    );
+    let classifier_c = OrderingTestClassifier::new(
+        "4",
+        RetryClassifierPriority::run_before(classifier_b.priority()),
+    );
+    let classifier_d = OrderingTestClassifier::new(
+        "3",
+        RetryClassifierPriority::run_before(classifier_c.priority()),
+    );
+    let classifier_e = OrderingTestClassifier::new(
+        "2",
+        RetryClassifierPriority::run_before(classifier_d.priority()),
+    );
+    let classifier_f = OrderingTestClassifier::new(
+        "1",
+        RetryClassifierPriority::run_before(classifier_e.priority()),
+    );
+
+    let config = aws_sdk_s3::Config::builder()
+        .with_test_defaults()
+        .sleep_impl(SharedAsyncSleep::new(TokioSleep::new()))
+        .http_client(http_client)
+        .retry_config(RetryConfig::standard())
+        .retry_classifier(classifier_d.clone())
+        .retry_classifier(classifier_b.clone())
+        .retry_classifier(classifier_f.clone())
+        .build();
+
+    let client = aws_sdk_s3::Client::from_conf(config);
+    let _ = client
+        .get_object()
+        .bucket("bucket")
+        .key("key")
+        .customize()
+        .config_override(
+            aws_sdk_s3::config::Config::builder()
+                .retry_classifier(classifier_c.clone())
+                .retry_classifier(classifier_a.clone())
+                .retry_classifier(classifier_e.clone()),
+        )
+        .send()
+        .await
+        .expect_err("fails without attempting a retry");
+
+    // ensure our classifiers were each called at least once.
+    assert_ne!(classifier_a.counter(), 0, "classifier_a was never called");
+    assert_ne!(classifier_b.counter(), 0, "classifier_b was never called");
+    assert_ne!(classifier_c.counter(), 0, "classifier_c was never called");
+    assert_ne!(classifier_d.counter(), 0, "classifier_d was never called");
+    assert_ne!(classifier_e.counter(), 0, "classifier_e was never called");
+    assert_ne!(classifier_f.counter(), 0, "classifier_f was never called");
+
+    // ensure the classifiers were called in the correct order.
+    logs_assert(|lines: &[&str]| {
+        let mut found_log_a = false;
+        let mut line_iter = lines.iter();
+
+        while found_log_a == false {
+            match line_iter.next() {
+                Some(&line) => {
+                    if line.contains("Running classifier 1") {
+                        found_log_a = true;
+                    }
+                }
+                None => {
+                    return Err("Couldn't find log line for classifier 1".to_owned());
+                }
+            }
+        }
+
+        for i in 2..=6 {
+            match line_iter.next() {
+                Some(&line) => {
+                    if line.contains(&format!("Running classifier {i}")) {
+                        // pass
+                    } else {
+                        return Err(format!("Expected to find log line for classifier {i} after {} but found '{line}'", i - 1));
+                    }
+                }
+                None => {
+                    return Err(format!("Logs ended earlier than expected ({i})"));
+                }
+            }
+        }
+
+        Ok(())
+    });
+}