Unwrap in libraries


#21

splitn(2, ...) on nonempty Vec return iterator with either 1 or 2 elements.
Thus first unwrap never fails. But second element might or might not be present and thus we need to use unwrap_or.

Next time please think at least three times before asking such question. It usually has really simple answer.


#22

What about OOM panics? Are you suggesting that all methods that allocate should return an error? It will be a nightmare, at least without exception… But there are a lot of languages with exceptions which are still crashing constantly, so it isn’t a solution.

Unwrap in a library on contract violation is fine.


#23

Hi, I wrote this code. In this crate specifically, to_u32 is used with the ? operator (propagating the error to the caller) when creating a Url (during parsing). The specific function you link to takes a &mut Url parameter and accesses its private field, so it assumes that the parser has already checked that this string is not longer than 4 GB.

Now, maybe it is possible to construct an edge case where the parser fails to check the length of the string in enough places and this .unwrap() ends up panicking. But that means you’d have found a bug in rust-url that should be fixed. It doesn’t mean that every user should use catch_unwind everywhere.


#24

Except that doing it the proper way depends on having a certain minimum degree of confidence that the codebase won’t wander off down some hard-to-trigger code path in the middle of a long run.

Otherwise, there’s a risk of "catch_unwind plus a UPS" becoming the "except BaseException: plus a UPS" guard of the Rust world as a “lazier but good enough” alternative to making the long-running task properly fault tolerant and resumable using some mechanism like journalling.


#25

If you run for example something like an HTTP server it sounds reasonable to to use catch_unwind so that if a bug makes something panic, one request fails (and returns an HTTP error) without bringing the whole server down. But that seems more like the exception than the norm? You’d do it in very few places.


#26

That is correct. I am not suggesting use of catch_unwind. In fact what I suggest is that the client code should not come to a point to use catch_unwind.

If the proper way of error handling in Rust is using Result/Option, then there should not be any call to panic! (directly or indirectly) in a library.

The problem with unwrap is that if it is not examined carefully to make sure it would not result in panic, then it would decrease robustness/resiliency of the whole application. I case of you code, you are correct and as you explained panic would not happen in to_u32, simply it was a bad example that I picked.

Using unwrap is an easy way to deal with errors and it is very tempting to use it everywhere. If you look at sample codes provided by many library authors, unwrap is used for dealing with errors which makes sense since that sample code is supposed to show the concept and using unwrap makes the code simpler to understand and less verbose. However if you are a new user of Rust, it gives you impression that is the way of dealing with errors in Rust.

Essentially unwrap is a footgun. Rust has made a good reputation that its compiler stops you from doing anything unsafe (unless code author explicitly indicates that with unsafe keyword).
So what I suggest here is that Rust compiler could go further and not only improves safety but also robustness/resiliency and I am not sure if “unwrap” helps in that direction.


#27

unwrap, when properly used, is a manifestation of a runtime invariant being violated. unwrap is as much a footgun as bugs themselves are a footgun.


#28

Except that, for lack of a panic-auditing helper, I’m considering making it standard practice to have my “shell scripts in Rust” either catch_unwrap or spawn a single worker thread just to guard against panics in any batch-processing task where I’m relying on a 3rd-party crate to process each file in the batch and I don’t want to have to journal whatever rolling calculation I’m maintaining as I work through the batch.

(And, when I say “or a worker thread”, I mean spawning the worker thread purely as a panic-catching boundary, rather than because there’s unutilized capacity that parallelism could leverage.)


#29

I’m sympathetic to @Bijan’s point. As much as we’d all like to enforce safety via the type system, it’s often either impossible or very unwieldy. As such, there will be a healthy dose of runtime checks in any significant application. Pushing all errors to the caller doesn’t make sense - some of them cannot be handled by them; they may not even know what they mean. And ultimately, the puck has to stop somewhere - if an invariant is violated (ie a bug) then someone somewhere has to either panic or do something about it. The further you move away from the spot of the violation, the less likely it is that the error can be handled in any reasonable manner, and moreover, the less likely it is to be easily determinable whether it’s even ok to continue running the application.

So, I think these things are a fact of life. Rust can reduce but not eliminate them. Errors should be surfaced only when it’s expected the caller can do something about them.

Being able to exploit the type system to reduce runtime errors is sort of the motivation for me posting https://users.rust-lang.org/t/examples-of-modeling-a-domain-with-strong-type-safety-expressiveness-and-performance.


