Error ergonomics

Hi everybody,

I was meant to write a blog post for the #Rust2018, but my blog is basically blocked because of the issue I am going to write about, so… :slight_smile: I hope you don’t mind I expose my half-planned idea here.

I saw the discussion around the catch Pre-RFC, and, together with the failure crate and the previous attempts (error-chain, for example) I appreciate a lot of people are trying to fix the problems we have with Error management, and I am extremely grateful to everyone who is trying.

But I think the solution can only be found in the compiler, as crates are too limited in this, especially in the stable channel.

The problem is that, in Rust, creating errors is quite easy, passing them around is hard, and catching them is nearly impossible, at least in my experience.

I am the author of strange, an almost unknown static website generator, and to write it I had to integrate different crates, namely pulldown, tera, handlebars, serde, hyper among others.

Almost all of them (plus std::io) have a different way of creating errors, and of course I want to add some debugging context to them.

The best solution was error-chain. It was as simple as defining a lot of foreign_links{} and .chain_err() every Result.

This made the compiler happy, as all my Result<Something> were returning the same error type: error-chain’s.

I tried to convert the codebase to failure and… failed. For over a week. :confused:

This made me think that what error-chain is doing should be the std::Error object (trait, struct, enum, whatever it takes. Probably a combination of all three) in the core. Create a special case if needed, but:

  • all the errors should be automatically converted into the core Error. No need for .into() or ?
  • they should be chainable (have a context that in turn can have a context, etc.)
  • they should support optional backtraces
  • they should be matchable

Failure is getting there, but adding a context is a real pain, and wrapping other errors is almost impossible. You need a lot of boilerplate.

An example of what I wish:

function read_config(f: &str) -> Result<Config, Error> {
  file::open(f)
    .and_then(|f| BufReader::new(f).read_to_end())
    .and_then(Config::from_json)
//    .error_context(|| format!("Error loading {}", f) // optional
}

(look, ma’, no question marks! :slight_smile:)

Basically I don’t care if I am returning an io::Error, a config::Error, a serde::Error or whatever.

When I use it, I would like to do

match read_config(f) {
  Err(io::Error(msg)) => panic!("Your disk is broken!"),
  Err(json::Error(msg)) => panic!("Your json is broken!"),
  Err(config::Error(msg)) => panic!("Your config parser is broken!"),
  Err(_) => panic!("Everything is broken!"),
  Ok(c) => panic!("No panic!")
}

And, naturally, print the causes like in the standard dump_error() function error-chain suggests.

Obviously, the errors should impl std::Error to be automatically converted, and the std::Error becomes an automatically-growing enum that can be matched against all errors that impl it.

It’s only half of an idea, but I hope you get the point.

As I said, the situation in the crates is already pretty bleak, as everyone implements a different error crate, and it’s going to get worse, with people not upgrading, eitheri because they can’t or unable (like me), and an official solution would at least solve the problem in a single way for the future, instead of leaving people to choose different paths.

I apologise if this came out too similar to a rant, but this is probably the most frustrating thing I found in Rust until now.

Thanks for reading. :slight_smile:

5 Likes

Can you say a bit more about what problems you had with failure?

Is this because you don't like ?, or are you making some other point here? It seems like ? (and the associated conversions) would help in this particular example.

6 Likes

I don’t agree with your conclusions but I think your pain points are very real.

As I think you kind of identify, the Error type in failure is intended to be the sort of global error type that you want. There are some limitations on it right now, though:

  1. Destructuring/downcasting it to a concrete error type. It seems like this is your big problem with failure. This is one motivation I have for allowing you to pattern match on trait objects to downcast them, so you could do something like this:
match err {
    err @ dyn io::Error => { ... }
}
  1. Returning your own errors. The ? performs an into conversion, but return Err(MyError) does not, so you end up doing either Err(MyError)? or return Err(MyError.into()), neither of which is super pleasant. This is the motivation behind throw in the catching functions thread.

  2. Error does not implement Fail. This is really unfortunate, because if you want to write a helper function you can have quite a bad time. This has definitely been the unforeseen problem of the failure crate (the other two problems I identified beforehand). The solution to this is specialization, but that’s proven to be a tough feature to design. I kinda think the real mistake was using the normal Into trait instead of a custom trait just for this use case.

In other words, I agree that we’re not quite there yet, but we have solutions in the wings for all of the big outstanding problems.

7 Likes

