derive_more icon indicating copy to clipboard operation
derive_more copied to clipboard

Add support for source forwarding in Error derive

Open MegaBluejay opened this issue 2 years ago • 6 comments

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

  • [x] Documentation is updated (if required)
  • [x] Add errors on usage of #[error(forward)] when no source field was specified/inferred
  • [x] Tests are added/updated (if required)
  • [x] CHANGELOG entry is added (if required)

MegaBluejay avatar Aug 14 '23 16:08 MegaBluejay

There were some updates to the error_generic_member_access API recently, which is why nightly tests are failing. I'll open a separate PR to fix this.

Done #294

MegaBluejay avatar Aug 15 '23 10:08 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?

MegaBluejay avatar Aug 18 '23 10:08 MegaBluejay

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

tyranron avatar Aug 18 '23 15:08 tyranron

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

JelteF avatar Aug 21 '23 20:08 JelteF

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.

MegaBluejay avatar Aug 23 '23 10:08 MegaBluejay

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

tyranron avatar Aug 23 '23 15:08 tyranron