Handling service errors requires three levels of object nesting

[joshtriplett]

trafficstars

Describe the feature

I'd like a simpler way to handle service-specific errors without having to match through three levels of object nesting.

Use Case

Here's the code I currently have to do an S3 GetObject and handle NoSuchKey errors:

        let out = self
            .s3
            .get_object()
            .bucket(self.bucket.clone())
            .expected_bucket_owner(self.expected_bucket_owner.clone())
            .key(key)
            .send()
            .instrument(info_span!("GetObject"))
            .await;
        match out {
            Err(aws_sdk_s3::types::SdkError::ServiceError {
                err:
                    aws_sdk_s3::error::GetObjectError {
                        kind: aws_sdk_s3::error::GetObjectErrorKind::NoSuchKey(_),
                        ..
                    },
                ..
            }) => Ok(None),
            Err(e) => Err(e)?,
            Ok(out) => {
                /* handle data here */
            }
        }

This feels excessively complex and verbose for a reasonably common case.

I would love to write something like this instead:

        let out = self
            .s3
            .get_object()
            .bucket(self.bucket.clone())
            .expected_bucket_owner(self.expected_bucket_owner.clone())
            .key(key)
            .send()
            .instrument(info_span!("GetObject"))
            .await
            .map_service_errors()?;
        match out {
            Err(aws_sdk_s3::error::GetObjectErrorKind::NoSuchKey(_)) => Ok(None),
            Err(e) => Err(e)?,
            Ok(out) => {
                /* handle data here */
            }
        }

Proposed Solution

A method to map Result<T, SdkError<SomeServiceError>> into Result<Result<T, SomeServiceErrorKind>, SdkError<SomeServiceError>>, making it possible to call .send().await.non_service_err()? and then match on the resulting SomeServiceErrorKind. This would simplify handling of specific service errors (e.g. GetObjectErrorKind::NoSuchKey).

Acknowledgements

• [ ] I may be able to implement this feature request
• [ ] This feature might incur a breaking change

A note for the community

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue, please leave a comment

[joshtriplett]

Rough solution sketch:

Add a trait HasErrorKind, with an associated type Kind and a method kind() returning that asociated type.

Modify codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/error/CombinedErrorGenerator.kt to generate an impl HasErrorKind for each error type.

trait MapServiceErrors {
    type Output;
    pub fn map_service_errors(self) -> Self::Output;
}

impl<T, E: HasErrorKind> MapServiceErrors for Result<T, SdkError<E>> {
    type Output = Result<Result<T, E::Kind>, SdkError<E>>;
    pub fn map_service_errors(self) -> Self::Output {
        match self {
            Err(SdkError::ServiceError { err, .. }) => Ok(Err(err.kind())),
            Err(e) => Err(e),
            Ok(t) => Ok(Ok(t)),
        }
    }
}

[Velfi]

This is an interesting proposal. Thank you for taking the time to think about how your experience could be improved and to share a possible solution with us.

I think that by checking the kind field, you could shorten your original example a little bit:

 // For when you want to handle multiple kinds of error
match res {
    Err(aws_sdk_s3::types::SdkError::ServiceError { err, .. }) => match err.kind {
        GetObjectErrorKind::InvalidObjectState(_) => {}
        GetObjectErrorKind::NoSuchKey(_) => {}
        GetObjectErrorKind::Unhandled(_) => {}
        _ => {}
    },
    Err(e) => Err(e).unwrap(),
    Ok(_out) => { /* handle data here */ }
}

or you could use the is_ methods like this:

match res {
    Err(aws_sdk_s3::types::SdkError::ServiceError { err, .. }) if err.is_no_such_key() => {
        /* handle error you don't care about */
    }
    Err(e) => Err(e).unwrap(),
    Ok(_out) => { /* handle data here */ }
}

I do agree that our error UX could be improved and it's something we're looking at focusing on in the future. We don't currently have an estimate on when we'll begin that work.

[joshtriplett]

@Velfi is_no_such_key does help, thank you. I do think it would help in general to have a way to ?-away everything other than a service-specific error. While I do think narrowing to the kind would be even more convenient, I think even just a function to filter out the non-ServiceError case would be an improvement.

trait MapServiceErrors {
    type Output;
    pub fn map_service_errors(self) -> Self::Output;
}

impl<T, E> MapServiceErrors for Result<T, SdkError<E>> {
    type Output = Result<Result<T, E>, SdkError<E>>;
    pub fn map_service_errors(self) -> Self::Output {
        match self {
            Err(SdkError::ServiceError { err, .. }) => Ok(Err(err)),
            Err(e) => Err(e),
            Ok(t) => Ok(Ok(t)),
        }
    }
}