Remix.run Logo
dev_l1x_be 3 days ago

I can't remember the last time I had any problem with the borrow checker. The junior solution is .clone(), better one is & (reference) and if you really need you can start to use <'a>. There is a mild annoyance with which function consumes what and the LLM era really helped with this.

My beef is sometimes with the ways traits are implemented or how AWS implemented Errors for the their library that is just pure madness.

vlovich123 3 days ago | parent | next [-]

> The junior solution is .clone()

I really hope it’s an Rc/Arc that you’re cloning. Just deep cloning the value to get ownership is dangerous when you’re doing it blindly.

dev_l1x_be 2 days ago | parent [-]

Sure thing, this is why you need to learn what happens with copy and clone. Interestingly smaller problems are going to be ok even with clone(). The real value is to be able to spot potential performance optimizations by grep '(copy|clone)'.

vlovich123 2 days ago | parent [-]

How are you going to grep for copies? You know there’s no .copy method right?

seivan 3 days ago | parent | prev [-]

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...