RFC: New Error Shapes

[rcoh]

Summary

Currently, generated errors from the SDK are in two layers. An outer layer, SDKError is generic in E, the type of error the service may have returned, eg. dynamodb::error::ListTablesError. This RFC proposes inverting that structure and making SdkError a variant of each modeled operation error.

However, this simplification does come with a tradeoff, namely, there is no longer a convenient place to put the raw HTTP response for introspection. This could be worked around, however. call_raw currently returns the success response alongside the raw HTTP response. The same thing could be done for errors. Or, the raw HTTP response could be included inside of SdkError. We could also stick the http response into the metadata that is attached to errors.

Details

The current structure of SdkError:

/// Failing Sdk Result
#[derive(Debug)]
pub enum SdkError<E> {
    /// The request failed during construction. It was not dispatched over the network.
    ConstructionFailure(BoxError),

    /// The request failed during dispatch. An HTTP response was not received. The request MAY
    /// have been sent.
    DispatchFailure(BoxError),

    /// A response was received but it was not parseable according the the protocol (for example
    /// the server hung up while the body was being read)
    ResponseError {
        raw: http::Response<ResponseBody>,
        err: BoxError,
    },

    /// An error response was received from the service
    ServiceError {
        err: E,
        raw: http::Response<ResponseBody>,
    },
}

In practice, a function signature may look like:

        pub async fn send(self) -> Result<ListTablesOutput, SdkError<ListTablesError>>

However, this can be cumbersome in the common case that you want to handle a specific error coming back from the SDK. The cleanest pattern currently looks like this:

    let data = match client
        .create_secret()
        .name(secret_name)
        .secret_string(secret_value)
        .send()
        .await
    {
        Ok(secret) => secret,
        Err(SdkError::ServiceError { err, .. })
            if matches!(err.kind, CreateSecretErrorKind::ResourceExistsError(_)) =>
        {
            panic!("This secret already exists!")
        }

        Err(other) => panic!("Failed to create secret: {}", other),
    };

We propose inverting the error structure so that SdkError is actually a variant of the individual response errors:

#[non_exhaustive]
#[derive(std::fmt::Debug)]
pub enum ListTablesErrorKind {
    InternalServerError(InternalServerError),
    InvalidEndpointError(InvalidEndpointError),

    /// An error sending or receiving a response
    SdkError(SdkError),
}

This would improve the error handling experience in the common case, especially with the planned addition of is_xyz helpers: (#298)

    let data = match client
        .create_secret()
        .name(secret_name)
        .secret_string(secret_value)
        .send()
        .await
    {
        Ok(secret) => secret,
        Err(err) if err.is_resource_exists_error() => {
            panic!("This secret already exists!")
        },
        Err(other) => panic!("Failed to create secret: {}", other),
    };