From dff2b8c7f088965663e975df26d714c2b4d76702 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 3 Oct 2022 15:42:09 -0700 Subject: [PATCH 1/3] Add `OpaqueError` and `DisplayErrorContext` to `aws-smithy-types` --- rust-runtime/aws-smithy-types/src/error.rs | 148 ++++++++++++++++++ .../src/error/display_context.rs | 90 +++++++++++ .../aws-smithy-types/src/error/opaque.rs | 69 ++++++++ rust-runtime/aws-smithy-types/src/lib.rs | 146 +---------------- 4 files changed, 309 insertions(+), 144 deletions(-) create mode 100644 rust-runtime/aws-smithy-types/src/error.rs create mode 100644 rust-runtime/aws-smithy-types/src/error/display_context.rs create mode 100644 rust-runtime/aws-smithy-types/src/error/opaque.rs diff --git a/rust-runtime/aws-smithy-types/src/error.rs b/rust-runtime/aws-smithy-types/src/error.rs new file mode 100644 index 0000000000..21e4fe068b --- /dev/null +++ b/rust-runtime/aws-smithy-types/src/error.rs @@ -0,0 +1,148 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Generic errors for Smithy codegen + +use crate::retry::{ErrorKind, ProvideErrorKind}; +use std::collections::HashMap; +use std::fmt; +use std::fmt::{Display, Formatter}; + +pub mod display_context; +pub mod opaque; + +/// Generic Error type +/// +/// For many services, Errors are modeled. However, many services only partially model errors or don't +/// model errors at all. In these cases, the SDK will return this generic error type to expose the +/// `code`, `message` and `request_id`. +#[derive(Debug, Eq, PartialEq, Default, Clone)] +pub struct Error { + code: Option, + message: Option, + request_id: Option, + extras: HashMap<&'static str, String>, +} + +/// Builder for [`Error`]. +#[derive(Debug, Default)] +pub struct Builder { + inner: Error, +} + +impl Builder { + /// Sets the error message. + pub fn message(&mut self, message: impl Into) -> &mut Self { + self.inner.message = Some(message.into()); + self + } + + /// Sets the error code. + pub fn code(&mut self, code: impl Into) -> &mut Self { + self.inner.code = Some(code.into()); + self + } + + /// Sets the request ID the error happened for. + pub fn request_id(&mut self, request_id: impl Into) -> &mut Self { + self.inner.request_id = Some(request_id.into()); + self + } + + /// Set a custom field on the error metadata + /// + /// Typically, these will be accessed with an extension trait: + /// ```rust + /// use aws_smithy_types::Error; + /// const HOST_ID: &str = "host_id"; + /// trait S3ErrorExt { + /// fn extended_request_id(&self) -> Option<&str>; + /// } + /// + /// impl S3ErrorExt for Error { + /// fn extended_request_id(&self) -> Option<&str> { + /// self.extra(HOST_ID) + /// } + /// } + /// + /// fn main() { + /// // Extension trait must be brought into scope + /// use S3ErrorExt; + /// let sdk_response: Result<(), Error> = Err(Error::builder().custom(HOST_ID, "x-1234").build()); + /// if let Err(err) = sdk_response { + /// println!("request id: {:?}, extended request id: {:?}", err.request_id(), err.extended_request_id()); + /// } + /// } + /// ``` + pub fn custom(&mut self, key: &'static str, value: impl Into) -> &mut Self { + self.inner.extras.insert(key, value.into()); + self + } + + /// Creates the error. + pub fn build(&mut self) -> Error { + std::mem::take(&mut self.inner) + } +} + +impl Error { + /// Returns the error code. + pub fn code(&self) -> Option<&str> { + self.code.as_deref() + } + /// Returns the error message. + pub fn message(&self) -> Option<&str> { + self.message.as_deref() + } + /// Returns the request ID the error occurred for, if it's available. + pub fn request_id(&self) -> Option<&str> { + self.request_id.as_deref() + } + /// Returns additional information about the error if it's present. + pub fn extra(&self, key: &'static str) -> Option<&str> { + self.extras.get(key).map(|k| k.as_str()) + } + + /// Creates an `Error` builder. + pub fn builder() -> Builder { + Builder::default() + } + + /// Converts an `Error` into a builder. + pub fn into_builder(self) -> Builder { + Builder { inner: self } + } +} + +impl ProvideErrorKind for Error { + fn retryable_error_kind(&self) -> Option { + None + } + + fn code(&self) -> Option<&str> { + Error::code(self) + } +} + +impl Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + let mut fmt = f.debug_struct("Error"); + if let Some(code) = &self.code { + fmt.field("code", code); + } + if let Some(message) = &self.message { + fmt.field("message", message); + } + if let Some(req_id) = &self.request_id { + fmt.field("request_id", req_id); + } + for (k, v) in &self.extras { + fmt.field(k, &v); + } + fmt.finish() + } +} + +impl std::error::Error for Error {} diff --git a/rust-runtime/aws-smithy-types/src/error/display_context.rs b/rust-runtime/aws-smithy-types/src/error/display_context.rs new file mode 100644 index 0000000000..cc33094466 --- /dev/null +++ b/rust-runtime/aws-smithy-types/src/error/display_context.rs @@ -0,0 +1,90 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Error wrapper that displays error context + +use std::error::Error; +use std::fmt; + +/// Provides a `Display` impl for an `Error` that outputs the full error context +/// +/// This is useful for emitting errors with `tracing` in cases where the error +/// is not returned back to the customer. +#[derive(Debug)] +pub struct DisplayErrorContext(pub E); + +impl fmt::Display for DisplayErrorContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write_err(f, &self.0) + } +} + +fn write_err(f: &mut fmt::Formatter<'_>, err: &dyn Error) -> fmt::Result { + write!(f, "{}", err)?; + if let Some(source) = err.source() { + write!(f, ": ")?; + write_err(f, source)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::error::Error; + use std::fmt; + + #[derive(Debug)] + struct TestError { + what: &'static str, + source: Option>, + } + + impl fmt::Display for TestError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.what) + } + } + + impl Error for TestError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + self.source.as_deref() + } + } + + #[test] + fn no_sources() { + assert_eq!( + "test", + format!( + "{}", + DisplayErrorContext(TestError { + what: "test", + source: None + }) + ) + ); + } + + #[test] + fn sources() { + assert_eq!( + "foo: bar: baz", + format!( + "{}", + DisplayErrorContext(TestError { + what: "foo", + source: Some(Box::new(TestError { + what: "bar", + source: Some(Box::new(TestError { + what: "baz", + source: None + })) + }) as Box<_>) + }) + ) + ); + } +} diff --git a/rust-runtime/aws-smithy-types/src/error/opaque.rs b/rust-runtime/aws-smithy-types/src/error/opaque.rs new file mode 100644 index 0000000000..e21ad756e8 --- /dev/null +++ b/rust-runtime/aws-smithy-types/src/error/opaque.rs @@ -0,0 +1,69 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Opaque error type used by generated code and runtime crates + +use crate::error::display_context::DisplayErrorContext; +use std::error::Error; +use std::fmt; + +/// Opaque error type +/// +/// Provides a `Display` implementation to show the formatted contents of the inner error type, +/// but does not allow downcasting of the inner error type. +#[derive(Debug)] +pub struct OpaqueError(Box); + +impl OpaqueError { + /// Creates a new `OpaqueError` around the given `inner` error + pub fn new(inner: impl Into>) -> Self { + Self(inner.into()) + } +} + +impl fmt::Display for OpaqueError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + DisplayErrorContext(self.0.as_ref()).fmt(f) + } +} + +impl Error for OpaqueError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + // Intentionally don't reveal the source since this is an opaque error type + None + } +} + +impl From for OpaqueError { + fn from(string: String) -> Self { + Self::new(string) + } +} + +impl From<&str> for OpaqueError { + fn from(string: &str) -> Self { + Self::new(string) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Error as IOError, ErrorKind as IOErrorKind}; + + #[test] + fn send_sync() { + fn verify_send_sync(_thing: Option) {} + verify_send_sync::>(None); + } + + #[test] + fn source() { + use std::error::Error as _; + let err = OpaqueError::new(IOError::new(IOErrorKind::Other, "test")); + assert!(err.source().is_none()); + assert_eq!("test", format!("{}", err)); + } +} diff --git a/rust-runtime/aws-smithy-types/src/lib.rs b/rust-runtime/aws-smithy-types/src/lib.rs index 3e7b599e15..c40bbe2074 100644 --- a/rust-runtime/aws-smithy-types/src/lib.rs +++ b/rust-runtime/aws-smithy-types/src/lib.rs @@ -18,11 +18,13 @@ use std::collections::HashMap; pub mod base64; pub mod date_time; pub mod endpoint; +pub mod error; pub mod primitive; pub mod retry; pub mod timeout; pub use crate::date_time::DateTime; +pub use error::Error; /// Binary Blob Type /// @@ -553,147 +555,3 @@ mod number { ); } } - -pub use error::Error; - -/// Generic errors for Smithy codegen -pub mod error { - use crate::retry::{ErrorKind, ProvideErrorKind}; - use std::collections::HashMap; - use std::fmt; - use std::fmt::{Display, Formatter}; - - /// Generic Error type - /// - /// For many services, Errors are modeled. However, many services only partially model errors or don't - /// model errors at all. In these cases, the SDK will return this generic error type to expose the - /// `code`, `message` and `request_id`. - #[derive(Debug, Eq, PartialEq, Default, Clone)] - pub struct Error { - code: Option, - message: Option, - request_id: Option, - extras: HashMap<&'static str, String>, - } - - /// Builder for [`Error`]. - #[derive(Debug, Default)] - pub struct Builder { - inner: Error, - } - - impl Builder { - /// Sets the error message. - pub fn message(&mut self, message: impl Into) -> &mut Self { - self.inner.message = Some(message.into()); - self - } - - /// Sets the error code. - pub fn code(&mut self, code: impl Into) -> &mut Self { - self.inner.code = Some(code.into()); - self - } - - /// Sets the request ID the error happened for. - pub fn request_id(&mut self, request_id: impl Into) -> &mut Self { - self.inner.request_id = Some(request_id.into()); - self - } - - /// Set a custom field on the error metadata - /// - /// Typically, these will be accessed with an extension trait: - /// ```rust - /// use aws_smithy_types::Error; - /// const HOST_ID: &str = "host_id"; - /// trait S3ErrorExt { - /// fn extended_request_id(&self) -> Option<&str>; - /// } - /// - /// impl S3ErrorExt for Error { - /// fn extended_request_id(&self) -> Option<&str> { - /// self.extra(HOST_ID) - /// } - /// } - /// - /// fn main() { - /// // Extension trait must be brought into scope - /// use S3ErrorExt; - /// let sdk_response: Result<(), Error> = Err(Error::builder().custom(HOST_ID, "x-1234").build()); - /// if let Err(err) = sdk_response { - /// println!("request id: {:?}, extended request id: {:?}", err.request_id(), err.extended_request_id()); - /// } - /// } - /// ``` - pub fn custom(&mut self, key: &'static str, value: impl Into) -> &mut Self { - self.inner.extras.insert(key, value.into()); - self - } - - /// Creates the error. - pub fn build(&mut self) -> Error { - std::mem::take(&mut self.inner) - } - } - - impl Error { - /// Returns the error code. - pub fn code(&self) -> Option<&str> { - self.code.as_deref() - } - /// Returns the error message. - pub fn message(&self) -> Option<&str> { - self.message.as_deref() - } - /// Returns the request ID the error occurred for, if it's available. - pub fn request_id(&self) -> Option<&str> { - self.request_id.as_deref() - } - /// Returns additional information about the error if it's present. - pub fn extra(&self, key: &'static str) -> Option<&str> { - self.extras.get(key).map(|k| k.as_str()) - } - - /// Creates an `Error` builder. - pub fn builder() -> Builder { - Builder::default() - } - - /// Converts an `Error` into a builder. - pub fn into_builder(self) -> Builder { - Builder { inner: self } - } - } - - impl ProvideErrorKind for Error { - fn retryable_error_kind(&self) -> Option { - None - } - - fn code(&self) -> Option<&str> { - Error::code(self) - } - } - - impl Display for Error { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let mut fmt = f.debug_struct("Error"); - if let Some(code) = &self.code { - fmt.field("code", code); - } - if let Some(message) = &self.message { - fmt.field("message", message); - } - if let Some(req_id) = &self.request_id { - fmt.field("request_id", req_id); - } - for (k, v) in &self.extras { - fmt.field(k, &v); - } - fmt.finish() - } - } - - impl std::error::Error for Error {} -} From 615e30a4de5a2cf63b6cbff43112d6c132394095 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 4 Oct 2022 12:53:03 -0700 Subject: [PATCH 2/3] Revamp `CredentialsError` --- .../aws-types/src/credentials/provider.rs | 76 ++++++++++--------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/aws/rust-runtime/aws-types/src/credentials/provider.rs b/aws/rust-runtime/aws-types/src/credentials/provider.rs index 04a425680b..753ba71abb 100644 --- a/aws/rust-runtime/aws-types/src/credentials/provider.rs +++ b/aws/rust-runtime/aws-types/src/credentials/provider.rs @@ -4,6 +4,7 @@ */ use crate::Credentials; +use aws_smithy_types::error::opaque::OpaqueError; use std::error::Error; use std::fmt::{self, Debug, Display, Formatter}; use std::sync::Arc; @@ -16,13 +17,16 @@ pub enum CredentialsError { /// No credentials were available for this provider #[non_exhaustive] CredentialsNotLoaded { - /// Underlying cause of the error. - context: Box, + /// Cause of the error + source: OpaqueError, }, /// Loading credentials from this provider exceeded the maximum allowed duration #[non_exhaustive] - ProviderTimedOut(Duration), + ProviderTimedOut { + /// The timeout duration + after: Duration, + }, /// The provider was given an invalid configuration /// @@ -31,8 +35,8 @@ pub enum CredentialsError { /// - assume role profile that forms an infinite loop #[non_exhaustive] InvalidConfiguration { - /// Underlying cause of the error. - cause: Box, + /// Cause of the error + source: OpaqueError, }, /// The provider experienced an error during credential resolution @@ -41,8 +45,8 @@ pub enum CredentialsError { /// read a configuration file. #[non_exhaustive] ProviderError { - /// Underlying cause of the error. - cause: Box, + /// Cause of the error + source: OpaqueError, }, /// An unexpected error occurred during credential resolution @@ -53,8 +57,8 @@ pub enum CredentialsError { /// - A provider returns data that is missing required fields #[non_exhaustive] Unhandled { - /// Underlying cause of the error. - cause: Box, + /// Cause of the error + source: OpaqueError, }, } @@ -64,9 +68,9 @@ impl CredentialsError { /// This error indicates the credentials provider was not enable or no configuration was set. /// This contrasts with [`invalid_configuration`](CredentialsError::InvalidConfiguration), indicating /// that the provider was configured in some way, but certain settings were invalid. - pub fn not_loaded(context: impl Into>) -> Self { + pub fn not_loaded(source: impl Into>) -> Self { CredentialsError::CredentialsNotLoaded { - context: context.into(), + source: OpaqueError::new(source), } } @@ -74,9 +78,9 @@ impl CredentialsError { /// /// Unhandled errors should not occur during normal operation and should be reserved for exceptional /// cases, such as a JSON API returning an output that was not parseable as JSON. - pub fn unhandled(cause: impl Into>) -> Self { + pub fn unhandled(source: impl Into>) -> Self { Self::Unhandled { - cause: cause.into(), + source: OpaqueError::new(source), } } @@ -84,48 +88,46 @@ impl CredentialsError { /// /// Provider errors may occur during normal use of a credentials provider, e.g. a 503 when /// retrieving credentials from IMDS. - pub fn provider_error(cause: impl Into>) -> Self { + pub fn provider_error(source: impl Into>) -> Self { Self::ProviderError { - cause: cause.into(), + source: OpaqueError::new(source), } } /// The provided configuration for a provider was invalid - pub fn invalid_configuration(cause: impl Into>) -> Self { + pub fn invalid_configuration( + source: impl Into>, + ) -> Self { Self::InvalidConfiguration { - cause: cause.into(), + source: OpaqueError::new(source), } } /// The credentials provider did not provide credentials within an allotted duration - pub fn provider_timed_out(context: Duration) -> Self { - Self::ProviderTimedOut(context) + pub fn provider_timed_out(after: Duration) -> Self { + Self::ProviderTimedOut { after } } } impl Display for CredentialsError { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { - CredentialsError::CredentialsNotLoaded { context } => { - write!(f, "The credential provider was not enabled: {}", context) + CredentialsError::CredentialsNotLoaded { source: _ } => { + write!(f, "The credential provider was not enabled") } - CredentialsError::ProviderTimedOut(d) => write!( + CredentialsError::ProviderTimedOut { after } => write!( f, "Credentials provider timed out after {} seconds", - d.as_secs() + after.as_secs() ), - CredentialsError::Unhandled { cause } => { - write!(f, "Unexpected credentials error: {}", cause) + CredentialsError::Unhandled { source: _ } => { + write!(f, "Unexpected credentials error") } - CredentialsError::InvalidConfiguration { cause } => { - write!( - f, - "The credentials provider was not properly configured: {}", - cause - ) + CredentialsError::InvalidConfiguration { source: _ } => { + write!(f, "The credentials provider was not properly configured") } - CredentialsError::ProviderError { cause } => { - write!(f, "An error occurred while loading credentials: {}", cause) + CredentialsError::ProviderError { source: _ } => { + write!(f, "An error occurred while loading credentials") } } } @@ -134,10 +136,10 @@ impl Display for CredentialsError { impl Error for CredentialsError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { - CredentialsError::Unhandled { cause } - | CredentialsError::ProviderError { cause } - | CredentialsError::InvalidConfiguration { cause } => Some(cause.as_ref() as _), - CredentialsError::CredentialsNotLoaded { context } => Some(context.as_ref() as _), + CredentialsError::Unhandled { source } + | CredentialsError::ProviderError { source } + | CredentialsError::InvalidConfiguration { source } + | CredentialsError::CredentialsNotLoaded { source } => Some(source), _ => None, } } From 24776c3c85c082c1a21ccaea7fd9be957a8c3c4c Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 4 Oct 2022 12:53:52 -0700 Subject: [PATCH 3/3] Revamp errors in `aws-config` --- .../aws-config/external-types.toml | 3 - .../aws-config/src/credential_process.rs | 7 +- aws/rust-runtime/aws-config/src/ecs.rs | 101 +++++---- .../aws-config/src/imds/client.rs | 195 ++++++++++++------ .../aws-config/src/imds/client/token.rs | 2 +- .../aws-config/src/imds/credentials.rs | 9 +- .../aws-config/src/imds/region.rs | 5 +- .../aws-config/src/json_credentials.rs | 34 +-- .../aws-config/src/meta/credentials/chain.rs | 13 +- .../aws-config/src/profile/credentials.rs | 29 ++- .../aws-config/src/profile/parser/source.rs | 2 +- aws/rust-runtime/aws-config/src/sso.rs | 34 ++- aws/rust-runtime/aws-config/src/test_case.rs | 8 +- .../aws-config/src/web_identity_token.rs | 3 +- 14 files changed, 270 insertions(+), 175 deletions(-) diff --git a/aws/rust-runtime/aws-config/external-types.toml b/aws/rust-runtime/aws-config/external-types.toml index 94b43e4d03..1111a66ee0 100644 --- a/aws/rust-runtime/aws-config/external-types.toml +++ b/aws/rust-runtime/aws-config/external-types.toml @@ -21,9 +21,6 @@ allowed_external_types = [ "http::uri::Uri", "tower_service::Service", - # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if `InvalidUri` should be exposed - "http::uri::InvalidUri", - # TODO(https://github.com/awslabs/smithy-rs/issues/1193): Decide if the following should be exposed "hyper::client::connect::Connection", "tokio::io::async_read::AsyncRead", diff --git a/aws/rust-runtime/aws-config/src/credential_process.rs b/aws/rust-runtime/aws-config/src/credential_process.rs index ab4727b309..a8577a0014 100644 --- a/aws/rust-runtime/aws-config/src/credential_process.rs +++ b/aws/rust-runtime/aws-config/src/credential_process.rs @@ -7,6 +7,7 @@ use crate::json_credentials::{json_parse_loop, InvalidJsonCredentials, RefreshableCredentials}; use aws_smithy_json::deserialize::Token; +use aws_smithy_types::error::opaque::OpaqueError; use aws_types::credentials::{future, CredentialsError, ProvideCredentials}; use aws_types::{credentials, Credentials}; use std::fmt; @@ -197,7 +198,7 @@ pub(crate) fn parse_credential_process_json_credentials( version = Some(i32::try_from(*value).map_err(|err| { InvalidJsonCredentials::InvalidField { field: "Version", - err: err.into(), + source: OpaqueError::new(err), } })?); } @@ -227,7 +228,7 @@ pub(crate) fn parse_credential_process_json_credentials( Some(version) => { return Err(InvalidJsonCredentials::InvalidField { field: "version", - err: format!("unknown version number: {}", version).into(), + source: format!("unknown version number: {}", version).into(), }) } } @@ -241,7 +242,7 @@ pub(crate) fn parse_credential_process_json_credentials( SystemTime::try_from(OffsetDateTime::parse(&expiration, &Rfc3339).map_err(|err| { InvalidJsonCredentials::InvalidField { field: "Expiration", - err: err.into(), + source: OpaqueError::new(err), } })?) .map_err(|_| { diff --git a/aws/rust-runtime/aws-config/src/ecs.rs b/aws/rust-runtime/aws-config/src/ecs.rs index 45a4a16992..a1e531d45b 100644 --- a/aws/rust-runtime/aws-config/src/ecs.rs +++ b/aws/rust-runtime/aws-config/src/ecs.rs @@ -53,9 +53,10 @@ use std::net::IpAddr; use aws_smithy_client::erase::boxclone::BoxCloneService; use aws_smithy_http::endpoint::Endpoint; +use aws_smithy_types::error::opaque::OpaqueError; use aws_types::credentials; use aws_types::credentials::{future, CredentialsError, ProvideCredentials}; -use http::uri::{InvalidUri, Scheme}; +use http::uri::Scheme; use http::{HeaderValue, Uri}; use tower::{Service, ServiceExt}; @@ -63,7 +64,6 @@ use crate::http_credential_provider::HttpCredentialProvider; use crate::provider_config::ProviderConfig; use aws_smithy_client::http_connector::ConnectorSettings; use aws_types::os_shim_internal::Env; -use http::header::InvalidHeaderValue; use std::time::Duration; use tokio::sync::OnceCell; @@ -100,7 +100,7 @@ impl EcsCredentialsProvider { Some(auth) => Some(HeaderValue::from_str(&auth).map_err(|err| { tracing::warn!(token = %auth, "invalid auth token"); CredentialsError::invalid_configuration(EcsConfigurationErr::InvalidAuthToken { - err, + source: OpaqueError::new(err), value: auth, }) })?), @@ -152,7 +152,10 @@ impl Provider { let mut dns = dns.or_else(tokio_dns); validate_full_uri(&full_uri, dns.as_mut()) .await - .map_err(|err| EcsConfigurationErr::InvalidFullUri { err, uri: full_uri }) + .map_err(|err| EcsConfigurationErr::InvalidFullUri { + source: OpaqueError::new(err), + uri: full_uri, + }) } else { Err(EcsConfigurationErr::NotConfigured) } @@ -184,7 +187,7 @@ impl Provider { Err(invalid_uri) => { tracing::warn!(uri = ?invalid_uri, "invalid URI loaded from environment"); return Err(EcsConfigurationErr::InvalidRelativeUri { - err: invalid_uri, + source: OpaqueError::new(invalid_uri), uri: relative_uri, }); } @@ -197,40 +200,28 @@ impl Provider { #[derive(Debug)] enum EcsConfigurationErr { - InvalidRelativeUri { - err: InvalidUri, - uri: String, - }, - InvalidFullUri { - err: InvalidFullUriError, - uri: String, - }, - InvalidAuthToken { - err: InvalidHeaderValue, - value: String, - }, + InvalidRelativeUri { source: OpaqueError, uri: String }, + InvalidFullUri { source: OpaqueError, uri: String }, + InvalidAuthToken { source: OpaqueError, value: String }, NotConfigured, } impl Display for EcsConfigurationErr { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - EcsConfigurationErr::InvalidRelativeUri { err, uri } => write!( - f, - "invalid relative URI for ECS provider ({}): {}", - err, uri - ), - EcsConfigurationErr::InvalidFullUri { err, uri } => { - write!(f, "invalid full URI for ECS provider ({}): {}", err, uri) + EcsConfigurationErr::InvalidRelativeUri { source: _, uri } => { + write!(f, "invalid relative URI ({uri}) for ECS provider") + } + EcsConfigurationErr::InvalidFullUri { source: _, uri } => { + write!(f, "invalid full URI ({uri}) for ECS provider") } EcsConfigurationErr::NotConfigured => write!( f, "No environment variables were set to configure ECS provider" ), - EcsConfigurationErr::InvalidAuthToken { err, value } => write!( + EcsConfigurationErr::InvalidAuthToken { source: _, value } => write!( f, - "`{}` could not be used as a header value for the auth token. {}", - value, err + "`{value}` could not be used as a header value for the auth token" ), } } @@ -239,9 +230,10 @@ impl Display for EcsConfigurationErr { impl Error for EcsConfigurationErr { fn source(&self) -> Option<&(dyn Error + 'static)> { match &self { - EcsConfigurationErr::InvalidRelativeUri { err, .. } => Some(err), - EcsConfigurationErr::InvalidFullUri { err, .. } => Some(err), - _ => None, + Self::InvalidRelativeUri { source, .. } => Some(source), + Self::InvalidFullUri { source, .. } => Some(source), + Self::InvalidAuthToken { source, .. } => Some(source), + Self::NotConfigured => None, } } } @@ -310,7 +302,10 @@ impl Builder { pub enum InvalidFullUriError { /// The provided URI could not be parsed as a URI #[non_exhaustive] - InvalidUri(InvalidUri), + InvalidUri { + /// Cause of the error + source: OpaqueError, + }, /// No Dns service was provided #[non_exhaustive] @@ -325,25 +320,42 @@ pub enum InvalidFullUriError { NotLoopback, /// DNS lookup failed when attempting to resolve the host to an IP Address for validation. - DnsLookupFailed(io::Error), + #[non_exhaustive] + DnsLookupFailed { + /// Cause of the error + source: OpaqueError, + }, +} + +impl InvalidFullUriError { + fn invalid_uri(source: impl Into>) -> Self { + Self::InvalidUri { + source: OpaqueError::new(source), + } + } + + fn dns_lookup_failed(source: impl Into>) -> Self { + Self::DnsLookupFailed { + source: OpaqueError::new(source), + } + } } impl Display for InvalidFullUriError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - InvalidFullUriError::InvalidUri(err) => write!(f, "URI was invalid: {}", err), - InvalidFullUriError::MissingHost => write!(f, "URI did not specify a host"), - InvalidFullUriError::NotLoopback => { + Self::InvalidUri { source: _ } => write!(f, "invalid URI"), + Self::MissingHost => write!(f, "URI did not specify a host"), + Self::NotLoopback => { write!(f, "URI did not refer to the loopback interface") } - InvalidFullUriError::DnsLookupFailed(err) => { + Self::DnsLookupFailed { source: _ } => { write!( f, - "failed to perform DNS lookup while validating URI: {}", - err + "failed to perform DNS lookup while validating URI", ) } - InvalidFullUriError::NoDnsService => write!(f, "No DNS service was provided. Enable `rt-tokio` or provide a `dns` service to the builder.") + Self::NoDnsService => write!(f, "No DNS service was provided. Enable `rt-tokio` or provide a `dns` service to the builder.") } } } @@ -351,9 +363,8 @@ impl Display for InvalidFullUriError { impl Error for InvalidFullUriError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { - InvalidFullUriError::InvalidUri(err) => Some(err), - InvalidFullUriError::DnsLookupFailed(err) => Some(err), - _ => None, + Self::InvalidUri { source, .. } | Self::DnsLookupFailed { source, .. } => Some(source), + Self::NoDnsService | Self::MissingHost | Self::NotLoopback => None, } } } @@ -373,7 +384,7 @@ async fn validate_full_uri( ) -> Result { let uri = uri .parse::() - .map_err(InvalidFullUriError::InvalidUri)?; + .map_err(InvalidFullUriError::invalid_uri)?; if uri.scheme() == Some(&Scheme::HTTPS) { return Ok(uri); } @@ -383,10 +394,10 @@ async fn validate_full_uri( Ok(addr) => addr.is_loopback(), Err(_domain_name) => { let dns = dns.ok_or(InvalidFullUriError::NoDnsService)?; - dns.ready().await.map_err(InvalidFullUriError::DnsLookupFailed)? + dns.ready().await.map_err(InvalidFullUriError::dns_lookup_failed)? .call(host.to_owned()) .await - .map_err(InvalidFullUriError::DnsLookupFailed)? + .map_err(InvalidFullUriError::dns_lookup_failed)? .iter() .all(|addr| { if !addr.is_loopback() { diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 10e6d419ab..e60023bcd9 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -7,15 +7,14 @@ //! //! Client for direct access to IMDSv2. -use std::borrow::Cow; -use std::convert::TryFrom; -use std::error::Error; -use std::fmt::{Display, Formatter}; -use std::str::FromStr; -use std::sync::Arc; -use std::time::Duration; - +use crate::connector::expect_connector; +use crate::imds::client::token::TokenMiddleware; +use crate::profile::credentials::ProfileFileError; +use crate::provider_config::ProviderConfig; +use crate::{profile, PKG_VERSION}; use aws_http::user_agent::{ApiMetadata, AwsUserAgent, UserAgentStage}; +use aws_sdk_sso::config::timeout::TimeoutConfig; +use aws_smithy_client::http_connector::ConnectorSettings; use aws_smithy_client::{erase::DynConnector, SdkSuccess}; use aws_smithy_client::{retry, SdkError}; use aws_smithy_http::body::SdkBody; @@ -27,22 +26,20 @@ use aws_smithy_http::retry::ClassifyRetry; use aws_smithy_http_tower::map_request::{ AsyncMapRequestLayer, AsyncMapRequestService, MapRequestLayer, MapRequestService, }; +use aws_smithy_types::error::opaque::OpaqueError; use aws_smithy_types::retry::{ErrorKind, RetryKind}; use aws_types::os_shim_internal::{Env, Fs}; - use bytes::Bytes; -use http::uri::InvalidUri; use http::{Response, Uri}; +use std::borrow::Cow; +use std::convert::TryFrom; +use std::error::Error; +use std::fmt::{Display, Formatter}; +use std::str::FromStr; +use std::sync::Arc; +use std::time::Duration; use tokio::sync::OnceCell; -use crate::connector::expect_connector; -use crate::imds::client::token::TokenMiddleware; -use crate::profile::credentials::ProfileFileError; -use crate::provider_config::ProviderConfig; -use crate::{profile, PKG_VERSION}; -use aws_sdk_sso::config::timeout::TimeoutConfig; -use aws_smithy_client::http_connector::ConnectorSettings; - mod token; // 6 hours @@ -207,11 +204,11 @@ impl Client { .map_err(|err| match err { SdkError::ConstructionFailure(err) => match err.downcast::() { Ok(token_failure) => *token_failure, - Err(other) => ImdsError::Unexpected(other), + Err(other) => ImdsError::unexpected(other), }, - SdkError::TimeoutError(err) => ImdsError::IoError(err), - SdkError::DispatchFailure(err) => ImdsError::IoError(err.into()), - SdkError::ResponseError { err, .. } => ImdsError::IoError(err), + SdkError::TimeoutError(err) => ImdsError::io_error(err), + SdkError::DispatchFailure(err) => ImdsError::io_error(err), + SdkError::ResponseError { err, .. } => ImdsError::io_error(err), SdkError::ServiceError { err: InnerImdsError::BadStatus, raw, @@ -221,7 +218,7 @@ impl Client { SdkError::ServiceError { err: InnerImdsError::InvalidUtf8, .. - } => ImdsError::Unexpected("IMDS returned invalid UTF-8".into()), + } => ImdsError::unexpected("IMDS returned invalid UTF-8"), }) } @@ -255,11 +252,16 @@ pub enum ImdsError { /// /// Requests to IMDS must be accompanied by a token obtained via a `PUT` request. This is handled /// transparently by the [`Client`]. - FailedToLoadToken(SdkError), + #[non_exhaustive] + FailedToLoadToken { + /// Cause of the error + source: SdkError, + }, /// The `path` was invalid for an IMDS request /// /// The `path` parameter must be a valid URI path segment, and it must begin with `/`. + #[non_exhaustive] InvalidPath, /// An error response was returned from IMDS @@ -272,17 +274,43 @@ pub enum ImdsError { /// IO Error /// /// An error occurred communication with IMDS - IoError(Box), + #[non_exhaustive] + IoError { + /// Cause of the error + source: OpaqueError, + }, /// An unexpected error occurred communicating with IMDS - Unexpected(Box), + #[non_exhaustive] + Unexpected { + /// Cause of the error + source: OpaqueError, + }, +} + +impl ImdsError { + fn failed_to_load_token(source: SdkError) -> Self { + Self::FailedToLoadToken { source } + } + + fn io_error(source: impl Into>) -> Self { + Self::IoError { + source: OpaqueError::new(source), + } + } + + fn unexpected(source: impl Into>) -> Self { + Self::Unexpected { + source: OpaqueError::new(source), + } + } } impl Display for ImdsError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - ImdsError::FailedToLoadToken(inner) => { - write!(f, "Failed to load session token: {}", inner) + ImdsError::FailedToLoadToken { .. } => { + write!(f, "Failed to load session token") } ImdsError::InvalidPath => write!( f, @@ -294,14 +322,12 @@ impl Display for ImdsError { response.status().as_u16(), response ), - ImdsError::IoError(err) => { - write!(f, "An IO error occurred communicating with IMDS: {}", err) + ImdsError::IoError { .. } => { + write!(f, "An IO error occurred communicating with IMDS") + } + ImdsError::Unexpected { .. } => { + write!(f, "An unexpected error occurred communicating with IMDS") } - ImdsError::Unexpected(err) => write!( - f, - "An unexpected error occurred communicating with IMDS: {}", - err - ), } } } @@ -309,8 +335,9 @@ impl Display for ImdsError { impl Error for ImdsError { fn source(&self) -> Option<&(dyn Error + 'static)> { match &self { - ImdsError::FailedToLoadToken(inner) => Some(inner), - _ => None, + Self::FailedToLoadToken { source } => Some(source), + Self::IoError { source } | Self::Unexpected { source } => Some(source), + Self::InvalidPath | Self::ErrorResponse { .. } => None, } } } @@ -434,24 +461,53 @@ pub struct Builder { /// Error constructing IMDSv2 Client #[derive(Debug)] +#[non_exhaustive] pub enum BuildError { /// The endpoint mode was invalid - InvalidEndpointMode(InvalidEndpointMode), + #[non_exhaustive] + InvalidEndpointMode { + /// Cause of the error + source: InvalidEndpointMode, + }, /// The AWS Profile (e.g. `~/.aws/config`) was invalid - InvalidProfile(ProfileFileError), + #[non_exhaustive] + InvalidProfile { + /// Cause of the error + source: ProfileFileError, + }, /// The specified endpoint was not a valid URI - InvalidEndpointUri(InvalidUri), + #[non_exhaustive] + InvalidEndpointUri { + /// Cause of the error + source: OpaqueError, + }, +} + +impl BuildError { + fn invalid_endpoint_mode(source: InvalidEndpointMode) -> Self { + Self::InvalidEndpointMode { source } + } + + fn invalid_profile(source: ProfileFileError) -> Self { + Self::InvalidProfile { source } + } + + fn invalid_endpoint_uri(source: impl Into>) -> Self { + Self::InvalidEndpointUri { + source: OpaqueError::new(source), + } + } } impl Display for BuildError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "failed to build IMDS client: ")?; + write!(f, "failed to build IMDS client because of an ")?; match self { - BuildError::InvalidEndpointMode(e) => write!(f, "{}", e), - BuildError::InvalidProfile(e) => write!(f, "{}", e), - BuildError::InvalidEndpointUri(e) => write!(f, "{}", e), + Self::InvalidEndpointMode { .. } => write!(f, "invalid endpoint mode"), + Self::InvalidProfile { .. } => write!(f, "invalid profile"), + Self::InvalidEndpointUri { .. } => write!(f, "invalid endpoint URI"), } } } @@ -459,9 +515,9 @@ impl Display for BuildError { impl Error for BuildError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { - BuildError::InvalidEndpointMode(e) => Some(e), - BuildError::InvalidProfile(e) => Some(e), - BuildError::InvalidEndpointUri(e) => Some(e), + Self::InvalidEndpointMode { source } => Some(source), + Self::InvalidProfile { source } => Some(source), + Self::InvalidEndpointUri { source } => Some(source), } } } @@ -628,14 +684,14 @@ impl EndpointSource { // load an endpoint override from the environment let profile = profile::load(fs, env, &Default::default()) .await - .map_err(BuildError::InvalidProfile)?; + .map_err(BuildError::invalid_profile)?; let uri_override = if let Ok(uri) = env.get(env::ENDPOINT) { Some(Cow::Owned(uri)) } else { profile.get(profile_keys::ENDPOINT).map(Cow::Borrowed) }; if let Some(uri) = uri_override { - return Uri::try_from(uri.as_ref()).map_err(BuildError::InvalidEndpointUri); + return Uri::try_from(uri.as_ref()).map_err(BuildError::invalid_endpoint_uri); } // if not, load a endpoint mode from the environment @@ -643,10 +699,10 @@ impl EndpointSource { mode } else if let Ok(mode) = env.get(env::ENDPOINT_MODE) { mode.parse::() - .map_err(BuildError::InvalidEndpointMode)? + .map_err(BuildError::invalid_endpoint_mode)? } else if let Some(mode) = profile.get(profile_keys::ENDPOINT_MODE) { mode.parse::() - .map_err(BuildError::InvalidEndpointMode)? + .map_err(BuildError::invalid_endpoint_mode)? } else { EndpointMode::IpV4 }; @@ -659,30 +715,36 @@ impl EndpointSource { /// Error retrieving token from IMDS #[derive(Debug)] +#[non_exhaustive] pub enum TokenError { /// The token was invalid /// /// Because tokens must be eventually sent as a header, the token must be a valid header value. + #[non_exhaustive] InvalidToken, /// No TTL was sent /// /// The token response must include a time-to-live indicating the lifespan of the token. + #[non_exhaustive] NoTtl, /// The TTL was invalid /// /// The TTL must be a valid positive integer. + #[non_exhaustive] InvalidTtl, /// Invalid Parameters /// /// The request to load a token was malformed. This indicates an SDK bug. + #[non_exhaustive] InvalidParameters, /// Forbidden /// /// IMDS is disabled or has been disallowed via permissions. + #[non_exhaustive] Forbidden, } @@ -763,6 +825,21 @@ pub(crate) mod test { use std::time::{Duration, UNIX_EPOCH}; use tracing_test::traced_test; + macro_rules! assert_full_error_contains { + ($err:expr, $contains:expr) => { + let err = $err; + let message = format!( + "{}", + aws_smithy_types::error::display_context::DisplayErrorContext(&err) + ); + assert!( + message.contains($contains), + "Error message '{message}' didn't contain text '{}'", + $contains + ); + }; + } + const TOKEN_A: &str = "AQAEAFTNrA4eEGx0AQgJ1arIq_Cc-t4tWt3fB0Hd8RKhXlKc5ccvhg=="; const TOKEN_B: &str = "alternatetoken=="; @@ -1026,7 +1103,7 @@ pub(crate) mod test { )]); let client = make_client(&connection).await; let err = client.get("/latest/metadata").await.expect_err("no token"); - assert!(format!("{}", err).contains("forbidden"), "{}", err); + assert_full_error_contains!(err, "forbidden"); connection.assert_requests_match(&[]); } @@ -1068,7 +1145,7 @@ pub(crate) mod test { )]); let client = make_client(&connection).await; let err = client.get("/latest/metadata").await.expect_err("no token"); - assert!(format!("{}", err).contains("Invalid Token"), "{}", err); + assert_full_error_contains!(err, "Invalid Token"); connection.assert_requests_match(&[]); } @@ -1089,7 +1166,7 @@ pub(crate) mod test { ]); let client = make_client(&connection).await; let err = client.get("/latest/metadata").await.expect_err("no token"); - assert!(format!("{}", err).contains("invalid UTF-8"), "{}", err); + assert_full_error_contains!(err, "invalid UTF-8"); connection.assert_requests_match(&[]); } @@ -1123,7 +1200,8 @@ pub(crate) mod test { time_elapsed ); match resp { - ImdsError::FailedToLoadToken(err) if format!("{}", err).contains("timeout") => {} // ok, + ImdsError::FailedToLoadToken { source } + if format!("{}", source).contains("timeout") => {} // ok, other => panic!( "wrong error, expected construction failure with TimedOutError inside: {}", other @@ -1181,12 +1259,7 @@ pub(crate) mod test { test, test_case.docs ), (Err(substr), Err(err)) => { - assert!( - format!("{}", err).contains(substr), - "`{}` did not contain `{}`", - err, - substr - ); + assert_full_error_contains!(err, substr); return; } (Ok(_uri), Err(e)) => panic!( diff --git a/aws/rust-runtime/aws-config/src/imds/client/token.rs b/aws/rust-runtime/aws-config/src/imds/client/token.rs index 4cf6f299fa..cfd531337d 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/token.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/token.rs @@ -150,7 +150,7 @@ impl TokenMiddleware { .client .call(operation) .await - .map_err(ImdsError::FailedToLoadToken)?; + .map_err(ImdsError::failed_to_load_token)?; let expiry = response.expiry; Ok((response, expiry)) } diff --git a/aws/rust-runtime/aws-config/src/imds/credentials.rs b/aws/rust-runtime/aws-config/src/imds/credentials.rs index e0b574704e..138fc9bf49 100644 --- a/aws/rust-runtime/aws-config/src/imds/credentials.rs +++ b/aws/rust-runtime/aws-config/src/imds/credentials.rs @@ -137,9 +137,12 @@ impl ImdsCredentialsProvider { ); Err(CredentialsError::not_loaded("received 404 from IMDS")) } - Err(ImdsError::FailedToLoadToken(SdkError::DispatchFailure(err))) => Err( - CredentialsError::not_loaded(format!("could not communicate with imds: {}", err)), - ), + Err(ImdsError::FailedToLoadToken { + source: SdkError::DispatchFailure(err), + }) => Err(CredentialsError::not_loaded(format!( + "could not communicate with IMDS: {}", + err + ))), Err(other) => Err(CredentialsError::provider_error(other)), } } diff --git a/aws/rust-runtime/aws-config/src/imds/region.rs b/aws/rust-runtime/aws-config/src/imds/region.rs index 33096f0f2b..cec3fc2d4c 100644 --- a/aws/rust-runtime/aws-config/src/imds/region.rs +++ b/aws/rust-runtime/aws-config/src/imds/region.rs @@ -12,10 +12,9 @@ use crate::imds; use crate::imds::client::LazyClient; use crate::meta::region::{future, ProvideRegion}; use crate::provider_config::ProviderConfig; - +use aws_smithy_types::error::display_context::DisplayErrorContext; use aws_types::os_shim_internal::Env; use aws_types::region::Region; - use tracing::Instrument; /// IMDSv2 Region Provider @@ -57,7 +56,7 @@ impl ImdsRegionProvider { Some(Region::new(region)) } Err(err) => { - tracing::warn!(err = % err, "failed to load region from IMDS"); + tracing::warn!(err = %DisplayErrorContext(&err), "failed to load region from IMDS"); None } } diff --git a/aws/rust-runtime/aws-config/src/json_credentials.rs b/aws/rust-runtime/aws-config/src/json_credentials.rs index 8e0650ac7a..31c3229256 100644 --- a/aws/rust-runtime/aws-config/src/json_credentials.rs +++ b/aws/rust-runtime/aws-config/src/json_credentials.rs @@ -6,6 +6,7 @@ use aws_smithy_json::deserialize::token::skip_value; use aws_smithy_json::deserialize::{json_token_iter, EscapeError, Token}; use aws_smithy_types::date_time::Format; +use aws_smithy_types::error::opaque::OpaqueError; use aws_smithy_types::DateTime; use std::borrow::Cow; use std::convert::TryFrom; @@ -16,14 +17,15 @@ use std::time::SystemTime; #[derive(Debug)] pub(crate) enum InvalidJsonCredentials { /// The response did not contain valid JSON - JsonError(Box), + JsonError(OpaqueError), + /// The response was missing a required field MissingField(&'static str), /// A field was invalid InvalidField { field: &'static str, - err: Box, + source: OpaqueError, }, /// Another unhandled error occurred @@ -32,36 +34,44 @@ pub(crate) enum InvalidJsonCredentials { impl From for InvalidJsonCredentials { fn from(err: EscapeError) -> Self { - InvalidJsonCredentials::JsonError(err.into()) + Self::JsonError(OpaqueError::new(err)) } } impl From for InvalidJsonCredentials { fn from(err: aws_smithy_json::deserialize::Error) -> Self { - InvalidJsonCredentials::JsonError(err.into()) + Self::JsonError(OpaqueError::new(err)) } } impl Display for InvalidJsonCredentials { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - InvalidJsonCredentials::JsonError(json) => { - write!(f, "invalid JSON in response: {}", json) + Self::JsonError(_) => { + write!(f, "invalid JSON in response") } - InvalidJsonCredentials::MissingField(field) => write!( + Self::MissingField(field) => write!( f, "Expected field `{}` in response but it was missing", field ), - InvalidJsonCredentials::Other(msg) => write!(f, "{}", msg), - InvalidJsonCredentials::InvalidField { field, err } => { - write!(f, "Invalid field in response: `{}`. {}", field, err) + Self::Other(msg) => write!(f, "{}", msg), + Self::InvalidField { field, source: _ } => { + write!(f, "Invalid field in response: `{field}`") } } } } -impl Error for InvalidJsonCredentials {} +impl Error for InvalidJsonCredentials { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::JsonError(source) => Some(source), + Self::InvalidField { source, .. } => Some(source), + Self::MissingField(_) | Self::Other(_) => None, + } + } +} #[derive(PartialEq, Eq)] pub(crate) struct RefreshableCredentials<'a> { @@ -181,7 +191,7 @@ pub(crate) fn parse_json_credentials( DateTime::from_str(expiration.as_ref(), Format::DateTime).map_err(|err| { InvalidJsonCredentials::InvalidField { field: "Expiration", - err: err.into(), + source: OpaqueError::new(err), } })?, ) diff --git a/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs b/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs index 542352920a..07012e55b2 100644 --- a/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs +++ b/aws/rust-runtime/aws-config/src/meta/credentials/chain.rs @@ -3,9 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -use std::borrow::Cow; - +use aws_smithy_types::error::display_context::DisplayErrorContext; use aws_types::credentials::{self, future, CredentialsError, ProvideCredentials}; +use std::borrow::Cow; use tracing::Instrument; /// Credentials provider that checks a series of inner providers @@ -82,12 +82,11 @@ impl CredentialsProviderChain { tracing::debug!(provider = %name, "loaded credentials"); return Ok(credentials); } - Err(CredentialsError::CredentialsNotLoaded { context, .. }) => { - tracing::debug!(provider = %name, context = %context, "provider in chain did not provide credentials"); + Err(CredentialsError::CredentialsNotLoaded { source, .. }) => { + tracing::debug!(provider = %name, context = %DisplayErrorContext(&source), "provider in chain did not provide credentials"); } - Err(e) => { - tracing::warn!(provider = %name, error = %e, "provider failed to provide credentials"); - return Err(e); + Err(err) => { + return Err(err); } } } diff --git a/aws/rust-runtime/aws-config/src/profile/credentials.rs b/aws/rust-runtime/aws-config/src/profile/credentials.rs index 1a025339cf..d8d0499b27 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials.rs @@ -214,7 +214,7 @@ impl ProfileFileCredentialsProvider { #[derive(Debug)] pub struct CouldNotReadProfileFile { pub(crate) path: PathBuf, - pub(crate) cause: std::io::Error, + pub(crate) source: std::io::Error, } /// An Error building a Credential source from an AWS Profile @@ -223,7 +223,10 @@ pub struct CouldNotReadProfileFile { pub enum ProfileFileError { /// The profile was not a valid AWS profile #[non_exhaustive] - CouldNotParseProfile(ProfileParseError), + CouldNotParseProfile { + /// Cause of the error + source: ProfileParseError, + }, /// No profiles existed (the profile was empty) #[non_exhaustive] @@ -293,13 +296,13 @@ impl ProfileFileError { impl Display for ProfileFileError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - ProfileFileError::CouldNotParseProfile(err) => { - write!(f, "could not parse profile file: {}", err) + ProfileFileError::CouldNotParseProfile { .. } => { + write!(f, "could not parse profile file") } ProfileFileError::CredentialLoop { profiles, next } => write!( f, "profile formed an infinite loop. first we loaded {:?}, \ - then attempted to reload {}", + then attempted to reload {}", profiles, next ), ProfileFileError::MissingCredentialSource { profile, message } => { @@ -336,16 +339,22 @@ impl Display for ProfileFileError { impl Error for ProfileFileError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { - ProfileFileError::CouldNotParseProfile(err) => Some(err), - ProfileFileError::CouldNotReadProfileFile(details) => Some(&details.cause), - _ => None, + Self::CouldNotParseProfile { source } => Some(source), + Self::CouldNotReadProfileFile(details) => Some(&details.source), + Self::NoProfilesDefined + | Self::ProfileDidNotContainCredentials { .. } + | Self::CredentialLoop { .. } + | Self::MissingCredentialSource { .. } + | Self::InvalidCredentialSource { .. } + | Self::MissingProfile { .. } + | Self::UnknownProvider { .. } => None, } } } impl From for ProfileFileError { - fn from(err: ProfileParseError) -> Self { - ProfileFileError::CouldNotParseProfile(err) + fn from(source: ProfileParseError) -> Self { + ProfileFileError::CouldNotParseProfile { source } } } diff --git a/aws/rust-runtime/aws-config/src/profile/parser/source.rs b/aws/rust-runtime/aws-config/src/profile/parser/source.rs index 50057805da..e9fac2fae7 100644 --- a/aws/rust-runtime/aws-config/src/profile/parser/source.rs +++ b/aws/rust-runtime/aws-config/src/profile/parser/source.rs @@ -129,7 +129,7 @@ async fn load_config_file( return Err(ProfileFileError::CouldNotReadProfileFile( CouldNotReadProfileFile { path: path.clone(), - cause: e, + source: e, }, )) } diff --git a/aws/rust-runtime/aws-config/src/sso.rs b/aws/rust-runtime/aws-config/src/sso.rs index 54bcc07b7f..8c73bd4dfb 100644 --- a/aws/rust-runtime/aws-config/src/sso.rs +++ b/aws/rust-runtime/aws-config/src/sso.rs @@ -13,25 +13,23 @@ use crate::fs_util::{home_dir, Os}; use crate::json_credentials::{json_parse_loop, InvalidJsonCredentials}; use crate::provider_config::ProviderConfig; - use aws_sdk_sso::middleware::DefaultMiddleware as SsoMiddleware; use aws_sdk_sso::model::RoleCredentials; use aws_smithy_client::erase::DynConnector; use aws_smithy_json::deserialize::Token; use aws_smithy_types::date_time::Format; +use aws_smithy_types::error::opaque::OpaqueError; use aws_smithy_types::DateTime; use aws_types::credentials::{CredentialsError, ProvideCredentials}; use aws_types::os_shim_internal::{Env, Fs}; use aws_types::region::Region; use aws_types::{credentials, Credentials}; - +use ring::digest; use std::convert::TryInto; use std::error::Error; use std::fmt::{Display, Formatter}; use std::io; use std::path::PathBuf; - -use ring::digest; use zeroize::Zeroizing; impl crate::provider_config::ProviderConfig { @@ -164,18 +162,18 @@ impl Builder { pub(crate) enum LoadTokenError { InvalidCredentials(InvalidJsonCredentials), NoHomeDirectory, - IoError { err: io::Error, path: PathBuf }, + IoError { source: io::Error, path: PathBuf }, } impl Display for LoadTokenError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - LoadTokenError::InvalidCredentials(err) => { - write!(f, "SSO Token was invalid (expected JSON): {}", err) + Self::InvalidCredentials(_) => { + write!(f, "SSO Token was invalid (expected JSON)") } - LoadTokenError::NoHomeDirectory => write!(f, "Could not resolve a home directory"), - LoadTokenError::IoError { err, path } => { - write!(f, "failed to read `{}`: {}", path.display(), err) + Self::NoHomeDirectory => write!(f, "Could not resolve a home directory"), + Self::IoError { source: _, path } => { + write!(f, "failed to read `{}`", path.display()) } } } @@ -184,9 +182,9 @@ impl Display for LoadTokenError { impl Error for LoadTokenError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { - LoadTokenError::InvalidCredentials(err) => Some(err as _), - LoadTokenError::NoHomeDirectory => None, - LoadTokenError::IoError { err, .. } => Some(err as _), + Self::InvalidCredentials(source) => Some(source as _), + Self::NoHomeDirectory => None, + Self::IoError { source, .. } => Some(source as _), } } } @@ -261,7 +259,7 @@ async fn load_token(start_url: &str, env: &Env, fs: &Fs) -> Result Result { let expires_at = DateTime::from_str(expires_at.as_ref(), Format::DateTime).map_err(|e| { InvalidJsonCredentials::InvalidField { field: "expiresAt", - err: e.into(), + source: OpaqueError::new(e), } })?; let region = region.map(Region::new); @@ -390,11 +388,7 @@ mod test { "startUrl": "https://d-abc123.awsapps.com/start" }"#; let err = parse_token_json(token).expect_err("invalid timestamp"); - assert!( - format!("{}", err).contains("Invalid field in response: `expiresAt`."), - "{}", - err - ); + assert_eq!("Invalid field in response: `expiresAt`", format!("{}", err)); } #[test] diff --git a/aws/rust-runtime/aws-config/src/test_case.rs b/aws/rust-runtime/aws-config/src/test_case.rs index 9543995503..cde5e242a8 100644 --- a/aws/rust-runtime/aws-config/src/test_case.rs +++ b/aws/rust-runtime/aws-config/src/test_case.rs @@ -3,17 +3,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +use crate::connector::default_connector; use crate::provider_config::ProviderConfig; - use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep, TokioSleep}; use aws_smithy_client::dvr::{NetworkTraffic, RecordingConnection, ReplayingConnection}; use aws_smithy_client::erase::DynConnector; +use aws_smithy_types::error::display_context::DisplayErrorContext; use aws_types::credentials::{self, ProvideCredentials}; use aws_types::os_shim_internal::{Env, Fs}; - use serde::Deserialize; - -use crate::connector::default_connector; use std::collections::HashMap; use std::error::Error; use std::fmt::Debug; @@ -101,7 +99,7 @@ where } (Err(err), GenericTestResult::ErrorContains(substr)) => { assert!( - format!("{}", err).contains(substr), + format!("{}", DisplayErrorContext(&err)).contains(substr), "`{}` did not contain `{}`", err, substr diff --git a/aws/rust-runtime/aws-config/src/web_identity_token.rs b/aws/rust-runtime/aws-config/src/web_identity_token.rs index 8ad9aa8614..d16b1f0bc5 100644 --- a/aws/rust-runtime/aws-config/src/web_identity_token.rs +++ b/aws/rust-runtime/aws-config/src/web_identity_token.rs @@ -266,6 +266,7 @@ mod test { }; use aws_sdk_sts::Region; use aws_smithy_async::rt::sleep::TokioSleep; + use aws_smithy_types::error::display_context::DisplayErrorContext; use aws_types::credentials::CredentialsError; use aws_types::os_shim_internal::{Env, Fs}; use std::collections::HashMap; @@ -308,7 +309,7 @@ mod test { .await .expect_err("should fail, provider not loaded"); assert!( - format!("{}", err).contains("AWS_ROLE_ARN"), + format!("{}", DisplayErrorContext(&err)).contains("AWS_ROLE_ARN"), "`{}` did not contain expected string", err );