Best practices for unwrap

help
1

Is it ever acceptable to use unwrap()/expect() in rust production level code? The more articles I read, the more opinions I see about avoiding unwrap at all costs but it seems as though most crate functions return a Result or an Option type, which would result in so many match or "if let" statements if unwrap/expect is not used.

For example, say I had a function that has 5 different function calls in it that all return a Result type. Would it be best practice to handle every single one within the function with a match statement or would you normally just propograte up the error to the caller by returning Result<(), Box>?

Using Unwrap

fn example() {
function1().unwrap();

function2().unwrap();

function3().unwrap();

}

Propagating Errors

fn example() -> Result<(), Box<dyn std::error::Error>> {
function1()?;

function2()?;

function3()?;

Ok(())

}

Catching Each Error

fn example() -> Result<(), Box<dyn std::error::Error>> {
match function1() { // Or could use "if let Err(e) = {}
     Ok(()) => info!("No error occured"),
     Err(error) => {
            error!("Error while executing function1: {}", error);
      }
};

match function2()  { // Or could use "if let Err(e) = {}
     Ok(()) => info!("No error occured"),
     Err(error) => {
            error!("Error while executing function2: {}", error);
      }
};

match function3() {  // Or could use "if let Err(e) = {}
     Ok(()) => info!("No error occured"),
     Err(error) => {
            error!("Error while executing function3: {}", error);
       
};

Ok(())

}
2 Likes
2

This When is unwrap() ok? discussion may be useful. It includes a link to a blog post on the same topic written by BurntSushi.

9 Likes
3

If you believe that the error can never happen, use unwrap or expect. In other words, if the only way for the error to happen is that there is a bug in the code somewhere, panic. Don't propagate errors resulting from bugs.

Propagate if the error is a valid possibility, i.e. if it's a defined part of the function's API.

19 Likes
4

The post by burntsushi is very good, and I just want to add that if you have a multithreaded or async program, it is important to remember that threads and tasks are panic boundaries, so you should have some means of propagating them. That is, if you're using expect when it would only fail due to a bug, you should probably expect the JoinHandle of the corresponding thread if you can.

1 Like
5

Yes, sometimes (rarely).

That is simply false.

You should propagate most errors using the ? operator. Do not make decisions on behalf of the caller — they won't be able to change the behavior if you hard-code an unwrap into your own code. Most errors can occur for whatever reason – design for robustness, don't just design for the happy path. That's why ?-bubbling was invented. Don't try to if let every single error, that's a code smell in itself (it's what Go does and it hurts).

The situation when it is acceptable to use unwrap()/expect() is when the structure or logic of your code makes it provably impossible for the error case to occur. For example:

  • it is never OK to unwrap an I/O error
  • it is never OK to unwrap an error resulting from parsing arbitrary malformed user input (eg. str::from_utf8() if the input is a byte stream provided by the user or read from a file)
  • it is OK to unwrap a slice.first()/slice.last() or a Vec::pop() when you are maintaining a data structure that mandates a known non-empty backing storage
  • it is OK to unwrap the result of parsing some statically known correct input, eg. the return value of Regex::new("literal regex")
13 Likes
6

Thank you for the detailed answer, I agree that in most circumstances it is best to have the caller decide what to do. But, by propagating errors via ? to the caller, doesn't that cause generic error displays which would be a bit more difficult to debug?

For example, say I had a function that did:

  1. Used reqwest to GET a value let response = client.get(url.clone()).send()?;
  2. Parse that returned value as json let subjects: Vec<String> = response.json()?;
  3. Do a cast of each String in the Vec to another type (say u32 for example)

Each one of those operations results in an .unwrap(), which would propagate up to the function caller because of ?. If the function caller just does a generic error!("Failed to fetch data and parse to u32");, doesn't that provide a little less context as to what actually went wrong (i.e. fetch vs json parse vs cast)?

7

I think you misunderstood what the ? operator actually does. It doesn't do an unwrap, it just a short hand for early returning in case of an error, essentially:

fn my_func() -> Result<(), SomeError> {
    // essentially the same as doing let foo = result?;
    let foo = match result {
        Ok(value) => value,
        Err(error) => return error,
    };

   // ....
}

There is no missing detail, the caller will get the same error type you did and will decide what to do about it.

I hope this helps, I'm not sure I understood your question.

8

Ah yes, you're right. I did misunderstand the use case. Makes sense now, thank you for clarifying!

1 Like
9

I have no idea what you mean by that. What an error's Debug or Display impl displays has exactly nothing to do with how you obtained the error instance itself.

10

No.

11

My usual response: It's acceptable whenever you as the developer would be happy getting a "you have to investigate this today" bug about it.

If I'm pretty sure something isn't possible, it should shout loudly if I'm wrong so I can go fix that. But I also shouldn't be forced to handling things that never happen in practice, because making people write a bunch of never-tested error handling paths reduces both velocity and reliability.

(Assuming you have a reasonable path to get telemetry about the failures and deliver updated builds, as well as an elegant handler to turn things into 500s or save recovery information or whatever. But those are all things you ought to have no matter what you're doing, because lots of things might panic and they're just good anyway.)

1 Like
12

As far as I am concerned unwrap() is an admission that I have no clue as to how to proceed in the face of that error. Or a statement that if that error happens there is no sensible way to recover. Either way it's better to abort immediately rather than try and limp along in a broken state. It's probably better to use expect() to help with future debugging.

1 Like
13

I'd say that you should only use unwrap if you're absolutely certain that it will either:

  1. Never fail
  2. Only fail if something is so catastrophically wrong that the best courseof action is to crash the entire program.

And in both of those cases you should always write a comment about why using unwrap is OK.

But, expect is essentially unwrap with an included comment so you might aswell just use that because it crashes with the comment included.

And once you have written your comment declaring the reason why unwrap/expect is OK. Read it through three times and consider whetheror not your reason is actually valid.

1 Like