Propagate error in closure

I'm facing a (beginner) issue that is probably quite simple. It's similar to

– but I suspect even simpler as I'm not dealing with iterators. A contrived example looks like this:

use anyhow::Result;
use std::str;

fn get_bytes() -> Result<Option<Vec<u8>>> {
    let bytes = b"Hello world!".to_vec();
    Ok(Some(bytes))
}

fn text_from_bytes(bytes: Result<Option<Vec<u8>>>) -> Result<Option<String>> {
    Ok(match bytes? {
        Some(v) => Some(str::from_utf8(&v)?.to_string()),
        None => None,
    })
}

fn main() {
    println!(
        "The text is: {}",
        text_from_bytes(get_bytes()).unwrap().unwrap()
    );
}

Basically, the get_bytes() function returns a value that's either a byte vector, or None, or an Error. The text_from_bytes() function should map this as follows:

  • If the input value is an error, this is propagated
  • If the input value is None, this is also propagated
  • If the input bytes can be decoded as UTF-8, the result is returned as a string
  • If the input bytes can't be decided as UTF-8, that error is also propagated

The implementation above already does that. But I feel that text_from_bytes() is more verbose than it needs to be. I feel that I should be able to implement it as a one-liner with a map() on the input Option, like this:

fn text_from_bytes(bytes: Result<Option<Vec<u8>>>) -> Result<Option<String>> {
    Ok(bytes?.map(|v| str::from_utf8(&v)?.to_string()))
}

but I get into trouble with the error propagation from the closure:

error[E0277]: the `?` operator can only be used in a closure that returns `Result` or `Option` (or another type that implements `FromResidual`)
  --> src/main.rs:14:41
   |
14 |     Ok(bytes?.map(|v| str::from_utf8(&v)?.to_string()))
   |                   ----------------------^------------
   |                   |                     |
   |                   |                     cannot use the `?` operator in a closure that returns `String`
   |                   this function should return `Result` or `Option` to accept `?`
   |
   = help: the trait `FromResidual<Result<Infallible, Utf8Error>>` is not implemented for `String`

I gather that I can't propagate an error in this way, and I more or less see why (the closure itself should return Option; it can't return a Result). But I'm not sure what the best solution is. Any pointers?

After mapping the Option with a fallible function, you can transpose the Option<Result> into a Result<Option> and use ? on that.

Ok(bytes?.map(String::from_utf8).transpose()?)

Alternatively, someone might prefer to explicitly convert the error than use ? and wrap the result in Ok()

bytes?.map(String::from_utf8).transpose().map_err(Into::into)
3 Likes

A third solution is to use map_or with a closure:

bytes?.map_or(Ok(None), |v| Ok(Some(String::from_utf8(v)?)))

The Ok(Some(result?)) is effectively doing a transpose.

You can also transpose first to move the Result to the inside, do all of the Result-y business there, map it over the Option, and then transpose it back to get the correct signature:

fn text_from_bytes(bytes: Result<Option<Vec<u8>>>) -> Result<Option<String>> {
    bytes
        .transpose()
        .map(|v| Ok(String::from_utf8(v?)?))
        .transpose()
}
1 Like

Thanks all for the responses! I see that transpose is the magic word here - and that makes a lot of sense.

If you'll bear with me, in my desire to learn, I have a couple of follow-up questions related to the above. Firstly, is either of

Ok(possible_error?)

vs

possible_error.map_err(Into::into)

considered idiomatic / "better style"? Or is it really down to personal preference? I'm coming from the world of Python where there is often a generally accepted idiomatic approach, so am trying to get the hang of similar unwritten rules in Rust.

And finally, is Result<Option<Vec<u8>>> generally the right way to represent something that could be None, an Error, or some data - or is there potentially a better way? For context, in my case this is the result of a SQL query; Ok(None) indicates that query returned no results, while Error indicates an error while executing the query. Given this, it doesn't feel right to switch the order of Option and Result. Though it does seem that a switch like that could make some of the subsequent data manipulation code more straightforward. Thoughts?

One thing you might consider is thinking about why you have the nesting, and whether different strategies might work better.

You've admitted this is contrived, so I don't know how well the following will apply to your actual code, but...

For example, it's somewhat odd to have a function that takes a Result as an argument. The Ok(match bytes? { part, for example, could just be match bytes { if it didn't take a Result as an argument. The "if you want to run this inside a result" part could maybe be delegated to an and_then in the caller. In general, you wouldn't want someone to have to text_from_bytes(Ok(blah)) to call your function.

Or how meaningful is the Option<Vec<u8>> to you? If you really need the distinction, then definitely keep it. But an empty Vec doesn't allocate, so you might be able to just use a Vec<u8> without the Option and save yourself some of the trouble.

2 Likes

I concur with scottmcm. I'm not an experienced Rust dev, but the thing that looks off to me is that text_from_bytes takes in a Result<Option<Vec<u8>>>, rather than an Option<Vec<u8>> (or even just a Vec<u8>).

I suppose I should keep my uninformed opinion to myself, but most code I see would look more like this:

fn text_from_bytes(bytes: Option<Vec<u8>>) -> Result<Option<String>> {
    Ok(match bytes {
        Some(v) => Some(str::from_utf8(&v)?.to_string()),
        None => None,
    })
}

fn main() -> Result<()> {
    println!(
        "The text is: {}",
        text_from_bytes(get_bytes()?).unwrap().unwrap()
    );
    Ok(())
}

If all the text_from_bytes method is going to do is unwrap the Result, it doesn't seem necessary to do that inside the method, as opposed to catching and dealing with the error beforehand.

(Playground version)

Thanks @scottmcm and @KSwanson what you're saying makes a lot of sense. The overall context is that the Vec<u8> comes from a SQL query - which is why it could also be None (record doesn't exist), or Err (the query raised some other kind of error). In other words, the Result<...> input is not something that a user (or main()) would produce, but is the result of a query.

Here is a playground with some broader sample code, the key bit being:

impl Db {
    fn select<K: AsRef<str>>(&self, key: K) -> Result<Option<Vec<u8>>> {
        self.0  // This is a rusqlite::Connection
            .query_row(
                &format!("SELECT {VALUE_FIELD} FROM '{TABLENAME}' WHERE {KEY_FIELD} = ?1"),
                [key.as_ref()],
                |r| r.get::<_, Vec<u8>>(0),
            )
            .optional()
            .map_err(Into::into)
    }

    pub fn get<T: DeserializeOwned>(&self, key: impl AsRef<str>) -> Result<Option<T>> {
        self.select(key)?
            .map(|v| serde_json::from_slice::<T>(&v))
            .transpose()
            .map_err(Into::into)
    }
}

With select corresponding to get_bytes in my initial (contrived) example, and get corresponding to text_from_bytes. The get() function is then the "external" interface presented by this code. I can't currently think of a better way to set this up, but welcome any and all suggestions!

Ooh, excellent -- a playground example that builds is perfect.

I think what I'd say is that trying to make this one expression isn't necessarily worth it.

Assuming we get let-else on stable soon, I'd probably just write it something like this:

pub fn get<T: DeserializeOwned>(&self, key: impl AsRef<str>) -> Result<Option<T>> {
    let opt = self.select(key)?;
    let Some(vec) = opt else { return Ok(None); };
    let val = serde_json::from_slice::<T>(&vec)?;
    Ok(Some(val))
}

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=db6b2f824f412150ea12fcd924438b70

2 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.