Idea for improved error handling ergonomics

[jnicklas]

The new Error trait and error casting is really great, and definitely makes handling errors much smoother. But it got me thinking that it would be extremely convenient if Rust provided a sensible default error type that would be convenient to work with. This error type could become the default type parameter of Result.

It would make it possible to write code like this:

fn read_file(path: &Path) -> Result<String> {
  let buffer = try!(File::open(path).read_to_end());
  let result = try!(String::from_utf8(buffer), "cannot read binary file");
  Ok(result)
}

Note that read_to_end returns a IoResult<Vec<u8>> and from_utf8 returns a Result<String, Vec<u8>>, but this all just works. No need to manually wrap errors or define any custom error types or cast back and forth, and it still provides good error messages.

Currently I’ve implemented this as a separate crate here: https://github.com/jnicklas/standard_error

Having this as an external crate is fine, but I think it’s generally useful, and it would be pretty awesome if it just worked with the regular Result type.

What do you think?

[mitsuhiko]

Have a look at rust-welder which is an experiment to improve the error handling system. https://github.com/mitsuhiko/rust-welder

[jnicklas]

@mitsuhiko I’ve seen rust-welder, thank you for pushing for better error handling, having error locations would be a huge step forward, IMO. I also included your fail! macro in case you didn’t see it.

I think both of these ideas complement each other very nicely. The StandardError enum I wrote could definitely make use of the CommonErrorData struct in welder.

My main purpose was getting away from having to define custom error types at all, when they’re not really important, and instead providing a sensible default which would ease the learning curve and make people productive faster. I’m not quite sure how you intend all of the items in welder to be used, so I might well have missed something in case you’ve added something which pulls in the same direction.

[mitsuhiko]

I’m playing with the idea of removing custom error types and having one, but currently that makes extracting extra information from those errors much harder. I was hoping associated types would help but they ICE on me left and right.

Ideally you have a generic error but you can do error.get_data::<NotFound>() and if the error has data of type NotFound you can operate on the returned data. But it’s really quite tricky to do nice things that make sense from programming and performance point of view.

[mitsuhiko]

I made a second attempt now that goes much further and changes the meaning of try!. Downside is that it no longer works with non errors but the upside is, that you can now have tracebacks.

mitsuhiko/rust-eh

Can produce debug tracebacks like this:

Traceback (most recent cause last):
  File "/Users/mitsuhiko/Development/rust-eh/examples/test.rs", line 42
    fail!(FileNotFound { file: Some(Path::new("/missing.txt")) });
  File "/Users/mitsuhiko/Development/rust-eh/examples/test.rs", line 46
    try!(test());
File not found: Failed to locate file (file=/missing.txt)

The above error resulted in another:

Traceback (most recent cause last):
  File "/Users/mitsuhiko/Development/rust-eh/examples/test.rs", line 52
    Err(err) => fail!(RecordNotFound, "could not find record", err),
Record not found: could not find record

[phildawes]

Maybe you could also add the output from std::rt::backtrace. E.g. capturing it with something like

use std::rt::backtrace;
use std::io::MemWriter;
let mut m = MemWriter::new();
let tracestr = backtrace::write(&mut m).ok()
                    .map_or("NO backtrace".to_string(), 
                          |_| String::from_utf8_lossy(m.get_ref()).to_string());

[jnicklas]

@mitsuhiko nice there’s a lot of really great stuff in there, this improves the status quo a lot.

What do you think about allowing try! to propagate additional arguments, like I added in standard_error. This would allow this:

fn baz() -> EhResult<()> {
    match bar() {
        Err(err) => fail!(RecordNotFound, "could not find record", err),
        Ok(x) => Ok(x),
    }
}

to be rewritten as:

fn baz() -> EhResult<()> {
    Ok(try!(bar(), RecordNotFound, "could not find record"))
}

Haven’t looked into how you implemented this, but it seems like that should be possible, right?

EDIT: this is extra useful where Result contains non-errors, like String.from_utf8.

[mitsuhiko]

Yeah, I was thinking of stealing that but then I did not want to encourage people to chain things too much Propagating is free, every time you do this however you build a new error even in release. I want to be at least a bit conscious about performance

[mitsuhiko]

I think I made a huge step forwards today with a less controversial attempt to error handling. It’s based on an idea that aturon came up with which uses tracebacks in debug builds and regular old errors in release builds.

