Deriving `Error`

The Error trait also has source() and backtrace() methods. They're optional, but is there any reasonable way we can support them?

2 Likes

This would be great to have, but it presents these design challenges and someone has to drive the consensus process:

  1. Error has a non-deriveable supertrait. Currently this is not true of any built-in deriveable traits; are we okay with this change to the consistency of the std derives?
  2. Error has several optional methods which users do care about - backtrace & source as Josh said. Can we generate these in the derive?
  3. If the solution to #2 requires attributes (as it does in many ecosystem derives), error would be the first std derive to have attributes. Again, are we okay with using this feature in std derives?

Obviously we shouldn't let theoretical purity around the std derives stop us from providing useful features to users, but we should also have a clear sense - if we are going to start introducing these complexities to the std derives - what the limitations on std derives will be. Probably we don't want this to start a slippery slope to providing derives that are complex DSLs with dozens of attribute features and so on.

11 Likes

(1) is a point that is obviously a matter of opinion; I think it's fine.

(2) has some analog, in the sense that Hash::has_slice and Clone::clone_from are optional methods. I have no idea if anyone actually uses those in practice, though, whereas the methods on Error are certainly used by some.

(3) is an interesting point. Would introducing a derive that has no options make sense? It would be forward compatible with adding attributes if that were ever desired.

I think having the trivial case handled would be worthwhile, while still allowing custom implementations that provide a source/backtrace. Though that raises the question: If there is a derive, would that lead people away from providing that info?

2 Likes

See also: thiserror

It goes a bit beyond derive(Error) in that it also acts as derive(Display). It also handles source and backtrace.

Personally, I like it. Personally, I'd also be potentially in favor of moving something like this to std, but I'd really like to see thiserror succeed first. (Wow, looking at its reverse dependencies, it already appears to be doing quite well.)

EDIT: Derp, I see thiserror was mentioned above. Ah well. So I guess if I'm to add something here, I'd say this: is it worth having a derive(Error) that doesn't handle Display?

7 Likes

I'd love to have a built-in derive(Display), too.

4 Likes

This is a good point, especially in lieu of https://github.com/rust-lang/rfcs/pull/2385, since it sorta implies we would never do that RFC, and perhaps we shouldn't.

I would personally like to see e.g. #[default] on enum variants used to make #[derive(Default)] work on enums. That seems quite useful to me.

What would it print?

4 Likes

I don't think we should have implied supertraits; rather, I think it makes sense to have aliases for sets of traits.

I'd love to have a mechanism to derive(Default) using #[default].

I'd want something very similar to thiserror, with a #[display] attribute. See https://github.com/dtolnay/thiserror/issues/65

3 Likes

Thiserror can derive for something that already has a Display impl. Just omit the error("...") attributes.

