20 ms·

Using unwrap() in Rust is okay

212 points by cp9 4y ago

dcsommer 4y ago
I totally agree with de-emphasizing the old "recoverable" vs. "unrecoverable" dichotomy (https://blog.burntsushi.net/unwrap/#what-about-recoverable-vs-unrecoverable-errors https://blog.burntsushi.net/unwrap/#what-about-recoverable-v...). Every time I've heard programmers (especially in the context of exceptions) try to define it, I've found it imprecise and open to debate. When invariant violations or mistakes by programmers (aka bugs) are detected, the program should halt as it is in an inconsistent state and continuing could be very dangerous (think privacy/security/data corruption). Otherwise, don't halt (handle it or have the caller handle it).
- arcticbull 4y ago
  Yep, there's not really any such thing (IME) as a 'recoverable' error - except with respect to I/O. There's either I/O errors - or there's logic errors. A failure with logic should nuke due to the app being in an inconsistent state; trust is lost. An I/O error should fail softly.
  - ithkuil 4y ago
    A parser library can return an error when the input is wrong. It doesn't necessarily mean the app is in an inconsistent state when that happens; it all depends on what the application does. It follows that aborting is often a sensible decision in application code and rarely in library code.
    - arcticbull 4y ago
      I would personally consider parsers as part of I/O, but point taken.
  - fatherzine 4y ago
    Partially true. In practice people implement multiplexed servers, for many reasons, including performance / throughput. A logic failure should nuke _the offending request_, not the entire server with all the unrelated concurrent requests.
    - AshamedCaptain 4y ago
      This is an attitude that I often see -- library authors who believe they own the process. Aborting may be the only sensible thing to do in runtimes where you can end up corrupting the entire process memory or the like, making recovery "dubiously possible", but absolutely not for anything higher level, where recovery may be safe and possible.
      - burntsushi 4y ago
        I gave examples covering this area in the post. How would you rewrite the code in this section[1], for example, to conform to your views? (Assume this code is in a library.) [1]: https://blog.burntsushi.net/unwrap/#what-about-when-invariants-cant-be-moved-to-compile-time https://blog.burntsushi.net/unwrap/#what-about-when-invarian...
        fatherzine 4y ago
        * Short term: add explicit runtime checks for every step that may panic. With the way jump prediction works in modern processors, the runtime cost may be smaller than one may naively assume it is. Some of us would take, say, a 10% perf. degradation instead of chasing prod panics in the middle of the night. * Long term: plug-in a richer type (logic) system, so one can safely prove the most costly runtime invariants at compile time.
        burntsushi 4y ago
        > add explicit runtime checks for every step that may panic And do... what when the check fails? Can you please write the code for it? Because I don't understand what the heck you mean here. If you don't know Rust, pseudo code is fine. > Long term: plug-in a richer type (logic) system, so one can safely prove the most costly runtime invariants at compile time. This is just copying what I already said in the blog post. Until someone can show me how to prove the correctness of arbitrary DFA construction and search from a user provided regular expression pattern and demonstrate its use in a practical programming language like Rust, I consider this a total non-answer, not a "long term" answer. Basically, your answer here confirms for me that your views on how code should be structured are incoherent.
  - jcelerier 4y ago
    > There's either I/O errors - or there's logic errors. A failure with logic should nuke due to the app being in an inconsistent state; trust is lost. nah. in GUI apps for instance you want the failure in the logic of a sub-sub-function to just tell the error "wops" when the button that triggered the action was clicked, not nuke the app (unless you hate your users). e.g. imagine a 3D software which allows to do mesh operations - user clicks on the "Smooth the mesh" button somewhere. Programmer forgot to handle a division by zero in some degenerate case of the smoothing computation which ends up leading to an exception: a value becomes zero, someone used unsigned integers for n in an "n - 1" computation which ends up in a call to array_of_floats.resize(0xffffffffffffffff) (and a likely std::bad_alloc being thrown if you're in c++). The original mesh is unchanged as the operation waits until the computation is complete to replace the old mesh with the new. If you ever decide to crash in this situation I am sure you will have great reviews on 3D modeling software comparisons.
    - arcticbull 4y ago
      I would actually want that to crash, yes. That would ensure it gets caught and resolved by developers during development. Or at least increase the odds thereof. Further, if it allocates a few gb, or if it's allocating a large amount of memory because the size parameter got smashed? If it crashes enough that you'd get terrible reviews it would definitely be caught during development. If it messes up your mesh silently then you'll definitely still get bad reviews. And actually having an std::bad_alloc thrown is even worse since you have no idea what state it left things after running some destructors when you weren't planning for it.
      - jcelerier 4y ago
        > That would ensure it gets caught and resolved by developers during development i don't know in which reality you live but in mine there's not much relationship between the existence of crashes, and them being resolved during development > And actually having an std::bad_alloc thrown is even worse since you have no idea what state it left things after running some destructors when you weren't planning for it. i don't even know what to say. that's the whole point of destructors - you know that things will be unwound in the reverse order from which they were created on automatic storage. that's, like, why c++ exists
- hinkley 4y ago
  Recoverable vs unrecoverable comes down to requirements. Certain companies known for having software that 'just works' tend to have both very few unrecoverable errors and very conservative feature sets to help facilitate that short list. It is very clearly a choice, even if many people are deciding by default. By not tackling an issue, you've chosen to have that issue.
  - dcsommer 4y ago
    I think we have compatible views. Each layer of the software must decide it's requirements and handle errors appropriately per requirements. You're right I didn't articulate when to handle an issue locally vs. pass it up. I think that's where requirements (and also explicit API guarantees) come into play. I do think that APIs that "overpromise" by not returning the errors they do not handle to the caller, and instead halt or throw an exception, do their users a disservice in the long-run. These just become undocumented cases that bite you later on. Better libraries have all these conditions baked into the API itself.
    - tsimionescu 4y ago
      But an exception is a part of the API, why are you putting it at the same level as halting?
      - dcsommer 4y ago
        In my experience, APIs that throw rarely define all the exceptions that can come from it, especially transitively. I see exceptions as a failed (because undocumented, but still important for correctness) attempt at compromising between halting and returning an error.
        hinkley 4y ago
        I wonder if there's a moral equivalent of borrow semantics where we more formally define error propagation.
- jcranmer 4y ago
  The criteria I tend to prefer is "expected" versus "unexpected" errors. I/O errors, especially network errors, are things that are going to be expected under reasonable operation, therefore it make sense that code should handle them. Similarly, user input resulting in incorrectly formatted code should be reasonably expected and therefore handled. But the same kinds of failures might not be reasonably expected in other circumstances--I wouldn't expect that the internal configuration files of an application should occur in reasonable operation, and therefore it makes sense to panic if they're corrupted... even if the cause is an I/O operation on a local disk, or parsing some JSON or TOML or INI or whatnot file. One implication of this is that it needs to be easy for any error system to promote an "expected error" into an "unexpected error"--which is what unwrap/expect does. The recoverable/unrecoverable error suggests that there ought to be no reason to do this, but there is absolutely a reason to do so: what category an error falls into is ultimately decided by the context of the error, not the generation of the error itself.
  - merb 4y ago
    Network errors might be retryable/routable differently, but often (especially when starting out) should probably returned to the User. I mean if s3 is down you can retry the call but often it is down then
    - dymk 4y ago
      Sort of. At my company, if we removed retries from our services, our reliability would drop precipitously. Something like 99.99% of retries succeed on the second try, if there's not a hard service outage. If there is a hard outage, well, not much to do about that.
    - preseinger 4y ago
      Only the root of the call stack, fn main, should be able to return anything to the user. Everything else should return errors to their callers through the normal return mechanisms. Anything else, anything that introduces the possibility of shadow control flow, makes it basically impossible to maintain a working mental model of nontrivial programs.
  - 4y ago
- simion314 4y ago
  Don't exception also halt your program if you ignore them? Also if using a library I don't want a bug in it to bring my program down, then I am forced to use workarounds like create a child process to use the library, start the child process from the main process and check on it to see if it fails or succeeds, that would be bad for performance and ugly.
- alerighi 4y ago
  > When invariant violations or mistakes by programmers (aka bugs) are detected, the program should halt as it is in an inconsistent state and continuing could be very dangerous (think privacy/security/data corruption). Otherwise, don't halt (handle it or have the caller handle it). Well it's not always the case. There are situations in which if you detect errors you want the program to continue running, and have only that particular functionality to fail. I tend to write resilient code, since I work in embedded systems and what you never want is the system to crash. Halting a CPU on an invariant violation (i.e. and assert failing) is something useful for debugging (you trigger the debugger and you then analyze why it happened), but something you generally don't want in production. Bette to have a ton of checks more and in case of an invariant violation (that maybe is resulting from a programmer mistake, but there is always the possibility of hardware memory corruption errors) to return an error and handle it in some ways (for example restart the task that returned the error, trying to go back to the last working state).
  - burntsushi 4y ago
    > There are situations in which if you detect errors you want the program to continue running, and have only that particular functionality to fail. Yes, like a web server. If a request handler fails by panicking, in a Rust program, you catch the panic, respond with a 500 error and log the panic somewhere. But you continue serving other requests. I talked about this in the blog post. The problem with your strategy is that it requires you to be aware of your own mistakes. That doesn't sound like a robust strategy, unless you're investing huge resources into sophisticated tooling and have drastically restricted the expressivity of your programming environment. That exists and is fine, and I even addressed that in the blog post too.
    - marshray 4y ago
      Great article BTW, loved it! Will certainly become a classic. The web server example scares me. Something happened during execution that the programmer didn't expect. There's a bug in the program. What if the panic is due to memory corruption (less likely in Rust) or internal data structure corruption? Without knowledge to the contrary, swallowing a panic and YOLO'ing execution is driving full speed down the road of very poorly defined behavior. If the programmer had sufficient knowledge to conclude it was safe, they could have just used a Result<> to report the error. So ... don't make panic part of your API, and don't recover from panics?
- stormbrew 4y ago
  To me the real issue is this is an extremely forced binary and there's really at least three meaningful categories (especially in software with a UI of any sort): - unactionable invariant violation (poisoned mutex, hard memory errors): crash immediately, something that should ever happen happened and there's no way to either handle or present the error to the user in a meaningful way. - unactionable (at the call site) but normal errors (couldn't open a file, disconnected from the remote end of a connection, etc): these need to be propagated up to where they can be turned into actionable information for a user, ideally. This is rarely a thing the call site where it happened can usefully do. - immediately actionable and normal errors (user input didn't validate, file user wanted to open doesn't exist, connection failed but can be retried with a backoff, etc). These need to be handled at the call site or maybe one or two levels up. You need an exception-like mechanism (or at least a process for emulating one, a la go MRV or C errno) to handle the second case, you often want it for the third case, but it never really makes sense to use it for the first. That said, I think in non-test rust code you should use expect instead of unwrap, because sometimes invariants do trip and that little tiny extra bit of info can make a huge difference to resolving it.
cedws 4y ago
The problem is that Rust developers don't just use unwrap() when it should panic. I've seen plenty of "production grade" crates basically just unwrap because the author didn't know how to handle it gracefully or just wanted to get the code compiling, then forgot about it.
- dymk 4y ago
  I'm confused, you seem to be saying opposite things. You're saying authors don't use unwrap when it should panic, and in the next sentence, you say they use unwrap (which causes a panic) when the failure could be gracefully recovered.
  - c7DJTLrn 4y ago
    "don't just"
    - OJFord 4y ago
      Heh, GP (I assume) and I initially parsed it as "don't just-use" (don't simply use) rather than "don't-just use" (don't only use).
    - dymk 4y ago
      Got it. OJFord has it right, my parens were in the wrong place.
- dureuill 4y ago
  not sure why you're getting downvoted. maybe the scope of what you're saying ('plenty of "production grade" crates unwrap [when they shouldn't]') calls for a reference? Or maybe because you're singling out Rust developers while the issue is certainly observed in all languages with similar mechanisms (see unchecked exceptions abuse in java, or aborting asserts abuse in C) Anyway i wouldn't say "plenty", but i did came across crates (parsers :/) that would unwrap on malformed input. the workaround is to encapsulate their use in a catch_unwind. for the record, i had a similar issue in a c++ lib where the author elected to abort on the unsupported input, so i'm somewhat thankful that the idiomatic mechanism is panic (which is recoverable if needs be) in Rust
richardwhiuk 4y ago
I think the `expect()` bad examples are something of a strawman. The `.expect()` for regex for example would say what the regex is matching for. I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`.
- burntsushi 4y ago
  So you're saying that the 'expect()' message when a regex compilation error occurs should be a translation from a terse domain specific language to bloviating prose? :-) What 'expect()' message would you write for this regex? https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005bd707e683b6518a684ab6b74dc4e/ucd-parse/src/unicode_data.rs#L124 https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005... I think 'unwrap()' there is perfectly appropriate. > I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`. Why?
  - richardwhiuk 4y ago
    .expect("UnicodeData::from_str regex failed to compile") With this, even with out backtrace, you can work out what happened. Without it, you just know that some regex somewhere is invalid.
    - burntsushi 4y ago
      Have you ever seen a Regex::new(..).unwrap() fail? It sounds like maybe not. It also sounds like you haven't seen an 'unwrap()' fail either. > Without it, you just know that some regex somewhere is invalid. That's bologna. As I discuss in the blog post, 'unwrap()' tells you the line number at which it panicked. There's even an example showing exactly this. There's even another example showing what happens when you call 'Regex::new(..).unwrap()' and it fails[1]: fn main() { regex::Regex::new(r"foo\p{glyph}bar").unwrap(); } And running it: $ cargo run Finished dev [unoptimized + debuginfo] target(s) in 0.00s Running `target/debug/rust-panic` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax( ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ regex parse error: foo\p{glyph}bar ^^^^^^^^^ error: Unicode property not found ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ )', main.rs:4:36 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrac So you get the line number (as is standard with any 'unwrap()') and you get the actual error message from the regex. No need to even enable the backtrace. You just don't need 'expect()' here. [1]: https://blog.burntsushi.net/unwrap/#why-not-use-expect-instead-of-unwrap https://blog.burntsushi.net/unwrap/#why-not-use-expect-inste...
berryton 4y ago
The Rust book has a small discussion about his in chapter 9.3 (https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-pa...). I think it is good to have some references (the blog post and the book) for when the horde comes after you.
koala_man 4y ago
tl;dr: Author convincingly argues that Rust `unwrap()` (Java `Optional.get`, Haskell `fromJust`) is fine when you either have checked that the call will not fail, or when you're in a unit test or similar where a panic is a helpful result. (And, conversely, that it's not fine to use it to avoid doing real error handling)
- OJFord 4y ago
  Buried in your 'or similar' is the more controversial and helpful (IMO) use - the infamous 'this should never happen' exceptions.
  - marcosdumay 4y ago
    Well, if you want a definitive answer, it's "it depends". Those errors are not all alike.
  - burntsushi 4y ago
    No, that isn't buried in the "or similar" of the GP. They mention that explicitly with "checked that the call will not fail." The "or similar" refers to documentation examples and prototyping/one-off scripts.
- drexlspivey 4y ago
  Is this supposed to be controversial? Even rust docs say so https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html#cases-in-which-you-have-more-information-than-the-compiler https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-pa...
  - burntsushi 4y ago
    Nope. But people are thoroughly confused by it. Comes up all of the time. And a lot of people have more extremist positions. Banning unwrap. Or banning any panicking branches at all. That's why my blog post covers an example where we convert a function that never panics (sensible) to a function whose signature says it might return an error, but it actually will never return an error (bonkers). People advocate for this. After publishing this article, it almost seems like people are more confused than I thought. Idk. Anyway, no, this blog is not meant to be controversial. It is meant to untangle knots.
ComputerGuru 4y ago
One runtime panic I wish was a compile-time error in the rust standard library is the use of an incorrect memory order, eg Ordering::Release with AtomicBool::load(). It would have been fairly trivial to set up generic constraints specifying if a read, write, or read-write ordering semantic is expected and to fail to compile if it wasn’t met.
- nynx 4y ago
  This is going to happen at some point now that const generics arrived.
  - ComputerGuru 4y ago
    Likely only once we finally get const parameters specified as regular arguments. It doesn’t need const at all, though. You just need three traits and either first class enum variants as types or a pseudo enum (mod/struct Ordering with ZST structs Acquire, Release, etc)
- pitaj 4y ago
  This could be a good application of a Clippy lint.
- oconnor663 4y ago
  I think the reason it's not a compile-time error is that it's actually possible to select your ordering at runtime, like this: use std::sync::atomic::*; let x = AtomicU64::new(0); let ordering = if rand::random() { Ordering::Relaxed } else { Ordering::SeqCst }; x.fetch_add(1, ordering);
  - ComputerGuru 4y ago
    Yes, I tried (a few months ago) mocking a PR for this that refactored the enum variants into ZST structs and that was the only sticking point for backwards compatibility.
ArrayBoundCheck 4y ago
Does this mean using C inside of rust is ok? I'm pretty sure the original team kept hitting memory problems and it was in fact not ok. Browsers crash often enough that I don't want unwrap making it worse
- TheDong 4y ago
  > Does this mean using C inside of rust is ok? I'm pretty sure the original team kept hitting memory problems and it was in fact not ok This reads a little like flamebait. Memory unsafety is bad. panic is _not_ a memory unsafe operation, it does not result in memory problems. This post is not related to memory safety in any way.
  - ArrayBoundCheck 4y ago
    The third sentence was the point. I don't want browsers being less reliable and terminating randomly due to unwraps
    - burntsushi 4y ago
      And how do you propose they do that? What you're saying is tautological. It's like saying, "I don't want browsers to use runtime invariants because they can be broken and thus causing my browser to terminate." Like... yeah, cool. How do you do that exactly? Also, please do consider reading the blog post I wrote. I wrote it to try to clarify a lot of confusing around this topic. It is nuanced, and it just can't be untangled in a few sentences in a HN comment.
    - 0x457 4y ago
      I think you missed the point of the article. In fact, the article specifically covers catching panics, so it doesn't bring down the entire application. As well as covering when panic is okay. It doesn't say you should just unwrap everything and treat panic as error handling.
      - ArrayBoundCheck 4y ago
ww520 4y ago
I generally just use expect(), assert!, or unreachable! rather than simply unwrap() to document the unexpected invalid state.
renewiltord 4y ago
Very neat. Just discovered anyhow and the context stuff from this post.
nnoitra 4y ago
This blog post will age out in 6 months.
- burntsushi 4y ago
  Why? It's basically an elaboration of the same advice I gave eight years ago. And I was not the first.
sitkack 4y ago
One can set the env var themselves in main(), many errors are transient, best to capture it the first time. // use std::env; env::set_var("RUST_BACKTRACE", "full");
- burntsushi 4y ago
  Hah. Ironically, set_var might be deprecated at some point and replaced with an unsafe alternative. (Long story. Short story is that it's currently unsound. If you have C code trying to read from the environment at the same time you end up with UB. If everything is Rust code though, then I believe you're fine.)
  - sitkack 4y ago
    I was trying to find a "turn on backtrace" as an official api but couldn't find one in 90s of looking. I see the backtrace crate for catching them in user space in process, which is nice. What would you recommend? Forking and execing yourself to set the env var? backtrace::enable_full() or something to that effect would be nice.
    - burntsushi 4y ago
      fork-exec is probably the most robust way. But having some std API to enable backtraces seems reasonable too. Someone just needs to put in the design work and champion it. std::backtrace will be stabilized soon. I wonder how far you could get with a custom panic hook that unconditionally captured and printed a backtrace? https://doc.rust-lang.org/std/panic/fn.set_hook.html https://doc.rust-lang.org/std/panic/fn.set_hook.html
jeffrallen 4y ago
When you've gotten to the point of saying, "well, it's ok to panic in example code" you've already lost the game. Novice programmers learn from example code, and novice programmers are an order of magnitude more common than experienced ones. A programming ecosystem that depends on 9 out of every 10 people being able to intuit that which the other 1 understands is not an ecosystem that's going to produce good code. Rust is a minefield of bear traps laid by experts, and I fear for the future of our industry if Go and Java programmers are required by some quirk of network effects or first mover advantage or whatever to starting programming (badly) in Rust.
- burntsushi 4y ago
  Did you read the blog? I addressed all of that.
schubart 4y ago
> When checking preconditions, make sure the panic message relates to the documented precondition, perhaps by adding a custom message. For example, > `assert!(!xs.is_empty(), "expected parameter 'xs' to be non-empty")`. This panics with > thread 'main' panicked at 'expected parameter 'xs' to be non-empty', src/main.rs:79:5 Without the custom message it's > thread 'main' panicked at 'assertion failed: !xs.is_empty()', src/main.rs:79:5 Given that panics should be for bugs, i.e. interpreted by developers, I'd say the second message is clear enough and a custom message just adds noise in the source code.
- burntsushi 4y ago
  I don't disagree. Definitely depends on how opaque the assertion test is. In this case, yeah, probably don't need a message. Pithy illustrative examples are hard.
cmrdporcupine 4y ago
The article has nuance. It's good. But the language has made it too easy to unwrap/expect and panic. This escape hatch should exist, for sure. But it ought to be more explicit, and more of a pain. It is far too easy to reach for this tool.
- burntsushi 4y ago
  Your focus on unwrap/expect seems arbitrary. Why aren't you commenting on the ease with which 'slice[i]' and 'x * y' fail? Or alloc failure? Is slice index syntax not also too easy to reach for? I ask because I suspect you've got a bit of motte and bailey going on here. The motte is "hey let's make unwrap/expect more verbose because we want people to be REALLY sure," but the bailey is "let's actually make everything that can panic a lot more verbose and totally change the character of the language and make it a lot less practical." I'd encourage you to read the "lint" section near the end: https://blog.burntsushi.net/unwrap/#should-we-lint-against-uses-of-unwrap https://blog.burntsushi.net/unwrap/#should-we-lint-against-u...
  - lmm 4y ago
    > Is slice index syntax not also too easy to reach for? It absolutely is. Modern language design should be discouraging getting an element by index; there are usually better alternatives e.g. iterating through a datastructure, or using combinators like zip to build the datastructure/view you need.
    - WesolyKubeczek 4y ago
      Because who needs performance? Phew
      - simon_o 4y ago
        Why would it be slower? Indexing may incur range checks, but iterators/combinators are pretty much destined to have them elided.
dont_panic_ 4y ago
my problem with panic is that it is like walking in a mine field, you don't know which function will blow up at any given time, and it's not checked by the compiler. If there was some sort of signal to mark a function as panicking (& vice versa), that would be nice.
- burntsushi 4y ago
  This is like saying, "my problem with bugs is that they're like walking in a mine field, you don't know which function might have a bug in it or not."
  - pornel 4y ago
    Not really. It's possible to verify that a call graph can't call panic anywhere, and there exist 3rd party hacky solutions for this already. Rust is just lacking first-class features for this.
    - burntsushi 4y ago
      Sure... And no-panic is cool, yes. But I stand by what I said. My perspective here is like this. "Okay, so you're complaining about panics, what's your alternative?" No, really, like what do you instead of panicking? That's what my blog explores. So what I'm saying is, if you're going to complain about all the various panic branches, and assuming those panic branches are legitimate (i.e., hitting them would be a bug), then to me, that's just like complaining about bugs in general. At least with panics, they're a lot more visible. Even something like 'untrusted'[1] (used in crypto libs like 'ring') uses 'unwrap()' in its implementation. What else are you going to do to remove that panicking branch? Complaining about its existence, to me, is basically like complaining that bugs exist and that they're hard to find. (Except panics are better, because panicking branches are much easier to find than arbitrary bugs.) [1]: https://github.com/briansmith/untrusted https://github.com/briansmith/untrusted
      - pornel 4y ago
        Assurance there are can be no panics has practical benefits, beyond the "but what about bugs?" question. Exception safety is a complication for unsafe code, and has been a source unsoundness in Rust (to the point there's an RFC proposing to remove ability to unwind from Drop entirely). It has performance costs (code bloat, inhibits code motion and autovectorization). No-panic as an assertion has a value of ensuring that your expectations match. You can ensure that an infallible function really is infallible. You can ensure that a function that returns Result fails only this way. You can ensure your code running in non-Rust stack frames doesn't need a double catch_unwind sandwich. Even when the code correctly uses panics for what it considers bugs, maybe the code's contract needs to be changed. For example you can have a function that requires a convex polygon as an input. If you pass it a non-convex polygon, it's clearly your bug. But if you get points from an untrusted source, adding an `is_convex` check may be insufficient, because due to rounding errors your check and function's check may disagree and you get a DoS vector, and you pay cost of checking twice. If you need it in real-time graphics, a non-panicking garbage-in garbage-out approach may be better.
yuan43 4y ago
The author notes that API simplicity might be a reason to avoid pushing invariants to compile time: > What do I mean by “API simplicity?” Well, this panic could be removed by moving this runtime invariant to a compile time invariant. Namely, the API could provide, for example, an AhoCorasickOverlapping type, and the overlapping search routines would be defined only on that type and not on AhoCorasick. Therefore, users of the crate could never call an overlapping search routine on an improperly configured automaton. The compiler simply wouldn’t allow it. > But this adds a lot of additional surface area to the API. And it does it in really pernicious ways. For example, an AhoCorasickOverlapping type would still want to have normal non-overlapping search routines, just like AhoCorasick does. It’s now reasonable to want to be able to write routines that accept any kind of Aho-Corasick automaton and run a non-overlapping search. In that case, either the aho-corasick crate or the programmer using the crate needs to define some kind of generic abstraction to enable that. Or, more likely, perhaps copy some code. > I thus made a judgment that having one type that can do everything—but might fail loudly for certain methods under certain configurations—would be best. The API design of aho-corasick isn’t going to result in subtle logic errors that silently produce incorrect results. If a mistake is made, then the caller is still going to get a panic with a clear message. At that point, the fix will be easy. What I gather from this is that the author chose to define a type (call it A) with an attribute that when set in a certain way will cause certain functions to panic. This was preferred to the alternative (two types, A and B) with functions specific to each and where panic was not possible. This kind of design decision comes up a lot, so understanding the reasoning here could be helpful in a lot of situations. Unfortunately, the passage is less than clear due to lack of source code inline and the highly-specific nature of the problem. An example with source code using more accessible algorithms might be an improvement here. That said, I'm skeptical that the full range of approach was considered. I sometimes find that the presence of unwrap is a smell pointing to types that have not been fully fleshed out. As an extreme case, consider a struct whose fields contained diverse data (numbers, colors, enumerated values), but which are all defined as strings. It will be very easy to put this struct into an inconsistent runtime state because nothing can be checked at compile time. The type itself is anemic. Replacing strings with more constrained types eliminates opportunities for panic - possibly all of them. I get that the whole point is "at what cost?" All I'm saying is that the tradeoffs aren't clear from the example in the passage.

212 points by cp9 4y ago

I totally agree with de-emphasizing the old "recoverable" vs. "unrecoverable" dichotomy (https://blog.burntsushi.net/unwrap/#what-about-recoverable-vs-unrecoverable-errors https://blog.burntsushi.net/unwrap/#what-about-recoverable-v...). Every time I've heard programmers (especially in the context of exceptions) try to define it, I've found it imprecise and open to debate. When invariant violations or mistakes by programmers (aka bugs) are detected, the program should halt as it is in an inconsistent state and continuing could be very dangerous (think privacy/security/data corruption). Otherwise, don't halt (handle it or have the caller handle it).

arcticbull 4y ago
Yep, there's not really any such thing (IME) as a 'recoverable' error - except with respect to I/O. There's either I/O errors - or there's logic errors. A failure with logic should nuke due to the app being in an inconsistent state; trust is lost. An I/O error should fail softly.
- ithkuil 4y ago
  A parser library can return an error when the input is wrong. It doesn't necessarily mean the app is in an inconsistent state when that happens; it all depends on what the application does. It follows that aborting is often a sensible decision in application code and rarely in library code.
  - arcticbull 4y ago
    I would personally consider parsers as part of I/O, but point taken.
- fatherzine 4y ago
  Partially true. In practice people implement multiplexed servers, for many reasons, including performance / throughput. A logic failure should nuke _the offending request_, not the entire server with all the unrelated concurrent requests.
  - AshamedCaptain 4y ago
    This is an attitude that I often see -- library authors who believe they own the process. Aborting may be the only sensible thing to do in runtimes where you can end up corrupting the entire process memory or the like, making recovery "dubiously possible", but absolutely not for anything higher level, where recovery may be safe and possible.
    - burntsushi 4y ago
      I gave examples covering this area in the post. How would you rewrite the code in this section[1], for example, to conform to your views? (Assume this code is in a library.) [1]: https://blog.burntsushi.net/unwrap/#what-about-when-invariants-cant-be-moved-to-compile-time https://blog.burntsushi.net/unwrap/#what-about-when-invarian...
      - fatherzine 4y ago
        * Short term: add explicit runtime checks for every step that may panic. With the way jump prediction works in modern processors, the runtime cost may be smaller than one may naively assume it is. Some of us would take, say, a 10% perf. degradation instead of chasing prod panics in the middle of the night. * Long term: plug-in a richer type (logic) system, so one can safely prove the most costly runtime invariants at compile time.
        burntsushi 4y ago
        > add explicit runtime checks for every step that may panic And do... what when the check fails? Can you please write the code for it? Because I don't understand what the heck you mean here. If you don't know Rust, pseudo code is fine. > Long term: plug-in a richer type (logic) system, so one can safely prove the most costly runtime invariants at compile time. This is just copying what I already said in the blog post. Until someone can show me how to prove the correctness of arbitrary DFA construction and search from a user provided regular expression pattern and demonstrate its use in a practical programming language like Rust, I consider this a total non-answer, not a "long term" answer. Basically, your answer here confirms for me that your views on how code should be structured are incoherent.
- jcelerier 4y ago
  > There's either I/O errors - or there's logic errors. A failure with logic should nuke due to the app being in an inconsistent state; trust is lost. nah. in GUI apps for instance you want the failure in the logic of a sub-sub-function to just tell the error "wops" when the button that triggered the action was clicked, not nuke the app (unless you hate your users). e.g. imagine a 3D software which allows to do mesh operations - user clicks on the "Smooth the mesh" button somewhere. Programmer forgot to handle a division by zero in some degenerate case of the smoothing computation which ends up leading to an exception: a value becomes zero, someone used unsigned integers for n in an "n - 1" computation which ends up in a call to array_of_floats.resize(0xffffffffffffffff) (and a likely std::bad_alloc being thrown if you're in c++). The original mesh is unchanged as the operation waits until the computation is complete to replace the old mesh with the new. If you ever decide to crash in this situation I am sure you will have great reviews on 3D modeling software comparisons.
  - arcticbull 4y ago
    I would actually want that to crash, yes. That would ensure it gets caught and resolved by developers during development. Or at least increase the odds thereof. Further, if it allocates a few gb, or if it's allocating a large amount of memory because the size parameter got smashed? If it crashes enough that you'd get terrible reviews it would definitely be caught during development. If it messes up your mesh silently then you'll definitely still get bad reviews. And actually having an std::bad_alloc thrown is even worse since you have no idea what state it left things after running some destructors when you weren't planning for it.
    - jcelerier 4y ago
      > That would ensure it gets caught and resolved by developers during development i don't know in which reality you live but in mine there's not much relationship between the existence of crashes, and them being resolved during development > And actually having an std::bad_alloc thrown is even worse since you have no idea what state it left things after running some destructors when you weren't planning for it. i don't even know what to say. that's the whole point of destructors - you know that things will be unwound in the reverse order from which they were created on automatic storage. that's, like, why c++ exists
hinkley 4y ago
Recoverable vs unrecoverable comes down to requirements. Certain companies known for having software that 'just works' tend to have both very few unrecoverable errors and very conservative feature sets to help facilitate that short list. It is very clearly a choice, even if many people are deciding by default. By not tackling an issue, you've chosen to have that issue.
- dcsommer 4y ago
  I think we have compatible views. Each layer of the software must decide it's requirements and handle errors appropriately per requirements. You're right I didn't articulate when to handle an issue locally vs. pass it up. I think that's where requirements (and also explicit API guarantees) come into play. I do think that APIs that "overpromise" by not returning the errors they do not handle to the caller, and instead halt or throw an exception, do their users a disservice in the long-run. These just become undocumented cases that bite you later on. Better libraries have all these conditions baked into the API itself.
  - tsimionescu 4y ago
    But an exception is a part of the API, why are you putting it at the same level as halting?
    - dcsommer 4y ago
      In my experience, APIs that throw rarely define all the exceptions that can come from it, especially transitively. I see exceptions as a failed (because undocumented, but still important for correctness) attempt at compromising between halting and returning an error.
      - hinkley 4y ago
        I wonder if there's a moral equivalent of borrow semantics where we more formally define error propagation.
jcranmer 4y ago
The criteria I tend to prefer is "expected" versus "unexpected" errors. I/O errors, especially network errors, are things that are going to be expected under reasonable operation, therefore it make sense that code should handle them. Similarly, user input resulting in incorrectly formatted code should be reasonably expected and therefore handled. But the same kinds of failures might not be reasonably expected in other circumstances--I wouldn't expect that the internal configuration files of an application should occur in reasonable operation, and therefore it makes sense to panic if they're corrupted... even if the cause is an I/O operation on a local disk, or parsing some JSON or TOML or INI or whatnot file. One implication of this is that it needs to be easy for any error system to promote an "expected error" into an "unexpected error"--which is what unwrap/expect does. The recoverable/unrecoverable error suggests that there ought to be no reason to do this, but there is absolutely a reason to do so: what category an error falls into is ultimately decided by the context of the error, not the generation of the error itself.
- merb 4y ago
  Network errors might be retryable/routable differently, but often (especially when starting out) should probably returned to the User. I mean if s3 is down you can retry the call but often it is down then
  - dymk 4y ago
    Sort of. At my company, if we removed retries from our services, our reliability would drop precipitously. Something like 99.99% of retries succeed on the second try, if there's not a hard service outage. If there is a hard outage, well, not much to do about that.
  - preseinger 4y ago
    Only the root of the call stack, fn main, should be able to return anything to the user. Everything else should return errors to their callers through the normal return mechanisms. Anything else, anything that introduces the possibility of shadow control flow, makes it basically impossible to maintain a working mental model of nontrivial programs.
- 4y ago
simion314 4y ago
Don't exception also halt your program if you ignore them? Also if using a library I don't want a bug in it to bring my program down, then I am forced to use workarounds like create a child process to use the library, start the child process from the main process and check on it to see if it fails or succeeds, that would be bad for performance and ugly.
alerighi 4y ago
> When invariant violations or mistakes by programmers (aka bugs) are detected, the program should halt as it is in an inconsistent state and continuing could be very dangerous (think privacy/security/data corruption). Otherwise, don't halt (handle it or have the caller handle it). Well it's not always the case. There are situations in which if you detect errors you want the program to continue running, and have only that particular functionality to fail. I tend to write resilient code, since I work in embedded systems and what you never want is the system to crash. Halting a CPU on an invariant violation (i.e. and assert failing) is something useful for debugging (you trigger the debugger and you then analyze why it happened), but something you generally don't want in production. Bette to have a ton of checks more and in case of an invariant violation (that maybe is resulting from a programmer mistake, but there is always the possibility of hardware memory corruption errors) to return an error and handle it in some ways (for example restart the task that returned the error, trying to go back to the last working state).
- burntsushi 4y ago
  > There are situations in which if you detect errors you want the program to continue running, and have only that particular functionality to fail. Yes, like a web server. If a request handler fails by panicking, in a Rust program, you catch the panic, respond with a 500 error and log the panic somewhere. But you continue serving other requests. I talked about this in the blog post. The problem with your strategy is that it requires you to be aware of your own mistakes. That doesn't sound like a robust strategy, unless you're investing huge resources into sophisticated tooling and have drastically restricted the expressivity of your programming environment. That exists and is fine, and I even addressed that in the blog post too.
  - marshray 4y ago
    Great article BTW, loved it! Will certainly become a classic. The web server example scares me. Something happened during execution that the programmer didn't expect. There's a bug in the program. What if the panic is due to memory corruption (less likely in Rust) or internal data structure corruption? Without knowledge to the contrary, swallowing a panic and YOLO'ing execution is driving full speed down the road of very poorly defined behavior. If the programmer had sufficient knowledge to conclude it was safe, they could have just used a Result<> to report the error. So ... don't make panic part of your API, and don't recover from panics?
stormbrew 4y ago
To me the real issue is this is an extremely forced binary and there's really at least three meaningful categories (especially in software with a UI of any sort): - unactionable invariant violation (poisoned mutex, hard memory errors): crash immediately, something that should ever happen happened and there's no way to either handle or present the error to the user in a meaningful way. - unactionable (at the call site) but normal errors (couldn't open a file, disconnected from the remote end of a connection, etc): these need to be propagated up to where they can be turned into actionable information for a user, ideally. This is rarely a thing the call site where it happened can usefully do. - immediately actionable and normal errors (user input didn't validate, file user wanted to open doesn't exist, connection failed but can be retried with a backoff, etc). These need to be handled at the call site or maybe one or two levels up. You need an exception-like mechanism (or at least a process for emulating one, a la go MRV or C errno) to handle the second case, you often want it for the third case, but it never really makes sense to use it for the first. That said, I think in non-test rust code you should use expect instead of unwrap, because sometimes invariants do trip and that little tiny extra bit of info can make a huge difference to resolving it.

cedws 4y ago

The problem is that Rust developers don't just use unwrap() when it should panic. I've seen plenty of "production grade" crates basically just unwrap because the author didn't know how to handle it gracefully or just wanted to get the code compiling, then forgot about it.

dymk 4y ago
I'm confused, you seem to be saying opposite things. You're saying authors don't use unwrap when it should panic, and in the next sentence, you say they use unwrap (which causes a panic) when the failure could be gracefully recovered.
- c7DJTLrn 4y ago
  "don't just"
  - OJFord 4y ago
    Heh, GP (I assume) and I initially parsed it as "don't just-use" (don't simply use) rather than "don't-just use" (don't only use).
  - dymk 4y ago
    Got it. OJFord has it right, my parens were in the wrong place.
dureuill 4y ago
not sure why you're getting downvoted. maybe the scope of what you're saying ('plenty of "production grade" crates unwrap [when they shouldn't]') calls for a reference? Or maybe because you're singling out Rust developers while the issue is certainly observed in all languages with similar mechanisms (see unchecked exceptions abuse in java, or aborting asserts abuse in C) Anyway i wouldn't say "plenty", but i did came across crates (parsers :/) that would unwrap on malformed input. the workaround is to encapsulate their use in a catch_unwind. for the record, i had a similar issue in a c++ lib where the author elected to abort on the unsupported input, so i'm somewhat thankful that the idiomatic mechanism is panic (which is recoverable if needs be) in Rust

richardwhiuk 4y ago

I think the `expect()` bad examples are something of a strawman. The `.expect()` for regex for example would say what the regex is matching for. I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`.

burntsushi 4y ago
So you're saying that the 'expect()' message when a regex compilation error occurs should be a translation from a terse domain specific language to bloviating prose? :-) What 'expect()' message would you write for this regex? https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005bd707e683b6518a684ab6b74dc4e/ucd-parse/src/unicode_data.rs#L124 https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005... I think 'unwrap()' there is perfectly appropriate. > I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`. Why?
- richardwhiuk 4y ago
  .expect("UnicodeData::from_str regex failed to compile") With this, even with out backtrace, you can work out what happened. Without it, you just know that some regex somewhere is invalid.
  - burntsushi 4y ago
    Have you ever seen a Regex::new(..).unwrap() fail? It sounds like maybe not. It also sounds like you haven't seen an 'unwrap()' fail either. > Without it, you just know that some regex somewhere is invalid. That's bologna. As I discuss in the blog post, 'unwrap()' tells you the line number at which it panicked. There's even an example showing exactly this. There's even another example showing what happens when you call 'Regex::new(..).unwrap()' and it fails[1]: fn main() { regex::Regex::new(r"foo\p{glyph}bar").unwrap(); } And running it: $ cargo run Finished dev [unoptimized + debuginfo] target(s) in 0.00s Running `target/debug/rust-panic` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax( ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ regex parse error: foo\p{glyph}bar ^^^^^^^^^ error: Unicode property not found ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ )', main.rs:4:36 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrac So you get the line number (as is standard with any 'unwrap()') and you get the actual error message from the regex. No need to even enable the backtrace. You just don't need 'expect()' here. [1]: https://blog.burntsushi.net/unwrap/#why-not-use-expect-instead-of-unwrap https://blog.burntsushi.net/unwrap/#why-not-use-expect-inste...

berryton 4y ago

The Rust book has a small discussion about his in chapter 9.3 (https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-pa...). I think it is good to have some references (the blog post and the book) for when the horde comes after you.

koala_man 4y ago

tl;dr: Author convincingly argues that Rust `unwrap()` (Java `Optional.get`, Haskell `fromJust`) is fine when you either have checked that the call will not fail, or when you're in a unit test or similar where a panic is a helpful result. (And, conversely, that it's not fine to use it to avoid doing real error handling)

OJFord 4y ago
Buried in your 'or similar' is the more controversial and helpful (IMO) use - the infamous 'this should never happen' exceptions.
- marcosdumay 4y ago
  Well, if you want a definitive answer, it's "it depends". Those errors are not all alike.
- burntsushi 4y ago
  No, that isn't buried in the "or similar" of the GP. They mention that explicitly with "checked that the call will not fail." The "or similar" refers to documentation examples and prototyping/one-off scripts.
drexlspivey 4y ago
Is this supposed to be controversial? Even rust docs say so https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html#cases-in-which-you-have-more-information-than-the-compiler https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-pa...
- burntsushi 4y ago
  Nope. But people are thoroughly confused by it. Comes up all of the time. And a lot of people have more extremist positions. Banning unwrap. Or banning any panicking branches at all. That's why my blog post covers an example where we convert a function that never panics (sensible) to a function whose signature says it might return an error, but it actually will never return an error (bonkers). People advocate for this. After publishing this article, it almost seems like people are more confused than I thought. Idk. Anyway, no, this blog is not meant to be controversial. It is meant to untangle knots.

ComputerGuru 4y ago

One runtime panic I wish was a compile-time error in the rust standard library is the use of an incorrect memory order, eg Ordering::Release with AtomicBool::load(). It would have been fairly trivial to set up generic constraints specifying if a read, write, or read-write ordering semantic is expected and to fail to compile if it wasn’t met.

nynx 4y ago
This is going to happen at some point now that const generics arrived.
- ComputerGuru 4y ago
  Likely only once we finally get const parameters specified as regular arguments. It doesn’t need const at all, though. You just need three traits and either first class enum variants as types or a pseudo enum (mod/struct Ordering with ZST structs Acquire, Release, etc)
pitaj 4y ago
This could be a good application of a Clippy lint.
oconnor663 4y ago
I think the reason it's not a compile-time error is that it's actually possible to select your ordering at runtime, like this: use std::sync::atomic::*; let x = AtomicU64::new(0); let ordering = if rand::random() { Ordering::Relaxed } else { Ordering::SeqCst }; x.fetch_add(1, ordering);
- ComputerGuru 4y ago
  Yes, I tried (a few months ago) mocking a PR for this that refactored the enum variants into ZST structs and that was the only sticking point for backwards compatibility.

ArrayBoundCheck 4y ago

Does this mean using C inside of rust is ok? I'm pretty sure the original team kept hitting memory problems and it was in fact not ok. Browsers crash often enough that I don't want unwrap making it worse

TheDong 4y ago
> Does this mean using C inside of rust is ok? I'm pretty sure the original team kept hitting memory problems and it was in fact not ok This reads a little like flamebait. Memory unsafety is bad. panic is _not_ a memory unsafe operation, it does not result in memory problems. This post is not related to memory safety in any way.
- ArrayBoundCheck 4y ago
  The third sentence was the point. I don't want browsers being less reliable and terminating randomly due to unwraps
  - burntsushi 4y ago
    And how do you propose they do that? What you're saying is tautological. It's like saying, "I don't want browsers to use runtime invariants because they can be broken and thus causing my browser to terminate." Like... yeah, cool. How do you do that exactly? Also, please do consider reading the blog post I wrote. I wrote it to try to clarify a lot of confusing around this topic. It is nuanced, and it just can't be untangled in a few sentences in a HN comment.
  - 0x457 4y ago
    I think you missed the point of the article. In fact, the article specifically covers catching panics, so it doesn't bring down the entire application. As well as covering when panic is okay. It doesn't say you should just unwrap everything and treat panic as error handling.
    - ArrayBoundCheck 4y ago

ww520 4y ago

I generally just use expect(), assert!, or unreachable! rather than simply unwrap() to document the unexpected invalid state.

renewiltord 4y ago

Very neat. Just discovered anyhow and the context stuff from this post.

nnoitra 4y ago

This blog post will age out in 6 months.

burntsushi 4y ago
Why? It's basically an elaboration of the same advice I gave eight years ago. And I was not the first.

sitkack 4y ago

One can set the env var themselves in main(), many errors are transient, best to capture it the first time. // use std::env; env::set_var("RUST_BACKTRACE", "full");

burntsushi 4y ago
Hah. Ironically, set_var might be deprecated at some point and replaced with an unsafe alternative. (Long story. Short story is that it's currently unsound. If you have C code trying to read from the environment at the same time you end up with UB. If everything is Rust code though, then I believe you're fine.)
- sitkack 4y ago
  I was trying to find a "turn on backtrace" as an official api but couldn't find one in 90s of looking. I see the backtrace crate for catching them in user space in process, which is nice. What would you recommend? Forking and execing yourself to set the env var? backtrace::enable_full() or something to that effect would be nice.
  - burntsushi 4y ago
    fork-exec is probably the most robust way. But having some std API to enable backtraces seems reasonable too. Someone just needs to put in the design work and champion it. std::backtrace will be stabilized soon. I wonder how far you could get with a custom panic hook that unconditionally captured and printed a backtrace? https://doc.rust-lang.org/std/panic/fn.set_hook.html https://doc.rust-lang.org/std/panic/fn.set_hook.html

jeffrallen 4y ago

When you've gotten to the point of saying, "well, it's ok to panic in example code" you've already lost the game. Novice programmers learn from example code, and novice programmers are an order of magnitude more common than experienced ones. A programming ecosystem that depends on 9 out of every 10 people being able to intuit that which the other 1 understands is not an ecosystem that's going to produce good code. Rust is a minefield of bear traps laid by experts, and I fear for the future of our industry if Go and Java programmers are required by some quirk of network effects or first mover advantage or whatever to starting programming (badly) in Rust.

burntsushi 4y ago
Did you read the blog? I addressed all of that.

schubart 4y ago

> When checking preconditions, make sure the panic message relates to the documented precondition, perhaps by adding a custom message. For example, > `assert!(!xs.is_empty(), "expected parameter 'xs' to be non-empty")`. This panics with > thread 'main' panicked at 'expected parameter 'xs' to be non-empty', src/main.rs:79:5 Without the custom message it's > thread 'main' panicked at 'assertion failed: !xs.is_empty()', src/main.rs:79:5 Given that panics should be for bugs, i.e. interpreted by developers, I'd say the second message is clear enough and a custom message just adds noise in the source code.

burntsushi 4y ago
I don't disagree. Definitely depends on how opaque the assertion test is. In this case, yeah, probably don't need a message. Pithy illustrative examples are hard.

cmrdporcupine 4y ago

The article has nuance. It's good. But the language has made it too easy to unwrap/expect and panic. This escape hatch should exist, for sure. But it ought to be more explicit, and more of a pain. It is far too easy to reach for this tool.

burntsushi 4y ago
Your focus on unwrap/expect seems arbitrary. Why aren't you commenting on the ease with which 'slice[i]' and 'x * y' fail? Or alloc failure? Is slice index syntax not also too easy to reach for? I ask because I suspect you've got a bit of motte and bailey going on here. The motte is "hey let's make unwrap/expect more verbose because we want people to be REALLY sure," but the bailey is "let's actually make everything that can panic a lot more verbose and totally change the character of the language and make it a lot less practical." I'd encourage you to read the "lint" section near the end: https://blog.burntsushi.net/unwrap/#should-we-lint-against-uses-of-unwrap https://blog.burntsushi.net/unwrap/#should-we-lint-against-u...
- lmm 4y ago
  > Is slice index syntax not also too easy to reach for? It absolutely is. Modern language design should be discouraging getting an element by index; there are usually better alternatives e.g. iterating through a datastructure, or using combinators like zip to build the datastructure/view you need.
  - WesolyKubeczek 4y ago
    Because who needs performance? Phew
    - simon_o 4y ago
      Why would it be slower? Indexing may incur range checks, but iterators/combinators are pretty much destined to have them elided.

dont_panic_ 4y ago

my problem with panic is that it is like walking in a mine field, you don't know which function will blow up at any given time, and it's not checked by the compiler. If there was some sort of signal to mark a function as panicking (& vice versa), that would be nice.

burntsushi 4y ago
This is like saying, "my problem with bugs is that they're like walking in a mine field, you don't know which function might have a bug in it or not."
- pornel 4y ago
  Not really. It's possible to verify that a call graph can't call panic anywhere, and there exist 3rd party hacky solutions for this already. Rust is just lacking first-class features for this.
  - burntsushi 4y ago
    Sure... And no-panic is cool, yes. But I stand by what I said. My perspective here is like this. "Okay, so you're complaining about panics, what's your alternative?" No, really, like what do you instead of panicking? That's what my blog explores. So what I'm saying is, if you're going to complain about all the various panic branches, and assuming those panic branches are legitimate (i.e., hitting them would be a bug), then to me, that's just like complaining about bugs in general. At least with panics, they're a lot more visible. Even something like 'untrusted'[1] (used in crypto libs like 'ring') uses 'unwrap()' in its implementation. What else are you going to do to remove that panicking branch? Complaining about its existence, to me, is basically like complaining that bugs exist and that they're hard to find. (Except panics are better, because panicking branches are much easier to find than arbitrary bugs.) [1]: https://github.com/briansmith/untrusted https://github.com/briansmith/untrusted
    - pornel 4y ago
      Assurance there are can be no panics has practical benefits, beyond the "but what about bugs?" question. Exception safety is a complication for unsafe code, and has been a source unsoundness in Rust (to the point there's an RFC proposing to remove ability to unwind from Drop entirely). It has performance costs (code bloat, inhibits code motion and autovectorization). No-panic as an assertion has a value of ensuring that your expectations match. You can ensure that an infallible function really is infallible. You can ensure that a function that returns Result fails only this way. You can ensure your code running in non-Rust stack frames doesn't need a double catch_unwind sandwich. Even when the code correctly uses panics for what it considers bugs, maybe the code's contract needs to be changed. For example you can have a function that requires a convex polygon as an input. If you pass it a non-convex polygon, it's clearly your bug. But if you get points from an untrusted source, adding an `is_convex` check may be insufficient, because due to rounding errors your check and function's check may disagree and you get a DoS vector, and you pay cost of checking twice. If you need it in real-time graphics, a non-panicking garbage-in garbage-out approach may be better.

yuan43 4y ago

The author notes that API simplicity might be a reason to avoid pushing invariants to compile time: > What do I mean by “API simplicity?” Well, this panic could be removed by moving this runtime invariant to a compile time invariant. Namely, the API could provide, for example, an AhoCorasickOverlapping type, and the overlapping search routines would be defined only on that type and not on AhoCorasick. Therefore, users of the crate could never call an overlapping search routine on an improperly configured automaton. The compiler simply wouldn’t allow it. > But this adds a lot of additional surface area to the API. And it does it in really pernicious ways. For example, an AhoCorasickOverlapping type would still want to have normal non-overlapping search routines, just like AhoCorasick does. It’s now reasonable to want to be able to write routines that accept any kind of Aho-Corasick automaton and run a non-overlapping search. In that case, either the aho-corasick crate or the programmer using the crate needs to define some kind of generic abstraction to enable that. Or, more likely, perhaps copy some code. > I thus made a judgment that having one type that can do everything—but might fail loudly for certain methods under certain configurations—would be best. The API design of aho-corasick isn’t going to result in subtle logic errors that silently produce incorrect results. If a mistake is made, then the caller is still going to get a panic with a clear message. At that point, the fix will be easy. What I gather from this is that the author chose to define a type (call it A) with an attribute that when set in a certain way will cause certain functions to panic. This was preferred to the alternative (two types, A and B) with functions specific to each and where panic was not possible. This kind of design decision comes up a lot, so understanding the reasoning here could be helpful in a lot of situations. Unfortunately, the passage is less than clear due to lack of source code inline and the highly-specific nature of the problem. An example with source code using more accessible algorithms might be an improvement here. That said, I'm skeptical that the full range of approach was considered. I sometimes find that the presence of unwrap is a smell pointing to types that have not been fully fleshed out. As an extreme case, consider a struct whose fields contained diverse data (numbers, colors, enumerated values), but which are all defined as strings. It will be very easy to put this struct into an inconsistent runtime state because nothing can be checked at compile time. The type itself is anemic. Replacing strings with more constrained types eliminates opportunities for panic - possibly all of them. I get that the whole point is "at what cost?" All I'm saying is that the tradeoffs aren't clear from the example in the passage.