Let me start by saying that I think the problem is not in failure, error_chain or any crate that uses them. I think the real problem is in the lack of a good error “thing” in the core language, that forces people to create error_chain and failure. I think the error management should be one of the basic blocks of a language, because the alternative is chaos and fragmentation, and many solutions (failure, ? and the catch pre-RFC) are just patches onto the big problem.

As for the ? operator, I still have mixed feelings. Sometimes I find myself trying to add it or remove it randomly and waiting to see what the compiler thinks of it.

It is great as an alias for .unwrap_or_return_error() (like in let f = file::open(f)?;), but when using combinators (that I love) is confusing. Sometimes I end up with a line ending in ...)?))?;

Now, coming to my particular problem with failure. This is the errors.rs using error-chain:

https://bitbucket.org/Alex_PK/strange/src/1337c7901880c194932bd9da8fbc79b9b2f02b4b/src/errors.rs?at=master&fileviewer=file-view-default

This is one of my attempts with failure:

https://bitbucket.org/Alex_PK/strange/src/a78d8d6916a27d26f7b5b3ff0076457be2e83bd8/src/errors.rs?at=failure&fileviewer=file-view-default

(the whole “failure” branch is but a small view on all the attempts I made to convert the codebase).

One goos example is this, in the handlebars functions:

With error-chain:

pub fn render_page(&self, tpl_type: &str, data: &Object) -> Result<String> {
	self.engine.read().unwrap()
		.render(tpl_type, data)
		.chain_err(|| ErrorKind::Tpl("Hbs".into(), format!("Unable to render template '{}'", tpl_type)))
}

A series of attempts with failure:

pub fn render_page(&self, tpl_type: &str, data: &Object) -> Result<String, Error> {
	self.engine.read().unwrap()
		.render(tpl_type, data)
		.context(ErrorKind::Error(format!("Unable to render template {}", tpl_type)))?

//		.map_err(SyncFailure::new)
//		.compat()
//		.with_context(|_| ErrorKind::Tpl(format!("Unable to render template {}", tpl_type))).into()

//		.map_err(|e| ErrorKind::HbsRender(SyncFailure::new(e), format!("Unable to render template {}", tpl_type)).into())

//		.with_context(|_| format!("Unable to render template '{}'", tpl_type))
}

And it’s still not working, because .context() creates a Context, not a Fail or an Error, and Context doesn’t have into Error, and I don’t know how to implement it, especially without rewriting half of the failure crate. The Error object seems to have private causes and backtraces (ar, at least, I found no way to set them), so I can’t use it either.

I went through https://boats.gitlab.io/failure/ maybe 20 times, already, and I’m still not sure what I should do, but the problem is always the same: there should not be 4 ways to do it. Just one, and it should be the official way.

I honestly don’t know what the solution could be. Maybe having a Result<T, impl Error> will solve everything, but my gut is telling me Error should be special at the compiler level, and the fact that special cases were created or are planned for it (? and catch) tells me I am at least half right.

My proposal is probably too similar to Exceptions, as I used them for the last 15 years in several languages, and my brain is wired to them, but this is also probably a good way to lower the entry barrier to the language: having a concept that is similar even if not identical.

1 Like

Context normally implements Fail, which means it implements Into<Error>. As your code sample shows you've identified, the actual problem has to do with Sync. The errors generated error-chain are never Sync, which is a (quite serious) bug in the error-chain crate. If the argument you pass to context is not Sync, context doesn't implement Fail.

A simple solution would be to just write this, since I don't know what using the error-chain type is buying you here:

self.engine.read().unwrap()
    .render(tpl_type, data)
    .context(format!("Unable to render template {}", tpl_type))?;

No joy… :confused:

error[E0599]: no method named `context` found for type `std::result::Result<std::string::String, handlebars::RenderError>` in the current scope
   --> src/tpl/engine_handlebars.rs:102:5
    |
102 |    .context(format!("Unable to render template {}", tpl_type))?
    |     ^^^^^^^
    |
    = note: the method `context` exists but the following trait bounds were not satisfied:
            `std::result::Result<std::string::String, handlebars::RenderError> : failure::ResultExt<_, _>`
    = help: items from traits can only be used if the trait is in scope
    = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
            candidate #1: `use quick_error::ResultExt;`

I don’t know where quick_error comes from, maybe a dependency.

Obviously I have

use failure::{SyncFailure, ResultExt};

