RFRFC: Better Option/Result error messages

[Aaronepower]

Currently .unwrap's are very unhelpful to a user. As seen by this playground. unwraps don’t even provide user’s with line numbers, or the function the panic occurred in. This leaves the user confused, and forced to go, and do a search, and replace, or a series of println!'s to figure out where their program has failed.This has made the use of unwrap, an anti pattern for the most part. The current recommended method is to have your own match statement, or to use expect, and provide your own error message. I was thinking of constructing Options with a error message.

This would allow API designers to optionally provide their own messages which would be much more descriptive than unwrap on None. The user’s expect message would of course override the message the Option is constructed with.

Pseudo code example:

fn return_option(x: u8) -> Option<u8> {
   Option::none_with("couldn't do the math? in: return_option")
}

[burntsushi]

Using unwrap isn’t an anti-pattern. Using unwrap as a way to show an error to an end user absolutely is an anti-pattern and really should never be done. If your program hits an unwrap, then it should likely be considered a bug and fixed. Panic messages aren’t suitable for end users.

However, unwraps as bug reports to the programmer are indeed quite useful. You can get a full backtrace out of them by setting RUST_BACKTRACE=1 when you run your program.

Using expect instead of unwrap is generally considered good practice, if only because it helps the programmer debug the problem.

If you want to return a detailed error message, use Result instead of Option.

[Aaronepower]

Using unwrap isn’t an anti-pattern. Using unwrap as a way to show an error to an end user absolutely is an anti-pattern and really should never be done. If your program hits an unwrap, then it should likely be considered a bug and fixed. Panic messages aren’t suitable for end users.

unwrap always prints an error message to the end user if it fails. unwrap will always be a way show an error message to the end user.

However, unwraps as bug reports to the programmer are indeed quite useful. You can get a full backtrace out of them by setting RUST_BACKTRACE=1 when you run your program.

This is impossible to convince an end user to do this in order to get the backtrace. Especially on Windows which even if you could, currently only provides a stack trace.

Using expect instead of unwrap is generally considered good practice, if only because it helps the programmer debug the problem.

Even if you only use expect's there is no guarantee the library you depend on does. When it panic!'s from, and unwrap due to your parameters. If you have multiple libraries, it’d be impossible to even figure out where to start.

If you want to return a detailed error message, use Result instead of Option.

Result's are just as bad once there is more than one place making the same type of call. Result's also don’t print well in panic!'s currently usually printing structName {kind: EnumName } which aren’t specific about how that’s the error type, and what could have caused it.

[rkruppe]

unwrap always prints an error message to the end user if it fails. unwrap will always be a way show an error message to the end user.

Panics are a terrible way to show error messages, as you note. The solution is to avoid panics as far as possible, not to make them slightly less terrible. I’m not sure if I’m misreading here, but panics should never be deliberately used to show the end user a message.

This is impossible to convince an end user to do this in order to get the backtrace. Especially on Windows which even if you could, currently only provides a stack trace.

Yes, debugging the issues non-technical users report is hard. But this proposal does not help much, as I will argue in more detail below. A better approach might be making it easier for users to give you the stack trace, e.g. by dumping it to an easily located file when a panic happens. People are already working on enabling this, in particular to allow reacting to a panic in a user-defined way (e.g. displaying a custom message to the end user).

Even if you only use expect’s there is no guarantee the library you depend on does. When it panic!'s from, and unwrap due to your parameters. If you have multiple libraries, it’d be impossible to even figure out where to start.

Where’s the guarantee that the libraries you depend on will put a meaningful error message on the Option? Especially since constructing a bare None (i.e. without error message) must remain valid for backwards compatibility.

Result’s are just as bad once there is more than one place making the same type of call.

Speaking of this, if there are several places calling (using your pseudo-code example from the first post) return_option, how do you tell which of those calls produces the None("couldn't do the math?")? And how do you get the value of x which provoked that? And the list goes on. This proposal feels more of a band aid to me, it doesn’t seem to address the actually hard parts of tracking down the source of an unexpected None.

Result’s also don’t print well in panic!'s currently usually printing structName {kind: EnumName } which aren’t specific about how that’s the error type, and what could have caused it.