The idea is that there is a Failure<E> which wraps an error like a Box. That way results stay small and the type derefs into the actual error. It uses FromError automatically so not much changes.

The big advantage is that in debug builds Failure<E> also carries debug information which can build entire tracebacks like before. API wise however nothing changes.

Code: https://github.com/mitsuhiko/rust-incidents Docs: http://mitsuhiko.github.io/rust-incidents/

[arcto]

I appreciate your efforts in trying to improve error handling. I have two remarks, however - one general and one specific to this solution:

1. Descriptions and other text aimed at the users are an abstraction leak, IMO. The error is an internal value; its consequence for the user is another story that should be told by the GUI of the caller. I think including them is a design error caused by not differentiating between developers and users.

2. I don’t like heap allocations in the general case (release) as a way to implement a debug feature. They introduce costs where none should be necessary.

[mitsuhiko]

Descriptions and other text aimed at the users are an abstraction leak, IMO. The error is an internal value; its consequence for the user is another story that should be told by the GUI of the caller. I think including them is a design error caused by not differentiating between developers and users.

That’s entirely incorrect. The reason for this is that not every error can be handled in a specific matter and not being able to give the user even a fallback error is the reason for user frustration (error 0x334223433) does not in any way help, however “Resource Unavailable: ran out of resource handles” gives a much better clue at what is happening. I have seen this time and time again that the “errors need to stay internal and be translated to users” result in an absolutely abysmal user experience for trying to google a problem. Separately though those are also intended to be used by developers.

I don’t like heap allocations in the general case (release) as a way to implement a debug feature. They introduce costs where none should be necessary.

There is no other place available. If you do not heap allocate them you destroy your performance for the 90% case of “no error happened” by blowing up the results to ridiculous sizes. It’s a major problem with the io error currently. A IoResult<u8> is 72 bytes!

[mitsuhiko]

As for no other place: with a wrapper type it could become an option to store it in TLS somewhere.

[arcto]

I think you misunderstand. Neither “error 0x334223433”, nor details about “resource handlers” is at the right level of description. Only the calling application has the required knowledge about the context of the call, which is why it should provide the error message, if a textual one is even needed.

I argue that neither of desc nor detail is a needed property of the error value.

How many bits does the IoErrorKind really need to represent all error kinds? Let’s see. There are 20 enum variants, that’s 5 bits of information. Let’s be generous and allow 1024 different error variants (i.e. 10 bits), per static error type. There you have 22 bits left of a 32 bit word to store a hash or serial no. of the source code location, for no extra cost. That’s pretty good.

[arcto]

Also, how is the library supposed to know the locale and language? Again, it’s the responsibility of the application (i.e. caller) to provide meaningful feedback to users.

[mitsuhiko]

It is not. The error messages provided are english only and untranslated. If you want to translate errors for end users you need to do the same thing you are proposing: map them.

[arcto]

I’m sorry. I should learn to mind my manners.

I think we should try to find an error representation that minimises size. Therefore I suggest not including anything that isn’t strictly necessary. Also, I have explained that the error strings are really a misdirected effort. The issuer of the error lacks information about context, locale, language. With tooling a hash value or serial number can be used to locate the error - that’s the improvement!

[mitsuhiko]

I think we should try to find an error representation that minimises size.

The error representation is up to the user. The whole point of incidents was to box them up so that the results get as small as possible.

Therefore I suggest not including anything that isn’t strictly necessary.

This is a trait method, it does not add anything to the size of the error. The trait methods are necessary to render an error under debug circumstances (and as a fallback) to a developer and user.

Also, I have explained that the error strings are really a misdirected effort.

For a world of applications that need to deal with i18n yes, for anyone else the lack of error descriptions is just causing frustration. All methods are already optional, if your errors do not need descriptions or you do not see the need to add them, you have the option.

With tooling a hash value or serial number can be used to locate the error - that’s the improvement!

Errors implement TypeId. That should help to map them to error strings in different languages.

[arcto]

The most efficient solution is probably one based on storing the error value in TLS, as you proposed. Using a wrapper similar to Option could mean zero bytes overhead if the Ok value is nullable (and the overhead of an enum otherwise).

I maintain that error messages should be avoided.

[system]

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