I am pretty sure I already tried that, and that’s the reason I tried to wrap it in so many ways.

This commit in handlebars makes the RenderError implement Sync: https://github.com/sunng87/handlebars-rust/commit/4e24a37b0fb433f3de42c7443cbee3bcbdd0ab5a

You are probably using a version of handlebars that does not contain that commit. I would recommend updating your version of handlebars to the latest version.

EDIT: It looks like that commit is only in the version 0.30.0-beta4, the absolute most recently released version

Maybe what we want is just anonymous sum types that are inferred, ideally globally, and matchable?

That is, you could have a function return “Result<RetType, impl ErrorTrait>” and then have the compiler automatically build an anonymous enum containing all its error types and implement some “ErrorTrait” type by matching.

Then, with an another additional feature you could match on it, specifying any type implementing ErrorTrait in the match branches.

If you wanted to be more specific, the crate could declare an Error type as an “abstract type” and have it inferred as a sum type globally in the crate, or the library could specify it as an anonymous sum type.

The other problem is adding backtraces, which could be done by extending the “?” operator to transform the result by adding a backtrace entry to it, after augmenting Result to also optionally store a backtrace.

Also, one could maybe change the compiler to implement all the error functionality by unwinding, since it’s probably faster (for the common non-error case).

1 Like

First of all: thank you for all the help. I really appreciate it.

But I am sorry to say that all my attempts are doing nothing more than adding frustration and enforcing my idea that errors in rust are completely screwed.

I updated handlebars to 0.30-beta.4.

This is using my own Error as described in “An Error and an ErroKind pair” in the failure site:

