Skip to content

Commit

Permalink
Revamp errors in aws-smithy-http
Browse files Browse the repository at this point in the history
  • Loading branch information
jdisanti committed Oct 29, 2022
1 parent 3afc0db commit c419c76
Show file tree
Hide file tree
Showing 57 changed files with 1,045 additions and 709 deletions.
32 changes: 18 additions & 14 deletions aws/rust-runtime/aws-config/src/http_credential_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl HttpCredentialProvider {
let credentials = self.client.call(self.operation(auth)).await;
match credentials {
Ok(creds) => Ok(creds),
Err(SdkError::ServiceError { err, .. }) => Err(err),
Err(SdkError::ServiceError(context)) => Err(context.into_err()),
Err(other) => Err(CredentialsError::unhandled(other)),
}
}
Expand Down Expand Up @@ -176,13 +176,20 @@ impl ClassifyRetry<SdkSuccess<Credentials>, SdkError<CredentialsError>>
RetryKind::Error(ErrorKind::TransientError)
}
// non-parseable 200s
Err(SdkError::ServiceError {
err: CredentialsError::Unhandled { .. },
raw,
}) if raw.http().status().is_success() => RetryKind::Error(ErrorKind::ServerError),
Err(SdkError::ServiceError(context))
if matches!(context.err(), CredentialsError::Unhandled { .. })
&& context.raw().http().status().is_success() =>
{
RetryKind::Error(ErrorKind::ServerError)
}
// 5xx errors
Err(SdkError::ServiceError { raw, .. } | SdkError::ResponseError { raw, .. })
if raw.http().status().is_server_error() =>
Err(SdkError::ResponseError(context))
if context.raw().http().status().is_server_error() =>
{
RetryKind::Error(ErrorKind::ServerError)
}
Err(SdkError::ServiceError(context))
if context.raw().http().status().is_server_error() =>
{
RetryKind::Error(ErrorKind::ServerError)
}
Expand Down Expand Up @@ -219,10 +226,10 @@ mod test {
raw: operation::Response::new(resp.map(SdkBody::from)),
parsed: creds,
}),
Err(err) => Err(SdkError::ServiceError {
Err(err) => Err(SdkError::service_error(
err,
raw: operation::Response::new(resp.map(SdkBody::from)),
}),
operation::Response::new(resp.map(SdkBody::from)),
)),
}
}

