Error ergonomics

[Alex-PK]

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.

[phaylon]

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'.

[Alex-PK]

Hi phaylon,

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

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()?.

[canndrew]

@Alex-PK

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

[vorner]

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.

[canndrew]

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.

[Alex-PK]

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

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.

[vorner]

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“.

[canndrew]

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.

[vorner]

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.

[glaebhoerl]

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.

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.

[mark-i-m]

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

[withoutboats]

It is relevant in that the user’s Ok(expr?) problem is solved by catching functions.

[Alex-PK]

You are right about the stack trace. panic!() is the right solution in this case.

It would be nice to have “magic constants” (like __FILE__ and __LINE__ in PHP) to be able to include them in the error message, but this is another story.

My point in having just an “automatic sum type including all the errors” was exactly to not having to convert them to other errors (losing the information about the original error), either automatically or semi-explicitly, but also to allow me to catch them where they happen, or just delegate that to the caller.

That’s why I said error-chain was the best solution (at least in my “app, not library” case): it allows me to wrap the error and bubble it up, or to transform it into another error. What’s missing is the ability to easily maintain the original error: I have to list them all in the foreign_links, thus effectively transforming them. It’s just easier to wrap them.

I am following the catch functions discussion, even if it’s quite hard for me, not having compilers theory knowledge, and not knowing C++, C# or Haskell. I can only discuss it from an end-user point of view.

I disagree that adding sugar to function signatures is easier than having an automatic handling of all types implementing a trait.

The Result<T, impl Error> I was speaking about earlier keeps the explicitness (seeing Ok(T) and Err(e) in the code) and avoids Ok(function_call()?) where the result is not guaranteed to be Ok at all.

[FlorentBecker]

It is relevant in that the user’s Ok(expr?) problem is solved by catching functions.

I fail to understand what the “Ok(expr?)” problem is. Only thing I have found mentionned is that Alex-PK finds it ugly. Rust should try not to look ugly, but all taste is acquired. As has been expressed, it is the plainest way to “simply return the result of a fallible function call within another”. @Alex-PK, do you have a more precise way to phrase what you mean by “ugly”? @withoutboats, do you see anything else in the “Ok(expr?)” problem

If ugly means that the connotations of Ok and ? conflict into a “definitely maybe” mess, maybe the best approach for syntax hilighting for ?-ed expressions, to make clear what the order of operations is: the whole line is Ok as long as expr is.

If ugly means that the solution feels like a gotcha where discoverability is not enough, then arguably improving the compiler’s error message (and adding an autocorrection in the IDE) is the most direct solution.

[Alex-PK]

It’s been cited in the “catching functions” thread too, and in a few Rust2018 blog posts, from what I saw.

It’s ugly for two reasons: one is the one you say: is it ok or not?

The other is that it hides the “non happy path”. While reading the code it appears to be the correct result, but in effect it hides the possibility of an error.

And, to take my original problem, look at this:

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

It’s, at least, a code smell. Removing the whole Ok( and ?) makes it much more readable, and quickly glancing at the signature makes it clear that it can return an error, while the Ok makes it look like it won’t.

You can mitigate it by assigning to a temp var:

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

	Ok(res)
}

But don’t you think this is simpler and more readable?

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

[FlorentBecker]

In this precise case, in current Rust, I’d write the following which preserves the method-chaining style of the rest of the fuction.

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

I see now that map_err(From::from) looks like a type-checking tax, but I prefer paying that tax to any solution with “spooky type coercion at a distance”. At least, you only have to learn it in one context.

[KillTheMule]

It would be nice to have “magic constants” (like FILE and LINE in PHP) to be able to include them in the error message, but this is another story.

Did you know https://doc.rust-lang.org/std/macro.line.html and https://doc.rust-lang.org/std/macro.file.html?

[euclio]

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 => { … } }

Is there an RFC for this?

[withoutboats]

Not yet. I wanted to look at dynamic types as a whole before proposing any particular new features.