#30

I just wanted to point out the issue of wrapping a 3rd party library call in something like catch_unwind is not limited to Rust.

For example in C# or Java which the main way of error handling is through exceptions, if a method of a class in a library throws an exception, you as a user would not know about that unless it is documented (or looking at the library code itself, assuming you have access to the source code of the library).
So basically the user would be at the mercy of documentation and documentation could be out of sync with the code (it is just matter of time) and therefore to be on the safe side you have to wrap that in try-catch block which makes your code verbose and ugly or the risk of unhandled runtime exception.

Note: Although Java supports checked exceptions which forces users to handle the exceptions or change the signature of the method, it is not very popular feature and many libraries derive their exceptions from RuntimeException.


#31

Except that, in Python (which is what Rust is replacing for me), I already have a try/except construct at the top level of the per-file code for things like “access denied”, so I just catch Exception instead and call it a day.


#32

I’d argue that’s exception handling (in the literal sense), but isn’t robust error handling. I’ve seen a similar pattern in C# and Java - some top-level handler that, at best, logs an error and then pretends like nothing ever happened. And I’ve also seen this lead to really unstable systems that continue running with corrupted state, oftentimes leading to painful debugging sessions, trying to figure out how some state could end up a certain way.

I’m not advocating for excessive unwraps, expects, and panics. In fact, IIRC zeromq had a similar problem at some point: it would abort a bit too happily, bringing down the entire application. But I think trying to surface every possible exception as a user-visible error doesn’t make sense either - it’s the other end of the spectrum. The trick (and the difference between first class libs and the rest) is finding the right (and appropriate) mix.


#33

OK, I agree. Now the question is how to know if that is used properly.
I guess there is a philosophical issue with using unwrap. A function that returns Result indicating there is possibility of failure and I cannot handle that myself. When the caller calls that function and unwrap it, effectively it says I do not agree, I am looking for the correct result always.


#34

Unwrap (or expect or anything similar) is essentially an assert. I think we’d all like to encode invariants into the type system and catch mistakes at compile time but for a variety of reasons some conditions will need to be checked at runtime. These assertions are basically cases where the author is saying they have reason to believe the result is always a success but they couldn’t verify that any other way than dynamically.

A Result return type is a proper error API - it’s not so much about internal invariants being violated but expected error conditions that a caller may know how to deal with (or they’ll panic/unwrap/etc but not you as the library) or would want to be informed about in a structured manner.


#35

Speaking of Option<>s though, I always felt uneasy about using unwraps for known good case but I could never figure a proper way to handle them.

By following the return-early paradigm, I want to first check the invalid values (like they do with error handling in go). I like the typescript approach where the compiler narrows down the type the more checks are done, e.g. for

const a: string|undefined = ...;
if (!a) {
  return ...;
}
// a's type is string here.

I would want to write it in rust this way:

let a: Option<String> = ...;
if (a.is_none()) {
    return ...;
}
let a = a.unwrap();

Is there any way to do the asme without unwrap and when try! doesn’t cover the return case completely?


#36

I think pattern matching is the closest you can get to this:

let a: Option<String> = ...;
if let Some(a) = a {
    // a is now a String and you can use it as such
}

I realize that this causes extra indentation and that it’s the direct opposite of the early return pattern you mentioned :slight_smile: But it’s nice since the structure of the code – the scopes really – ensure that you cannot call unwrap on a None value by mistake.


#37

Using match and redefining a takes it closer to the narrowing version:

let a: Option<String> = ...;
let a = match a {
    Some(a) => a,
    None => return
};
// the new (shadowing) a is now a String and you can use it as such

This is basically the same as what the try! macro and the ? operator does. There’s even talk about (not sure how far it has come) extending ? to work with Option.


#38

? already works with Option if the function returns an Option, e.g.:

fn bar(s: Option<String>) -> Option<usize> {
    let s = s?; // returns None if `s` is None, `s` is now String
    Some(s.len())
}

There are also combinators on Option (e.g. map, unwrap_or, map_or, etc) that can make working with Options more pleasant.


#39

Oh, you are totally right and now I remember reading about it! Oops!


#40

I realize that this causes extra indentation and that it’s the direct opposite of the early return pattern you mentioned

Using match and redefining a takes it closer to the narrowing version:

Exactly, and that’s what I don’t like about it. Rust forces me to either do deep nesting or to rewrite blocks into isolated functions, either of which has its downsides.