Improve error messaging for auth scheme selection

CHANGELOG.next.toml

@@ -32,3 +32,15 @@ result of the compilation."""
 references = ["aws-sdk-rust#975", "smithy-rs#3269"]
 meta = { "breaking" = false, "tada" = true, "bug" = false }
 author = "jdisanti"
+
+[[aws-sdk-rust]]
+message = "Improve the error messages for when auth fails to select an auth scheme for a request."
+references = ["aws-sdk-rust#979", "smithy-rs#3277"]
+meta = { "breaking" = false, "tada" = false, "bug" = false }
+author = "jdisanti"
+
+[[smithy-rs]]
+message = "Improve the error messages for when auth fails to select an auth scheme for a request."
+references = ["smithy-rs#3277"]
+meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
+author = "jdisanti"

aws/sdk/integration-tests/dynamodb/tests/auth_scheme_error.rs

@@ -0,0 +1,32 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+use aws_sdk_dynamodb::config::Region;
+use aws_sdk_dynamodb::error::DisplayErrorContext;
+use aws_sdk_dynamodb::{Client, Config};
+use aws_smithy_runtime::client::http::test_util::capture_request;
+
+#[tokio::test]
+async fn auth_scheme_error() {
+    let (http_client, _) = capture_request(None);
+    let config = Config::builder()
+        .behavior_version_latest()
+        .http_client(http_client)
+        .region(Region::new("us-west-2"))
+        // intentionally omitting credentials_provider
+        .build();
+    let client = Client::from_conf(config);
+
+    let err = client
+        .list_tables()
+        .send()
+        .await
+        .expect_err("there is no credential provider, so this must fail");
+    let err = DisplayErrorContext(&err).to_string();
+    let expected = "\"sigv4\" wasn't an option because there was no identity resolver for it. Be sure to set an identity";
+    if !err.contains(expected) {
+        panic!("expected '{}'\nto contain '{}'", err, expected);
+    }
+}

rust-runtime/aws-smithy-runtime/src/client/orchestrator/auth.rs

@@ -20,18 +20,78 @@ use std::error::Error as StdError;
 use std::fmt;
 use tracing::trace;
 
+#[derive(Debug)]
+struct NoMatchingAuthSchemeError(ExploredList);
+
+impl fmt::Display for NoMatchingAuthSchemeError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let explored = &self.0;
+
+        // Use the information we have about the auth options that were explored to construct
+        // as helpful of an error message as possible.
+        if explored.items().count() == 0 {
+            return f
+                .write_str("no auth options are available. This is a bug. Please file an issue.");
+        }
+        if explored
+            .items()
+            .all(|explored| matches!(explored.result, ExploreResult::NoAuthScheme))
+        {
+            return f
+                .write_str("no auth schemes are registered. This is a bug. Please file an issue.");
+        }
+
+        let mut try_add_identity = false;
+        let mut likely_bug = false;
+        f.write_str("failed to select an auth scheme to sign the request with.")?;
+        for item in explored.items() {
+            write!(
+                f,
+                " \"{}\" wasn't an option because ",
+                item.scheme_id.as_str()
+            )?;
+            f.write_str(match item.result {
+                        ExploreResult::NoAuthScheme => {
+                            likely_bug = true;
+                            "no auth scheme was registered for it."
+                        },
+                        ExploreResult::NoIdentityResolver => {
+                            try_add_identity = true;
+                            "there was no identity resolver for it."
+                        }
+                        ExploreResult::MissingEndpointConfig => {
+                            likely_bug = true;
+                            "it has auth config in its endpoint config, but this scheme wasn't listed in that endpoint config."
+                        },
+                        ExploreResult::NotExplored => unreachable!(),
+                    })?;
+        }
+        if try_add_identity {
+            f.write_str(" Be sure to set an identity, such as credentials, auth token, or other identity type that is required for this service.")?;
+        } else if likely_bug {
+            f.write_str(" This is likely a bug.")?;
+        }
+        if explored.truncated {
+            f.write_str(" Note: there were other auth schemes that were evaluated that weren't listed here.")?;
+        }
+
+        Ok(())
+    }
+}
+
+impl StdError for NoMatchingAuthSchemeError {}
+
 #[derive(Debug)]
 enum AuthOrchestrationError {
-    NoMatchingAuthScheme,
+    MissingEndpointConfig,
     BadAuthSchemeEndpointConfig(Cow<'static, str>),
 }
 
 impl fmt::Display for AuthOrchestrationError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            Self::NoMatchingAuthScheme => f.write_str(
-                "no auth scheme matched auth scheme options. This is a bug. Please file an issue.",
-            ),
+            // This error is never bubbled up
+            Self::MissingEndpointConfig => f.write_str("missing endpoint config"),
             Self::BadAuthSchemeEndpointConfig(message) => f.write_str(message),
         }
     }
@@ -59,6 +119,8 @@ pub(super) async fn orchestrate_auth(
         "orchestrating auth",
     );
 
+    let mut explored = ExploredList::default();
+
     // Iterate over IDs of possibly-supported auth schemes
     for &scheme_id in options.as_ref() {
         // For each ID, try to resolve the corresponding auth scheme.
@@ -95,16 +157,21 @@ pub(super) async fn orchestrate_auth(
                         )?;
                         return Ok(());
                     }
-                    Err(AuthOrchestrationError::NoMatchingAuthScheme) => {
+                    Err(AuthOrchestrationError::MissingEndpointConfig) => {
+                        explored.push(scheme_id, ExploreResult::MissingEndpointConfig);
                         continue;
                     }
                     Err(other_err) => return Err(other_err.into()),
                 }
+            } else {
+                explored.push(scheme_id, ExploreResult::NoIdentityResolver);
             }
+        } else {
+            explored.push(scheme_id, ExploreResult::NoAuthScheme);
         }
     }
 
-    Err(AuthOrchestrationError::NoMatchingAuthScheme.into())
+    Err(NoMatchingAuthSchemeError(explored).into())
 }
 
 fn extract_endpoint_auth_scheme_config(
@@ -135,10 +202,66 @@ fn extract_endpoint_auth_scheme_config(
                 .and_then(Document::as_string);
             config_scheme_id == Some(scheme_id.as_str())
         })
-        .ok_or(AuthOrchestrationError::NoMatchingAuthScheme)?;
+        .ok_or(AuthOrchestrationError::MissingEndpointConfig)?;
     Ok(AuthSchemeEndpointConfig::from(Some(auth_scheme_config)))
 }
 
+#[derive(Debug)]
+enum ExploreResult {
+    NotExplored,
+    NoAuthScheme,
+    NoIdentityResolver,
+    MissingEndpointConfig,
+}
+
+/// Information about an evaluated auth option.
+/// This should be kept small so it can fit in an array on the stack.
+#[derive(Debug)]
+struct ExploredAuthOption {
+    scheme_id: AuthSchemeId,
+    result: ExploreResult,
+}
+impl Default for ExploredAuthOption {
+    fn default() -> Self {
+        Self {
+            scheme_id: AuthSchemeId::new(""),
+            result: ExploreResult::NotExplored,
+        }
+    }
+}
+
+const MAX_EXPLORED_LIST_LEN: usize = 8;
+
+/// Stack allocated list of explored auth options for error messaging
+#[derive(Default)]
+struct ExploredList {
+    items: [ExploredAuthOption; MAX_EXPLORED_LIST_LEN],
+    len: usize,
+    truncated: bool,
+}
+impl ExploredList {
+    fn items(&self) -> impl Iterator<Item = &ExploredAuthOption> {
+        self.items.iter().take(self.len)
+    }
+
+    fn push(&mut self, scheme_id: AuthSchemeId, result: ExploreResult) {
+        if self.len + 1 >= self.items.len() {
+            self.truncated = true;
+        } else {
+            self.items[self.len] = ExploredAuthOption { scheme_id, result };
+            self.len += 1;
+        }
+    }
+}
+impl fmt::Debug for ExploredList {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("ExploredList")
+            .field("items", &&self.items[0..self.len])
+            .field("truncated", &self.truncated)
+            .finish()
+    }
+}
+
 #[cfg(all(test, feature = "test-util"))]
 mod tests {
     use super::*;
@@ -481,4 +604,83 @@ mod tests {
                 .unwrap()
         );
     }
+
+    #[test]
+    fn friendly_error_messages() {
+        let err = NoMatchingAuthSchemeError(ExploredList::default());
+        assert_eq!(
+            "no auth options are available. This is a bug. Please file an issue.",
+            err.to_string()
+        );
+
+        let mut list = ExploredList::default();
+        list.push(
+            AuthSchemeId::new("SigV4"),
+            ExploreResult::NoIdentityResolver,
+        );
+        list.push(
+            AuthSchemeId::new("SigV4a"),
+            ExploreResult::NoIdentityResolver,
+        );
+        let err = NoMatchingAuthSchemeError(list);
+        assert_eq!(
+            "failed to select an auth scheme to sign the request with. \
+            \"SigV4\" wasn't an option because there was no identity resolver for it. \
+            \"SigV4a\" wasn't an option because there was no identity resolver for it. \
+            Be sure to set an identity, such as credentials, auth token, or other identity \
+            type that is required for this service.",
+            err.to_string()
+        );
+
+        // It should prioritize the suggestion to try an identity before saying it's a bug
+        let mut list = ExploredList::default();
+        list.push(
+            AuthSchemeId::new("SigV4"),
+            ExploreResult::NoIdentityResolver,
+        );
+        list.push(
+            AuthSchemeId::new("SigV4a"),
+            ExploreResult::MissingEndpointConfig,
+        );
+        let err = NoMatchingAuthSchemeError(list);
+        assert_eq!(
+            "failed to select an auth scheme to sign the request with. \
+            \"SigV4\" wasn't an option because there was no identity resolver for it. \
+            \"SigV4a\" wasn't an option because it has auth config in its endpoint config, \
+            but this scheme wasn't listed in that endpoint config. \
+            Be sure to set an identity, such as credentials, auth token, or other identity \
+            type that is required for this service.",
+            err.to_string()
+        );
+
+        // Otherwise, it should suggest it's a bug
+        let mut list = ExploredList::default();
+        list.push(
+            AuthSchemeId::new("SigV4a"),
+            ExploreResult::MissingEndpointConfig,
+        );
+        let err = NoMatchingAuthSchemeError(list);
+        assert_eq!(
+            "failed to select an auth scheme to sign the request with. \
+            \"SigV4a\" wasn't an option because it has auth config in its endpoint config, \
+            but this scheme wasn't listed in that endpoint config. \
+            This is likely a bug.",
+            err.to_string()
+        );
+
+        // Truncation should be indicated
+        let mut list = ExploredList::default();
+        for _ in 0..=MAX_EXPLORED_LIST_LEN {
+            list.push(
+                AuthSchemeId::new("dontcare"),
+                ExploreResult::MissingEndpointConfig,
+            );
+        }
+        let err = NoMatchingAuthSchemeError(list).to_string();
+        if !err.contains(
+            "Note: there were other auth schemes that were evaluated that weren't listed here",
+        ) {
+            panic!("The error should indicate that the explored list was truncated.");
+        }
+    }
 }