How best to test errors

[sazzer]

I'm building a service using the aws_sdk_cognitoidentityprovider crate, and it's been pretty simple so far. However, I want to be able to have build-time automation tests of my service to know that it works.

The recommended way to do this seems to be to have a wrapper around the AWS SDK Client type, and then be able to swap that wrapper out for another one. I've done that, and it works well enough. I've got a trait that exposes the operations I want, and an implementation that is just a thin wrapper around aws_sdk_cognitoidentityprovider::Client, and all seemed good.

However, what I'm struggling with is testing error cases. I can construct the success results easily enough, but I can't work out how to construct the error types. Is there any decent way to construct the correct error types for my mocked out API to return? Or do I need to instead be returning my own error types and doing the mapping in my wrapper? In which case, how do I test that? :)

Cheers

[jdisanti]

Right now, there isn't a way construct most of the errors the SDK returns. I think we should make it possible to construct the error types for better testing, and this has been requested in smithy-lang/smithy-rs#2208

Like you said, you can have your wrapper return your own error types, but that won't enable you to test the wrapper's error matching logic.

[rcoh]

we're working on getting a release out soon—in the meantime, you can test it out by switching your dependencies to the next branch of this repository.

https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-git-repositories

[sazzer]

So - it works :) That's fantastic, because it means that I can actually test the error cases from the SDK.

However, unless I'm missing something it's a little bit unwieldy. Which isn't the end of the world, but I thought I'd mention it in case there's any opportunity for future improvement. :)

For example, the mock version of my wrapper around the aws_sdk_cognitoidentityprovider::Client has the following method:

    async fn admin_get_user(&self, username: &str) -> Result<AdminGetUserOutput, SdkError<AdminGetUserError>> {
        Err(SdkError::ServiceError(
            ServiceError::builder()
                .source(AdminGetUserError::UserNotFoundException(
                    UserNotFoundException::builder().message("User does not exist").build(),
                ))
                .raw(aws_smithy_http::operation::Response::new(
                    http::Response::builder()
                        .body(aws_smithy_http::body::SdkBody::empty())
                        .unwrap(),
                ))
                .build(),
        ))
    }

That's all just to produce the UserNotFoundException error case, when it feels like it should be possible in a much simpler way. In particular, the .raw() portion isn't hugely useful and could probably be defaulted:

    async fn admin_get_user(&self, username: &str) -> Result<AdminGetUserOutput, SdkError<AdminGetUserError>> {
        Err(SdkError::ServiceError(
            ServiceError::builder()
                .source(AdminGetUserError::UserNotFoundException(
                    UserNotFoundException::builder().message("User does not exist").build(),
                ))
                .build(),
        ))
    }

And possibly even have builders on the individual error types instead of needing the various levels of wrapping: (Though that's significantly more work to achieve for the benefit)

    async fn admin_get_user(&self, username: &str) -> Result<AdminGetUserOutput, SdkError<AdminGetUserError>> {
        Err(AdminGetUserError::user_not_found_exception_builder().message("User does not exist").build())
    }

Just a couple of thoughts :)

Cheers

[jdisanti]

Nice! I like it. The raw response has been causing other issues for us, so I wouldn't be surprised if we change that in the near future (to hopefully eliminate the generic for it entirely). If we can eliminate the generic, then we can probably also make it optional.

I think, ideally, in some future hypothetical SDK, you would be able to do (note the into()):

    async fn admin_get_user(&self, username: &str) -> Result<AdminGetUserOutput, SdkError<AdminGetUserError>> {
        Err(AdminGetUserError::builder().message("User does not exist").build().into())
    }

[sazzer]

That would be awesome - I'll look forward to it ;)

Cheers

[ivan-kiselev]

What I ended up doing is discarding the raw part altogether and just returning the service error, like so, which didn't indeed prove to be useful in error handling.

Simplified example:

async fn create_user(&self, ...) -> Result<..., aws_sdk_cognitoidentityprovider::operation::sign_up::SignUpError>  {
...
self
  .sign_up()
  // ..params
  .send()
  .await.
  .map_err(SdkError::into_service_error)
}

[github-actions[bot]]

Hello! Reopening this discussion to make it searchable.