Add support for source forwarding in Error derive

[MegaBluejay]

Resolves #428
Related to #110, #200

Synopsis

Adds support for #[error(forward)] attribute, which forwards the .source() implementation to a field.

Solution

Use existing forward fields in State for parsing the attributes, and keep the source-inferring logic the same as without forwarding.

Backtrace forwarding

The current behaviour is that if the field annotated/inferred to be the source is annotated with #[error(backtrace)], a forwarded provide() is generated.

This was in the example usage, but not documented in the list above, so I added it.

Checklist

• Documentation is updated (if required)
  • Document new behaviour
  • Add an explanation of advised usage as previously suggested?
• Add errors on usage of #[error(forward)] when no source field was specified/inferred
• Tests are added/updated (if required)
• CHANGELOG entry is added (if required)

[MegaBluejay]

@tyranron I implemented this with the existing parsing since it was an easy change.

Would it be preferable to refactor the derive first, similarly to #286?

[tyranron]

@MegaBluejay no, let's postpone refactoring here as a minor priority task.

[JelteF]

copy paste error?

Suggested change

	4. The source field is annotated with `#[error(backtrace)]`. Then it would
	4. The source field is annotated with `#[error(forward)]`. Then it would

[MegaBluejay]

No that's correct, provide is forwarded only when the source is annotated with #[error(backtrace)].

That behaviour is unchanged in this PR.

The reasoning for keeping a separate attribute is to make source forwarding work on stable, since there's no good way of generating feature-gated code.

I also think this is somewhat unintuitive, but I haven't come up with a better idea.

Perhaps it could make sense to disallow #[error(backtrace)] on the source field if there's no #[error(forward)]?

[JelteF]

I'm having a hard time understanding the usecase for such forwarding. Could you share a concrete example for when this is useful?

[MegaBluejay]

Could you share a concrete example for when this is useful?

The main use case I can see is transparent error variants

Something like

#[derive(Debug, Display, Error)
enum Error {
    ...
    #[error(forward)]
    #[display("{_0}")]
    #[debug("{_0:?}")]
    Other(anyhow::Error),
}

Where there's no new information added by the wrapper, so adding it to the source stack is unnecessary.

[tyranron]

@MegaBluejay yes, I start thinking that maybe transparent is better here than forward. Let's postpone it for a little while... I'll write up some design thoughts during the following week. And, of course, will describe the motivation more clearly.

[JonahPlusPlus]

Example use case:
I want to have a composite error type, and forwarding the source would simplify deriving Display and Error.

e.g.

#[derive(Debug, Display, Error)]
#[display("`{rust_name}` should work but {kind}")]
struct Error {
  rust_name: &'static str,
  #[error(forward)]
  kind: ErrorKind
}

#[derive(Debug, Display, Error)]
enum ErrorKind {
  #[display("foo was encountered")]
  Foo,
  #[display("bar errored")]
  Bar {
    #[error(source)]
    external: ExternalError
  }
}

We have data common to all variants, so moving it into a struct makes sense. But we also have source information only relevant to specific variants, so we need some way to use the source in the enum and we don't want to treat the ErrorKind as it's own source, since the enum doesn't represent an "abstraction barrier" by itself. So forwarding the error makes the most sense here.

Note that this pattern isn't available in thiserror, since #[error(transparent)] also delegates the Display impl.

[nerditation]

I start thinking that maybe transparent is better here than forward

what's the current status on this matter? it seems to be stalled for some time?

I agree #[error(transparent)] would be useful, and it is what other libraries like thiserror and snafu already have.

on the other hand, I would also like to have an orthogonal attribute to forward the source() method call, e.g. something like #[error(source(forward))], and maybe #[error(backtrace(forward))] too, for completeness. (well, this is not really orthogonal to, but probably mutual exclusive with transparent).

they serve different purpose and use cases, one example is shown by @JonahPlusPlus, where an error type shares some common information, while each variant has a different source, so it makes sense to use a struct Error for the shared fields, and use an enum ErrorKind to distinguish the different error cases and their sources.

in a manual implementation, I can match self.kind directly to extract the source, something like:

struct Error {
    // shared by all cases
    object_id: i32
    //...
    // different per error case
    kind: ErrorKind,
}
enum ErrorKind {
    OutOfBound, // no source error
    Read(io::Error), // io error
    ParseX(num:ParseIntError), // parsing error for `X`
    ParseY(num:ParseIntError), // for `Y`
    //...etc
}
impl StdError for Error {
    fn source(&self) -> Option<&dyn StdError + 'static> {
        match self.kind {
            //...
        }
    }
}

but for a procedural macro, a derive on struct Error cannot see the definition of enum ErrorKind, so the pattern matching must be derived on ErrorKind, but it is logically not a layer in the error stack, so I don't want to use it as the "source" of my struct Error type, and an attribute to forward just the source() method call would be great in this situation.

this design is also flexible, e.g. the ErrorKind might just holds the sources initially, but later each error case might add specific fields to the variant, then the ErrorKind itself can logically be the source of Error, so we can remove the forward annotation, e.g. #[error(source(forward))] to #[error(source)], and it still works. on the other hand, we could remove the common fields in the struct and (selectively) move them to some variants, in which case the struct Error becomes a real transparent wrapper of ErrorKind, and that still works too.

we may bikeshed the name, forward, delegate, passthrough, whatever, but we need to make the decision to support it first.