Unverified Commit fba88df3 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Revamp errors in `aws-smithy-http` (#1884)

parent b43905ea
Loading
Loading
Loading
Loading
+18 −14
Original line number Diff line number Diff line
@@ -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)),
        }
    }
@@ -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)
            }
@@ -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)),
            )),
        }
    }

@@ -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
+47 −33
Original line number Diff line number Diff line
@@ -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::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())
                    }
                    InnerImdsError::BadStatus => ImdsError::ErrorResponse {
                        response: context.into_raw().into_parts().0,
                    },
                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::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()),
            })
    }

@@ -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,
        }
    }
@@ -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==";

@@ -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(&[]);
    }

@@ -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))
@@ -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(&[]);
    }

@@ -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(&[]);
    }

@@ -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()
@@ -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
@@ -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!(
+7 −3
Original line number Diff line number Diff line
@@ -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};
@@ -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)),
        }
    }
+14 −14
Original line number Diff line number Diff line
@@ -19,6 +19,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
@@ -253,21 +254,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 },
                        | AssumeRoleErrorKind::MalformedPolicyDocumentException(_)
                ) =>
            {
                Err(CredentialsError::invalid_configuration(
                    assumed.err().unwrap(),
                ))
            }
                    _ => {}
                }
                tracing::warn!(error = ?err.message(), "STS refused to grant assume role");
                Err(CredentialsError::provider_error(SdkError::ServiceError {
                    err,
                    raw,
                }))
            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)),
        }
+5 −5
Original line number Diff line number Diff line
@@ -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;
@@ -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: {}",
Loading