Re-Try Moved Parameter on Err

help
1

I'm wondering if there's a more efficient way to handle the case where a function (from a library I don't control) moves a value (large buffer of memory), but can be re-tried upon failure. Concrete example found here:

fn retryable_fn(a :Vec<u8>) -> Result<(), ()> { ... }

fn main() {
    let b = vec![0; 1024];
    let mut res = retryable_fn(b.clone());
    
    if res.is_err() {
        res = retryable_fn(b);
        
        if res.is_err() {
            panic!("Didn't work!!!");
        }
    }
    
    println!("Worked!!!");
}

My only option seems to be to clone b on every call to retryable_fn. However, this means (I think) that I'm copying a fairly large buffer for the common case where it works. I'd really like something where I preferably don't have to copy the buffer at all (though I realize that pretty much breaks the ownership model), or at least only when the function fails and I have to re-try it.

The exact example I'm working with is the reqwest library and re-trying a POST/PUT when it fails.

2

Then don't take ownership of the Vec<u8> or return the Vec<u8> on error from retryable_fn

With the first way, you can do

fn retryable_fn(vec: &mut Vec<u8>) -> Result<(), ()> { ... }

With the second way

fn retryable_fn(vec: Vec<u8>) -> Result<(), Vec<u8>> { ... }
3

@RustyYato thanks for the reply, but I don't control retryable_fn, it's from a library. My specific case is the reqwest library: reqwest::RequestBuilder - Rust

4

Ok, it looks like you can't directly use Vec<u8> if you want to cheaply retry. You can use bytes::Bytes instead.

fn main() {
    let b = vec![0; 1024];
    let b = bytes::Bytes::from(b);
    let mut res = retryable_fn(b.clone());
    
    if res.is_err() {
        res = retryable_fn(b);
        
        if res.is_err() {
            panic!("Didn't work!!!");
        }
    }
    
    println!("Worked!!!");
}

Cloning a bytes::Bytes is just incrementing an atomic integer, so it is really cheap. The only downside is that Bytes is immutable. But that doesn't matter for this case.

3 Likes
5

Interesting... if I wasn't passing a Vec<u8> but some other complex struct that is still large, there is no other way to do this without calling clone()?

6

In order to use reqwest::RequestBuilder::body you would have to implement Into<Body>, and the only ways to do that all go through bytes::Bytes or reqwest::Body::wrap_stream.

In the first case, we don't have any issues. Just convert to a Bytes and clone it as much as you want.

In the second case, you are cloning a stream, which should be cheap, if not then collect the stream into a Bytes and then you are at case 1.

1 Like
7

Looks like I'm going to have to implement Into/From for bytes::Bytes, so at that point I might as well just do it for Rc:

error[E0277]: the trait bound `reqwest::Body: std::convert::From<bytes::Bytes>` is not satisfied
   --> src/api.rs:264:94
    |
264 |                 let mut res = client.put(signature.url.as_str()).headers(header_map.clone()).body(buff_bytes.clone()).send();
    |                                                                                              ^^^^ the trait `std::convert::From<bytes::Bytes>` is not implemented for `reqwest::Body`
    |
    = help: the following implementations were found:
              <reqwest::Body as std::convert::From<&'static [u8]>>
              <reqwest::Body as std::convert::From<&'static str>>
              <reqwest::Body as std::convert::From<std::fs::File>>
              <reqwest::Body as std::convert::From<std::string::String>>
              <reqwest::Body as std::convert::From<std::vec::Vec<u8>>>
    = note: required because of the requirements on the impl of `std::convert::Into<reqwest::Body>` for `bytes::Bytes`
8

What version of reqwest and bytes are you using?

9

BTW, in general, when you pass ownership to a function it means the function will free that object however it wants, whenever it wants. That vec will be destroyed before you see the error, and there's no way to resurrect it.

In such cases the library has to cooperate to return the object back on error, or accept a reference, or Cow, or Arc. Otherwise objects have only a single owner, and when they're gone, they're gone.

10

To elaborate on that, @wspeirs: I suggest you actually submit a pull reqwest so that the error variant gives you back the (unused) parameter. This will probably improve experience for the other users of the library as well. (I do realize this is not an easy or immediate solution, but I think it's the proper one.)

11

This might be hard, since the error doesn't come out from the function in question itself (i.e. not from the function which directly consumes the parameter), but later, when the request is called. Perhaps reqwest could hand back the whole body in case of error, but, again - what is an error here? Should it hand the request body back, if server returned code 500? Or 404? Or 451? Or 204?

12

Why not make it hand back the entire parameter? I mean I wouldn't make the effort to try and decompose the parameter because that can have arbitrary semantics (and varying level of usefulness). An API design principle I like to follow and see being followed is "do not lose/restrict information unnecessarily". So if the error just contained the whole parameter itself, I'd think that would be enough and sufficiently general.

13

It is certainly nuanced. The function signature is: pub fn body<T: Into<Body>>(self, body: T), and there are implementations for buffers (Vec/slice), but also for files and other such streams. That's probably the biggest issue: what do you do if you've consumed 90% of a stream, then get a timeout or IO failure of some sort? For my case, a buffer, it's easy to re-use it. I guess that'd be the only real improvement to make, have a separate method in RequestBuilder that takes a buffer by reference.

Happy to contribute back, just not 100% sure how to improve it... thoughts?

14

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.