Expand Down Expand Up @@ -278,10 +285,7 @@ mod test {
assert!(
matches!(
sdk_error,
SdkError::ServiceError {
err: CredentialsError::ProviderError { .. },
..
}
SdkError::ServiceError(ref context) if matches!(context.err(), CredentialsError::ProviderError { .. })
),
"should be provider error: {}",
sdk_error
Expand Down
80 changes: 47 additions & 33 deletions aws/rust-runtime/aws-config/src/imds/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,23 +206,26 @@ impl Client {
.call(operation)
.await
.map_err(|err| match err {
SdkError::ConstructionFailure(err) => match err.downcast::<ImdsError>() {
Ok(token_failure) => *token_failure,
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::ServiceError {
err: InnerImdsError::BadStatus,
raw,
} => ImdsError::ErrorResponse {
response: raw.into_parts().0,
SdkError::ConstructionFailure(_) if err.source().is_some() => {
match err.into_source().map(|e| e.downcast::<ImdsError>()) {
Ok(Ok(token_failure)) => *token_failure,
Ok(Err(err)) => ImdsError::Unexpected(err),
Err(err) => ImdsError::Unexpected(err.into()),
}
}
SdkError::ConstructionFailure(_) => ImdsError::Unexpected(err.into()),
SdkError::ServiceError(context) => match context.err() {
InnerImdsError::InvalidUtf8 => {
ImdsError::Unexpected("IMDS returned invalid UTF-8".into())
}
_ => ImdsError::ErrorResponse {
response: context.into_raw().into_parts().0,
},
},
SdkError::ServiceError {
err: InnerImdsError::InvalidUtf8,
..
} => ImdsError::Unexpected("IMDS returned invalid UTF-8".into()),
SdkError::TimeoutError(_)
| SdkError::DispatchFailure(_)
| SdkError::ResponseError(_) => ImdsError::IoError(err.into()),
_ => ImdsError::Unexpected(err.into()),
})
}

Expand Down Expand Up @@ -735,9 +738,8 @@ impl<T, E> ClassifyRetry<SdkSuccess<T>, SdkError<E>> for ImdsResponseRetryClassi
fn classify_retry(&self, response: Result<&SdkSuccess<T>, &SdkError<E>>) -> RetryKind {
match response {
Ok(_) => RetryKind::Unnecessary,
Err(SdkError::ResponseError { raw, .. }) | Err(SdkError::ServiceError { raw, .. }) => {
Self::classify(raw)
}
Err(SdkError::ResponseError(context)) => Self::classify(context.raw()),
Err(SdkError::ServiceError(context)) => Self::classify(context.raw()),
_ => RetryKind::UnretryableFailure,
}
}
Expand All @@ -764,6 +766,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::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==";

Expand Down Expand Up @@ -1027,7 +1044,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(&[]);
}

Expand All @@ -1050,10 +1067,10 @@ pub(crate) mod test {
);

// Emulate a failure to parse the response body (using an io error since it's easy to construct in a test)
let failure = SdkError::<()>::ResponseError {
err: Box::new(io::Error::new(io::ErrorKind::BrokenPipe, "fail to parse")),
raw: response_200(),
};
let failure = SdkError::<()>::response_error(
io::Error::new(io::ErrorKind::BrokenPipe, "fail to parse"),
response_200(),
);
assert_eq!(
RetryKind::UnretryableFailure,
classifier.classify_retry(Err::<&SdkSuccess<()>, _>(&failure))
Expand All @@ -1069,7 +1086,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(&[]);
}

Expand All @@ -1090,7 +1107,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(&[]);
}

Expand All @@ -1099,6 +1116,7 @@ pub(crate) mod test {
#[cfg(any(feature = "rustls", feature = "native-tls"))]
async fn one_second_connect_timeout() {
use crate::imds::client::ImdsError;
use aws_smithy_types::error::display::DisplayErrorContext;
use std::time::SystemTime;

let client = Client::builder()
Expand All @@ -1124,7 +1142,8 @@ pub(crate) mod test {
time_elapsed
);
match resp {
ImdsError::FailedToLoadToken(err) if format!("{}", err).contains("timeout") => {} // ok,
ImdsError::FailedToLoadToken(err)
if format!("{}", DisplayErrorContext(&err)).contains("timeout") => {} // ok,
other => panic!(
"wrong error, expected construction failure with TimedOutError inside: {}",
other
Expand Down Expand Up @@ -1182,12 +1201,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!(
Expand Down
10 changes: 7 additions & 3 deletions aws/rust-runtime/aws-config/src/imds/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::imds::client::{ImdsError, LazyClient};
use crate::json_credentials::{parse_json_credentials, JsonCredentials, RefreshableCredentials};
use crate::provider_config::ProviderConfig;
use aws_smithy_client::SdkError;
use aws_smithy_types::error::display::DisplayErrorContext;
use aws_types::credentials::{future, CredentialsError, ProvideCredentials};
use aws_types::os_shim_internal::Env;
use aws_types::{credentials, Credentials};
Expand Down Expand Up @@ -137,9 +138,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(ref err @ SdkError::DispatchFailure(_))) => {
Err(CredentialsError::not_loaded(format!(
"could not communicate with IMDS: {}",
DisplayErrorContext(&err)
)))
}
Err(other) => Err(CredentialsError::provider_error(other)),
}
}
Expand Down
28 changes: 14 additions & 14 deletions aws/rust-runtime/aws-config/src/sts/assume_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::time::Duration;

use crate::meta::credentials::LazyCachingCredentialsProvider;
use crate::provider_config::ProviderConfig;
use aws_smithy_types::error::display::DisplayErrorContext;
use tracing::Instrument;

/// Credentials provider that uses credentials provided by another provider to assume a role
Expand Down Expand Up @@ -226,21 +227,20 @@ impl Inner {
);
super::util::into_credentials(assumed.credentials, "AssumeRoleProvider")
}
Err(SdkError::ServiceError { err, raw }) => {
match err.kind {
Err(SdkError::ServiceError(ref context))
if matches!(
context.err().kind,
AssumeRoleErrorKind::RegionDisabledException(_)
| AssumeRoleErrorKind::MalformedPolicyDocumentException(_) => {
return Err(CredentialsError::invalid_configuration(
SdkError::ServiceError { err, raw },
))
}
_ => {}
}
tracing::warn!(error = ?err.message(), "STS refused to grant assume role");
Err(CredentialsError::provider_error(SdkError::ServiceError {
err,
raw,
}))
| AssumeRoleErrorKind::MalformedPolicyDocumentException(_)
) =>
{
Err(CredentialsError::invalid_configuration(
assumed.err().unwrap(),
))
}
Err(SdkError::ServiceError(ref context)) => {
tracing::warn!(error = %DisplayErrorContext(context.err()), "STS refused to grant assume role");
Err(CredentialsError::provider_error(assumed.err().unwrap()))
}
Err(err) => Err(CredentialsError::provider_error(err)),
}
Expand Down
10 changes: 5 additions & 5 deletions aws/rust-runtime/aws-config/src/test_case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use aws_types::os_shim_internal::{Env, Fs};
use serde::Deserialize;

use crate::connector::default_connector;
use aws_smithy_types::error::display::DisplayErrorContext;
use std::collections::HashMap;
use std::error::Error;
use std::fmt::Debug;
Expand Down Expand Up @@ -100,12 +101,11 @@ where
assert_eq!(expected, &actual.into(), "incorrect result was returned")
}
(Err(err), GenericTestResult::ErrorContains(substr)) => {
let message = format!("{}", DisplayErrorContext(&err));
assert!(
format!("{}", err).contains(substr),
"`{}` did not contain `{}`",
err,
substr
)
message.contains(substr),
"`{message}` did not contain `{substr}`"
);
}
(Err(actual_error), GenericTestResult::Ok(expected_creds)) => panic!(
"expected credentials ({:?}) but an error was returned: {}",
Expand Down
15 changes: 7 additions & 8 deletions aws/rust-runtime/aws-endpoint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub use partition::Partition;
pub use partition::PartitionResolver;
use std::collections::HashMap;

use aws_smithy_http::endpoint::Error as EndpointError;
use aws_smithy_http::endpoint::error::ResolveEndpointError;
use aws_smithy_http::endpoint::{apply_endpoint, EndpointPrefix, ResolveEndpoint};
use aws_smithy_http::middleware::MapRequest;
use aws_smithy_http::operation::Request;
Expand Down Expand Up @@ -54,19 +54,18 @@ impl EndpointShim {
}

impl ResolveEndpoint<Params> for EndpointShim {
fn resolve_endpoint(
&self,
params: &Params,
) -> Result<SmithyEndpoint, aws_smithy_http::endpoint::Error> {
fn resolve_endpoint(&self, params: &Params) -> Result<SmithyEndpoint, ResolveEndpointError> {
let aws_endpoint = self
.0
.resolve_endpoint(
params
.region
.as_ref()
.ok_or_else(|| EndpointError::message("no region in params"))?,
.ok_or_else(|| ResolveEndpointError::message("no region in params"))?,
)
.map_err(|err| EndpointError::message("failure resolving endpoint").with_cause(err))?;
.map_err(|err| {
ResolveEndpointError::message("failure resolving endpoint").with_source(err)
})?;
let uri = aws_endpoint.endpoint().uri();
let mut auth_scheme =
HashMap::from([("name".to_string(), Document::String("sigv4".into()))]);
Expand Down Expand Up @@ -134,7 +133,7 @@ impl MapRequest for AwsEndpointStage {
// its place
return Err(AwsEndpointStageError::EndpointResolutionError(std::mem::replace(
e,
aws_smithy_http::endpoint::Error::message("the original error was directly returned")
ResolveEndpointError::message("the original error was directly returned")
).into()));
}
};
Expand Down
16 changes: 8 additions & 8 deletions aws/rust-runtime/aws-http/src/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ mod test {
err: E,
raw: http::Response<&'static str>,
) -> Result<SdkSuccess<()>, SdkError<E>> {
Err(SdkError::ServiceError {
Err(SdkError::service_error(
err,
raw: operation::Response::new(raw.map(SdkBody::from)),
})
operation::Response::new(raw.map(SdkBody::from)),
))
}

#[test]
Expand Down Expand Up @@ -263,10 +263,10 @@ mod test {
let policy = AwsResponseRetryClassifier::new();
assert_eq!(
policy.classify_retry(
Result::<SdkSuccess<()>, SdkError<UnmodeledError>>::Err(SdkError::ResponseError {
err: Box::new(UnmodeledError),
raw: operation::Response::new(http::Response::new("OK").map(SdkBody::from)),
})
Result::<SdkSuccess<()>, SdkError<UnmodeledError>>::Err(SdkError::response_error(
UnmodeledError,
operation::Response::new(http::Response::new("OK").map(SdkBody::from)),
))
.as_ref()
),
RetryKind::Error(ErrorKind::TransientError)
Expand All @@ -276,7 +276,7 @@ mod test {
#[test]
fn test_timeout_error() {
let policy = AwsResponseRetryClassifier::new();
let err: Result<(), SdkError<UnmodeledError>> = Err(SdkError::TimeoutError("blah".into()));
let err: Result<(), SdkError<UnmodeledError>> = Err(SdkError::timeout_error("blah"));
assert_eq!(
policy.classify_retry(err.as_ref()),
RetryKind::Error(ErrorKind::TransientError)
Expand Down
Loading

0 comments on commit c419c76

Please sign in to comment.