How did AWS mess up errors?

▲ dev_l1x_be 3 days ago | parent [-]

Maybe I am holding it wrong.

Here is one piece of the problem:

  while let Some(page) = object_stream.next().await {
        match page {
            // ListObjectsV2Output
            Ok(p) => {
                if let Some(contents) = p.contents {
                    all_objects.extend(contents);
                }
            }
            // SdkError<ListObjectsV2Error, Response>
            Err(err) => {
                let raw_response = &err.raw_response();
                let service_error = &err.as_service_error();
                error!("ListObjectsV2Error: {:?} {:?}", &service_error, &raw_response);
                return Err(S3Error::Error(format!("ListObjectsV2Error: {:?}", err)));
            }
        }
    }

▲ estebank 3 days ago | parent [-]

Out of curiosity, why are you borrowing that many times? The following should work:

    while let Some(page) = object_stream.next().await {
        match page {
            // ListObjectsV2Output
            Ok(p) => {
                if let Some(contents) = p.contents {
                    all_objects.extend(contents);
                }
            }
            // SdkError<ListObjectsV2Error, Response>
            Err(err) => {
                let raw_response = err.raw_response();
                let service_error = err.as_service_error();
                error!("ListObjectsV2Error: {:?} {:?}", service_error, raw_response);
                return Err(S3Error::Error(format!("ListObjectsV2Error: {:?}", err)));
            }
        }
    }

I would have written it this way

    while let Some(page) = object_stream.next().await {
        let p: ListObjectsV2Output = page.map_err(|err| {
            // SdkError<ListObjectsV2Error, Response>
            let raw_response = err.raw_response();
            let service_error = err.as_service_error();
            error!("ListObjectsV2Error: {service_error:?} {raw_response:?}");
            S3Error::Error(format!("ListObjectsV2Error: {err:?}"))
        })?;
        if let Some(contents) = p.contents {
            all_objects.extend(contents);
        }
    }

although if your crate defines `S3Error`, then I would prefer to write

    while let Some(page) = object_stream.next().await {
        if let Some(contents) = page?.contents {
            all_objects.extend(contents);
        }
    }

by implementing `From`:

    impl From<SdkError<ListObjectsV2Error, Response>> for S3Error {
        fn from(err: SdkError<ListObjectsV2Error, Response>) -> S3Error {
            let raw_response = err.raw_response();
            let service_error = err.as_service_error();
            error!("ListObjectsV2Error: {service_error:?} {raw_response:?}");
            S3Error::Error(format!("ListObjectsV2Error: {err:?}"))
        }
    }

▲

dev_l1x_be 2 days ago | parent [-]

Excellent! Thank you!

My problem is that I should have something like

(http_status, reason) where http_status is a String or u16, reason is a enum with SomeError(String) structure. So essentially having a flat meaningful structure instead of this what we currently have. I do not have any mental model about the error structure of the AWS libs or don't even know where to start to create that mental model. As a result I just try to turn everything to a string and return it altogether hoping that the real issue is there somwhere in that structure.

I think the AWS library error handling is way to complex for what it does and one way we could improve that if Rust had a great example of a binary (bin) project that has lets say 2 layers of functions and showing how to organize your error effectively.

Now do this for a lib project. Without this you end up with this hot mess. At least this is how I see it. If you have a suggestion how should I return errors from a util.rs that has s3_list_objects() to my http handler than I would love to hear what you have to say.

Thanks for your suggestions anyway! I am going to re-implement my error handling and see if it gives us more clarity with impl.

	▲	estebank 2 days ago \| parent [-]
		You might want to look at anyhow and thiserror, the former for applications and for libraries the latter. thiserror "just" makes it easier to do what I suggested of writing a manual `impl From` so that `?` can transform from the error you're getting to the error you want. When the API you consume is very granular, that's actually great because it means that you have a lot of control over the transformation (it's hard to add detail that isn't already there), but it can be annoying when you don't care about that granularity (like when you just want to emit an error during shutdown or log during recovery). https://momori.dev/posts/rust-error-handling-thiserror-anyho... burntsushi has a good writeup about their difference in usecase here: https://www.reddit.com/r/rust/comments/1cnhy7d/whats_the_wis...