I am a bit confused about using unwrap. From what I have read and understood so far every function which could fail should return either Result or Option and unwrap/except is more appropriate for tests/prototypes. But that is not the case in the libraries that I have seen so far, unwrap is everywhere (even in tokio, servo...). Just as an example (Ref. 2, below) if to_u32 fails, the new function panics and the caller thread terminates unless the caller wraps the call inside panic::catch_unwind. To summarize it looks like if you are using a 3rd party library then you should examine every function to see if it uses unwrap and if so then wrap it in a panic::catch_unwind which would be ugly. Is that correct? Am I missing something?
unwraps are fine so long as a failure indicates a bug that should be fixed. The advice "don't unwrap in a library" is pithy, and a good first approximation, but ultimately is not sufficient. The actual advice should be, "libraries should not use panics as a means of error handling." When a library properly unwraps, it's generally because there is an invariant that is either impossible or difficult to move into the type system. Libraries, may, on occasion, shift the burden of ensuring panics don't happen to callers. unwrap is itself one such example. Another word for this might be "contract violation."
Some applications, like web servers, may want to use catch_unwind at a high level if they want to prevent a bug from taking down the entire web server (assuming the application has enabled panics to unwind, which is the default, but panics can also be made to abort). But catch_unwind should generally not be littered throughout your code, and it is almost always wrong to use catch_unwind in a library unless you have a very good reason for requiring that panics must unwind. (QuickCheck is one such example, since it is specifically designed for testing.)
Thanks for the explanation. However the issue is how to know if unwrap has been used in a library safely. If an API returns Result it means the caller should deal with it not silence it with unwrap.
This is a serious issue for Rust. It affects robustness and reliability. Probably unwrap and expect should be limited to testing only and should be enforced by the compiler.
a + b, a - b, a * b, a / b, a % b, and a[b] can all panic. Is your proposal to mandate that any function that does math or indexes into a vector needs to return a Result?
Not all failures are able to be handled, or meaningful to be passed, in a manner other than panic.
Unwrap is useful in a lot of places where it can be statically proven that the unwrap can't panic (for example, a pre-checked parse), and equally useful in places where the only possible resolution of a failure is termination.
If you're encountering panics when consuming a library that shouldn't be occurring, and have a reasonable way to handle them, then yeah; file an issue saying "hey, you're aborting the process too early"
You can also establish the precondition yourself: if a library function panics on you and you need it to not do that, and have a way of dealing with failure, filter your calls to it. Before passing in a value that may induce failure, check it, and only call the function if it's valid; if not, do your recovery process immediately rather than wait on the library to do its work and then fail.
Probably unwrap and expect should be limited to testing only and should be enforced by the compiler.
is simply untenable. The fastest way to get past this lint is to replace all .unwrap() with .or_else(|_| panic!()), and now the lint passes and nothing has been gained.
Erroneous panics are a bug. The existence of .unwrap() is not. Punting all error resolution to the client is equally a smell.
The concern is very valid. A well-written library will only panic on contract violations or unrecoverable errors, but not all libraries are well written. However, if you get the point of papering over such bugs with catch_unwind, you should probably use a different library.
Functions only fail (Result::Err) when given a subset of possible inputs*. When programmer gives specific input, their unwrap is (hopefully) saying I intend the input will never make the function fail. There is then no error to report. (Of cause they could have made a mistake.)
(*inputs covers not just the function arguments but other communication such as outcome of file reads and writes.)
My take on this has always been that we need some kind of static analyzer which will walk your tree of dependencies and report all places where your call graph may be able to transitively reach a panic!. (Sort of the next step after turning on Clippy's lints for unchecked math.)
Then, it'd be much easier to manually audit such cases and add them to the tool's whitelist.
In the end, you'd be left with a setup where your integration tests could be set up to fail if an un-audited panic! slips in.
Let me give an example, in form_urlencoded.rs from servo/rust-url (form_urlencoded.rs), next function returns OptionSelf::Item which is great, so as a caller of this function I know the result could be None and I would handle that. However inside that function there are unwraps (unwrap call) which could panic. Now considering that function returns Option, line 100 could have returned None instead of unwrap in case of failure.
That would be completely wrong for an iterator and would lead to bug propagation away from source. If an iterator were to support failure then the Item would be a Result.
Lets take example you link (calling to_u32). The functions converts usize (which might have 32, 64 or some other number of bits depending on architecture) to u32. It fails, if input is greater than 2^32.
It is quite obvious that url.serialization.len() would never ever be greater than 2^32. So it is safe to unwrap.
You've actually found a case where unwrap will not result in a panic, since input was determined to be non-empty before. Calling unwrap on the next line could panic, but there's unwrap_or, which will not panic.
form_urlencoded is another great example.
You call split in Vec (which is not empty). Split on nonempty Vec returns iterator with at least one element.
And then you call next on the iterator, with at least one element. This will never fail! You can unwrap!
I agree that in that particular case next would not result in panic, but why the next one could where unwrap_or has been used.
If you try below code (I replaced unwrap_or with unwrap) it would not result in panic.
let str1 = Vec::<u8>::new();
let mut split2 = str1.splitn(2, |&b| b == b'&');
let sequence = split2.next().unwrap();
let sequence1 = split2.next().unwrap(); // self.input = split2.next().unwrap_or(&[][..]);
println!("{:?}-{:?}", sequence, sequence1);
a + b, a - b, a * b, a / b, a % b, and a[b] can all panic. Is your proposal to mandate that any function that does math or indexes into a vector needs to return a Result?
If a function takes a numeric as input and that could be zero, it should be checked first and the function should return an error saying that input is not acceptable instead of panic.
This isn't always that easy. There were times when I needed something like this:
if foo.is_some() {
use(foo.unwrap())
}
This trivial example can be written with if let instead, but in more complex cases when multiple variables and lifetimes are involved, such pattern becomes convenient.