self.engine.read().unwrap()
	.render(tpl_type, data
	.context(ErrorKind::Error(format!("Unable to render template {}", tpl_type)))?

Error:

error[E0308]: match arms have incompatible types
   --> src/tpl/engine_handlebars.rs:100:3
...
^ expected enum `std::result::Result`, found struct `std::string::String`
    |
    = note: expected type `std::result::Result<std::string::String, errors::Error>`
               found type `std::string::String`

Removing the question mark:

error[E0308]: mismatched types
   --> src/tpl/engine_handlebars.rs:100:3
    |
99  |    pub fn render_page(&self, tpl_type: &str, data: &Object) -> Result<String, Error> {
    |                                                                --------------------- expected `std::result::Result<std::string::String, errors::Error>` because of return type
...
^ expected struct `errors::Error`, found struct `failure::Context`
    |
    = note: expected type `std::result::Result<_, errors::Error>`
               found type `std::result::Result<_, failure::Context<errors::ErrorKind>>`

Using failure’s Error:

error[E0308]: match arms have incompatible types
   --> src/tpl/engine_handlebars.rs:100:3
    |
100 | /   self.engine.read().unwrap()
101 | |    .render(tpl_type, data)
102 | |    .context(format!("Unable to render template {}", tpl_type))?
    | |_______________________________________________________________^ expected enum `std::result::Result`, found struct `std::string::String`
    |
    = note: expected type `std::result::Result<std::string::String, failure::Error>`
               found type `std::string::String`

Removing the question mark:

error[E0308]: mismatched types
   --> src/tpl/engine_handlebars.rs:100:3
    |
99  |    pub fn render_page(&self, tpl_type: &str, data: &Object) -> Result<String, Error> {
    |                                                                --------------------- expected `std::result::Result<std::string::String, failure::Error>` because of return type
100 | /   self.engine.read().unwrap()
101 | |    .render(tpl_type, data)
102 | |    .context(format!("Unable to render template {}", tpl_type))
    | |______________________________________________________________^ expected struct `failure::Error`, found struct `failure::Context`
    |
    = note: expected type `std::result::Result<_, failure::Error>`
               found type `std::result::Result<_, failure::Context<std::string::String>>`

I can probably work out a solution by spending more time on it, wrapping errors more, implementing more traits, etc.

The point is: I should not have to.

Apologies for a comment from the sidelines, but the errors make it seem like you just need to wrap your final expression in Ok(...).

Using foo()? will essentially return Err(converted_error) and otherwise unwrap the value in the result. So since your function needs to return a result, you’ll need to mark the normal value as the Ok result like Ok(foo()?).

Again, I might be off the mark here, but that’s what jumped out at me from the errors (expected type 'Result<X, ..>', found type 'X'.

Hi phaylon,

no need to apologise. We are here to discuss together, right? :slight_smile:

Your solution is probably the quickest and simplest.

What I would like to see changed in Rust is exactly the ability to avoid it. Writing Ok(something()?) is ugly. I would prefer to write just something()?.

@Alex-PK

But Ok(something()?) and something()? are two completely different things.

That’s where opinions differ somewhat.

Yes, it’s a bit ugly (especially Ok(()) sometimes). On the other hand, Do What I Mean solutions are harder to reason about what happens and can do surprising things at times. (eg. auto-conversion operators in C++ are source of many hard to find bugs). It kind of goes against being strongly typed and such. I myself prefer the current explicit but ugly way over the surprises.

1 Like

something()? is a T and Ok(something()?) is a Result<T, E>. If you mean Ok(something()?) then write Ok(something()?). I grew up on languages that allow you to write one thing and mean another. All it does it make code more confusing. More confusing to read, and more confusing to interpret the errors from. If you were able to write something()? where a Result<T, E> was expected - even though the types don’t match - that would be ugly. And that kind of nonsense was deliberately avoided when designing Rust.

Yup, sorry. My train got into station and I sent the message before finishing it. :smiley:

I meant I just want to write something(), not something()?. Take this, for example:

fn something() -> Result<String, IoError> { ... }

fn myfunc() -> Result<String, DifferentError> {
  something()
}

I would like to be able to do it without thinking about transforming the error manually.

The reason I am still undecided about the ? operator is probably this: it does it, but it’s neither explicit nor implicit. You lose the ability to check the original error up in the call chain. You cannot trace back where the error comes from. And you still have to write the ? plus the Ok() wrapping.

Maybe it’s better to make it completely implicit (as I suggested) or make it completely explicit (by implementing a .wrap_err() or something similar), so I can trace back.

Yep, you can actually do it in two ways (and can choose which one):

Ok(something()?)
something().map_err(From::from)

They do the same, but convey a slightly different connotation. The first one is „check if something suceeded, exit with the converted error if not. All went well? Then we’re done with whatever the something returned, cool, celebrate“.

The other is „Return whatever something returned, just convert its possible error as needed“.

The reason I am still undecided about the ? operator is probably this: it does it, but it’s neither explicit nor implicit. You lose the ability to check the original error up in the call chain. You cannot trace back where the error comes from. And you still have to write the ? plus the Ok() wrapping.

Maybe it’s better to make it completely implicit (as I suggested) or make it completely explicit (by implementing a .wrap_err() or something similar), so I can trace back.

Okay that I agree with, sorry for ranting. I actually think that the ? operator was a mistake as is the whole of error-chain and failure. If someone wants to throw an error which is opaque (can't be handled programatically) and returns an error message with a call stack, then that's what panic!() is for. The return value should be for values that are actually meant to be handled. ? gives people an easy way to not handle an error and just mindlessly throw it up the stack, even changing its type in the process and losing almost all the relevant information about what caused it. AFAICT the original, root cause of all this is io::Error. io::Error set a precedent of having Rust code use shitty, non-typesafe error types. It's the reason that opening a file can result in an (eg.) WriteZero error. It trained people to not look at error values because they know that most the variants of the error can't possibly occur and they don't expect to be able to exhaustively match on the ones that can, or to be able to tell which is which. Once we polluted our code with meaningless, unusable error types we were forced to give people ? to brush them out of the way.

But this is all academic since we can't turn back time now.

4 Likes

I think there are some vague plans to be able to unwrap the original errors (or, to do more general downcasting of trait objects). It doesn’t solve all the problems, but it does solve the problem of opaqueness. And having one carrier error type isn’t a problem, if used in a reasonable way.

Although Box<Error> (or failure::Error, or whichever is the preferred way these days) throws away the possibility of exhaustively pattern matching the error (which I agree sounds like a bad idea outside of prototyping and quicky one-off programs and such), much like panic!() does, at least they still preserve the property that you know which functions can't result in an error (i.e. the ones which don't return a Result), unlike panic!().

My sense is that there's still a material difference between indiscriminately catching all application-level errors using Error and printing an error message or whatever (which are generally problems in the environment like "file not found"), and also catching every other possible panic (which are generally bugs in the program), which would lead into a Java-land of throwing up dialog boxes saying "NullPointerException" or "Array access out of bounds" or so on.

2 Likes

As an aside, this seems relevant: Pre-RFC: Catching Functions