To panic or to Result

help
#1

Hello,

I sometime have some difficulties choosing between panicking or returning a Result.
Thus, I would like to gather your opinions on these examples. In theory, I can always return a Result and be safe, but it sometime feels overkill since I end up using a lot of unwrap/expect when I know that calling the function won't produce a failure. Which is, most of the time. Also, I mostly do scientific computing where a failure is almost always a sign of a bug except for stuff like IO.

For each of them, would you panic or return a Result (or maybe an Option) or do both by defining 2 functions, one that panics and the other that return a Result and let the user choose?

Also feel free to suggest other examples.

1. Matrix and vector multiplication

This should fail if the number of columns in the matrix is different from the length of the vector.

2. A function with a probability as an input

This should fail if the probability is not between 0 and 1.

3. The mean of a Vec

This should fail if the Vec is empty.

#2

I would probably panic in all of these cases, and might also provide try_ versions that return Result if it seemed likely that someone would want it. In all of these cases, because it seems very common for the calling code to provably ensure that the input is valid, returning a Result makes no sense.

There are two cases that I can think of where you must not panic:

  1. If it's something that the caller cannot possibly ensure, like opening a file. Since it's always possible for the file to be deleted right before the program opens it, your program must check for an error.

  2. If checking input validity twice would be expensive, like parsing some file format. This is an efficiency thing instead of a correctness thing, but it's probably a good idea.

Really, API design is a matter of taste as much as it is anything else. It's like any other form of writing. But if someone wanted rules to keep in mind, these would be the bare minimum thou shall not panic on these APIs guidelines.

5 Likes
#3

To my mind if the error situation is something that is totally unexpected and cannot be recovered from then an emergency stop is called for. That is to say panic.

Many other errors are caused by faulty input data, communication failures and the like, and can be handled with some kind or Result. Which can then be used to retry, do something different, inform the user and so on.

#4

You should assert! your preconditions when:

  • It's easy and cheap for the caller to check them if they want, and
  • The way your type is used means there are often algorithmic invariants that mean the condition holds, and thus if you did return a Result almost everyone would .unwrap() it anyway.

Now, you might also provide a non-panicking version (try_foo or similar) as a convenience, but those are the conditions where I'd have the shorter-and-best name be the panicking one.

So what's the contrapositive? You should not panic when:

  • Checking the conditions is expensive (like don't panic for bad UTF-8, either return Result or take a str/String instead so it can't happen).
  • The condition is something you want to be able to change easier (like a minimum length for a password, say)
  • The exact condition is complicated enough to be error-prone (though this may be indicative of an awkward API design in the first place)
  • It's impossible to pre-check the condition (file existence being the usual example, or anything going over a network)
  • It's unlikely that the programmer would "just know" that it's right (which, admittedly, is pretty fuzzy, and it's hard to come up with an example that doesn't violate one of the other rules)

See also Why does rust have [ ] indexing? - #3 by scottmcm

Yeah, exactly.

To look at your examples:

I'd just panic for this. It's extremely unlikely that the programmer just got arbitrary matricies from somewhere and wants the product without knowing what they are. They probably created them expecting to multiply them, so might as well let them -- they can check the sizes manually if needed.

(And of course for static sizes, const generics is coming soon to make this a compile-time check.)

I'd probably assert on this, because it's another situation where it's likely that math is making it true.

That said, this might be a place where a newtype could be interesting, so that you can communicate the guarantee in the type system without checking it. That'd let you do 1-p or p * q without checking, but you could have adding probabilities require re-checking (or override something to do p + q - p * q). Dunno if it'd make sense in your case, though.

This one I'm on the edge about. I'd probably say it should either return an Option (as other statistics like Iterator::min and friends do) or return NAN (as sum / count would).

2 Likes
Why can I call .iter() on an immutable empty array?
#5

If you don't mean to otherwise handle arbitrary matrices and vectors (i.e. when you control the matrix and vector size, so it is trusted, and it does not come from user input), you could panic on this one.

Alternatively, if it's possible to know the size statically, you should probably encode this invariant using types instead.

My advice is basically the same here, except that this one is much easier to enforce using types. You can write a newtype wrapper around f64 of which the constructor returns an Option<Wrapper>, and yields None if the probability value is not between 0 and 1. Then you could pass this wrapper down to any probability-accepting functions, which can now safely rely on it being in the unit interval.

I'd say follow the standard library here. Iterator::min() and Iterator::max() return Option<T>, which is None if the iterator happens to be empty.

2 Likes
#6

Panic. This is almost certainly a logic error. Even if the vector and/or matrix happen to be user input, the compatibility of dimensions should be validated before attempting to multiply. It would be optimal to enforce the compatibility at compile time, using the type system, but that's not always feasible.

A probability outside [0, 1] is a logic error, an impossibility, and most preferably one would create a newtype for it that enforces that invariant, rather than just passing a naked f32 or f64.

Option. Without context, taking the mean of an empty vector is not necessarily an error or an impossibility, the mean simply doesn't exist. Option can be turned into a Result or a panic easily enough by calling code which hopefully knows more about the context.

2 Likes
#7

I mean, I can't disagree with you :sweat_smile:

1 Like
#8

Awesome, thanks everyone for all the answers. Pretty much what I thought, but this sentence from the rust book made me doubt

In rare situations, it’s more appropriate to write code that panics instead of returning a `Result

Maybe, scientific computing and linear algebra are some exceptions where it is often more reasonable to panic instead of returning a Result.

#9

There is an important security principle called "death before confusion." If you reach an unexpected state, you may be vulnerable to an attacker. My code has a gazillion places where I know an error can occur that return a Result, and a couple of dozen places where panic is the right answer.

1 Like
#10

Write all quickly. i.e. no use of Result no explicit checks.
Test for expected result.
Satanize the input you run on.
Don't give your code to anyone.

Of course I differ as not in the same field.
There is a big problem in asking such a question in that answers may get more thought than typical reality.

#11

I agree with the consensus here, but I would add that you shouldn't worry about writing unwrap. If you write your main function to return a Result (anyhow::Result<()> is a good choice), you can just use ? throughout your code.

1 Like
#12

I often refer to Vexing Exceptions to try to categorize error conditions. Not everything there applies to Rust, because the author was working on C# at the time, and Rust doesn't have exceptions per se, but I think the categorizations are useful.

If I may translate Lippert's advice to Rust: fatal and boneheaded error conditions are appropriate to turn into panics; if panicking would be vexing or the error is exogenous, you should return a Result.

Considering your three scenarios in that light:

  1. is almost certainly boneheaded, for reasons already discussed. Panic.
  2. is also a boneheaded error, for pretty much the same reasons, and also if you're reading probabilities from the user you should validate them at input time so nonsensical numbers don't ever infect your number-crunching code. Panic.
  3. would be vexing (that is, if it panicked) because querying an empty vector for its mean is not necessarily a bug, and the correct answer when the vector is empty is "the mean does not exist", i.e. None. Return Option. (NaN is an interesting idea, and that's viable; I think which one you use probably depends on what kind of number crunching you expect it to be used for.)

(The system also works for @notriddle's "must not panic" examples, which would be exogenous and vexing, respectively.)

tl; dr: I agree with the consensus and back it up by appealing to a blog post from 2008

1 Like