So use (or lobby for std and other libraries to use) Result<T, &'static str> or Result<T, String> or an error type that implements Debug in a prettier way. In fact, your proposed extension of Option is isomorphic to Result<T, &'static str> or Result<T, String>. What you want already exists, people just don’t use it frequently. Partly because error handling is something people are not naturally good at, but also because it is not as useful as you think. Panics are bugs, panic messages are therefore aimed at the developers. Furthermore, developers have stack traces available and those already tell you which call panicked.

To be clear, I’m not claiming the situation is perfect. There is much that could be improved, for example it can indeed be tricky to make the connection between “this unwrap call failed” and "this line caused the the Option to be None". However, I don’t see how this proposal really helps. And I haven’t even started seriously considering its downsides and costs.

[Aaronepower]

The solution is to avoid panics as far as possible, not to make them slightly less terrible.

I think that’s an impossible goal. Run-time failures are an unavoidable part of programming. So a goal of a programming language should be to make them easier to find, and fix. Not to expect people to program perfectly the first time, then they’d never have to worry about Run-time errors, and then they wouldn’t also need any of Rust’s safety guarantees.

Where’s the guarantee that the libraries you depend on will put a meaningful error message on the Option?

There would be no guarantee’s that library writers would do this, and this isn’t designed for bad programmers, as bad programmers are always going to give you bad error messages. This is to enable good programmers to have helpful error messages so people have an easier time using their library. Also if this were implemented we could at least guarantee rust itself has helpful error messages, which would be enough for me.

how do you tell which of those calls produces the None(“couldn’t do the math?”)? And how do you get the value of x which provoked that?

My example was trival. A better example would be something like the following. Where you could attach what the parameter values, and based on the code path provide a different error message.

fn foo(x: &str) -> Result<usize, NumEnum> {
   let x = match u8::from_str(x) {
       Ok(value) => value,
       Err(err) => Err(err).with(format!("Couldn't get a number from input. PARAMETERS: x:{}", x))
   }

  if x < 10 {
     Err(SmallNumber).with(format!("X is less than ten. PARAMETERS: x:{}", x))
  } else {
      Ok(1337)
  }
}

your proposed extension of Option is isomorphic to Result<T, &'static str> or Result<T, String>. What you want already exists

No it doesn’t. As that option makes Err's impossible to match against. Also things like IntParseError already have descriptions, but they hidden in the source code, and not used publicly. What I want is a message that is only shown on panic!. This would allow programmers to keep with the enums which allow for easier error handling. And when they haven’t handled an error, or the library they use panic!s they’ll have a helpful error message so they could easily fix it.

[luxvio]

I can’t have much opinion because I mostly write code about REST APIs and panic’ing (overriden message or not) is the horror in that particular area … but I would like to point something.

This is impossible to convince an end user to do this in order to get the backtrace. Especially on Windows which even if you could, currently only provides a stack trace.

burntsushi:

To be honest, I don’t know if this is kinda the correct way of doing it, but I do this at initialization time (it can be added an if to the set_var to check if the build is a debug one, along with the RUST_LOG counterpart), and works:

use std::env;

fn main()
{
    env::set_var("RUST_BACKTRACE", "1");
    let bad_string = "dont parse me as int";
    let _ : i64 = bad_string.parse().unwrap();

}

Rust playpen → http://is.gd/0XCVL1

In one of my crates I got a module called logging which optionally sets up RUST_BACKTRACE and RUST_LOG and fires up logging with the log crate through syslog, or env_logger (configurable with clap, for a rest api).
I don’t know how future-proof it is, but it’s awesome to be able to change that at runtime. So no need to ask changing environment variables to a user for now

Regards.

[wthrowe]

Whether or not this is desirable, it is not possible because it conflicts with the guarantees given by the Option type. Option<&u8> is guaranteed to have the same representation as a pointer, so there is nowhere available to store such a message. (Also there is the technical issue that Option is defined in libcore and can’t depend on anything that uses an allocator (like String), so this would have to be restricted to &'static strs, removing a large chunk of any benefit.)

[Aaronepower]

That’s a pretty cool trick I’ll have to use that. RUST_BACKTRACE doesn’t work well on Windows, and this solution wouldn’t work building a library. Programmers wouldn’t appreciate you setting environment variables that aren’t part of what your crate’s purpose especially on release builds.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.