(It's part of my "default field values" draft as a holistic set of improvements re. defaults, https://github.com/Centril/rfcs/pull/19, but I haven't had the time to finish the RFC and I'm not sure we should burden the overburdened compiler team with that right now, albeit it would be a nice feature.)

5 Likes

Didn't realize that. Prior art, then! And given how popular the crate is, I think it's worth discussing at least.

I think there's value in finishing and reviewing that RFC, and it can sit on the queue to be prioritized based on value and available implementer time.

What would be the way forward on this? It seems like most of the people here seem to agree that in principle something along this line could exist, and the prior art via thiserror indicates that it could be quite common. There are a number of points that need to be worked out, of course.

I presume an RFC would be necessary at some point?

This is something of a tangent, but: talking with some people the other day, there was an interesting observation about thiserror and its current Error derive. Really the main value of thiserror is its Display derive,. If you want to write a crate with optional no_std support with thiserror, you don't actually want to derive Error, but you still want the Display derive. There's an open PR to add a std feature to thiserror so when you derive(Error) without that feature enabled, you don't... actually derive Error, you just derive Display, which seems weird.

For crates like this, it seems like the better way to go is instead a crate like displaydoc to do the Display derive, and then something like this:

#[cfg(feature = "std")]
impl std::error::Error for MyError {}

It'd sure be nice if that were instead:

#[cfg_attr(std, derive(Error))]
struct MyError { ... }

Error has several optional methods which users do care about - backtrace & source as Josh said. Can we generate these in the derive?

I think if I were using a custom derive, it'd be for the above trivial case that just treats std::error::Error like a marker trait. Anything fancier than that and I'd probably just handwrite it, personally.

I don't know what the implementation details are, but I'd sure love if more of the built-in custom derives supported attributes for various things, especially Default. I know there are plenty of additional crates to provide "power user derives", but for something like "I want to derive(Default), but change the default for one field", it's usually easy enough to handwrite the Default impl.

It sure would be nice to do things like:

#[derive(Default)]
struct MyStruct {
   foo: bool,
   bar: usize,
   #[default(myfunc)]
   baz: MyType,
} 

fn myfunc() -> MyType {
    ...
}

...or...

#[derive(Default)
enum X {
   #[default]
   A,
   B,
   C,
}
1 Like

I'll have to disagree here.

Errors that are a wrapper around upstream errors are typically still considered good (when implemented correctly), and are not uncommon. Implementing Error correctly means hooking up cause, etc. to the wrapped error.

Forwarding these methods is simple but very boilerplatey, and it just compounds for libraries taking the more-errors (as opposed to god-error) approach.

3 Likes

As others have mentioned with regard to Default and a possible #[default] attribute on enums, I think having a #[source] attribute to indicate what should be returned from the source() method would also make sense. From my (admittedly limited) proc macro experience, it wouldn't be too difficult to implement, either.

2 Likes

I strongly agree.

You'll want to use Error::source these days, but yes, I also agree with that.

To me that's gravy, and can be added in the future. I don't think that problem is big enough to block making some incremental progress on a std::err::Error derive which handles the "marker trait" use case, nor do I see how adding that derive now would negatively impact future work on such a feature.

The attribute DSL for doing that sounds like it'd need a lot of design work / consideration / bikeshedding, whereas a "marker trait" derive is a relatively simple, self-contained thing. As @withoutboats noted earlier, if you wanted to add an attribute DSL for this, it would be the first std derive to have attributes, which I think might make it even more controversial.

That said:

If that's all it is, it might be ok. Seems like it would only work for structs?

1 Like

Unless there were desire to implement it on enums as well. The source is optional, so it's not like each variant would have to have the source attribute.

Something like this would be possible:

#[derive(Error)] // assume Debug and Display are implemented
enum MyError {
    Foo,
    Bar {
        #[source]
        internal_error: io::Error,
    },
    Baz(#[source] io::Error),
}

could generate the following

impl ::std::error::Error for MyError {
    fn source(&self) -> Option<&(dyn ::std::error::Error + 'static)> {
        match *self {
            MyError::Foo => None,
            MyError::Bar { internal_error, .. } => Some(internal_error),
            MyError::Baz(_0) => Some(_0),
        }
    }
}

As I've said before, I believe this to be feasible to generate procedurally.

1 Like

I'm tenatively against adding a #[derive(Error)] that simply expands to impl std::error::Error for MyType {} with no support for backtrace or source.

My understanding is that if you wanted to add any custom handling for source / backtrace you'd have to remove the derive and type it out manually anyway. This seems like we'd just be adding a derive to save typing 10ish characters. When a new user wants to go and implement an error that has a root cause or a backtrace they're going to waste time trying to figure out how to do that via the derive only to eventually learn that it's just a short hand for an empty Error trait impl that they never learned how to work with in the first place. This seems like an unnecessary source of confusion for almost no gain.

That said, I'm totally in favor of something like thiserror getting into std with the accompanying attributes and support for backtrace / source / transparent errors and possibly some sort of basic Display derive to go alongside it, though I think that display derive should be more similar to thiserror's display derive in implementation than displaydoc's. I would not want to force people to use their doc comments as their display impls.

6 Likes

FWIW the derive_more crate just added a derive for Error in the latest release. There's a bit too much magic for my liking (automatically using a field named source, for example), but the general idea is quite similar to those in this thread.

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