Error: don't drop the original error

[sgued]

Hi,

When trying to implement some basic activitypub functionality, I found it very hard to know where the errors came from. For example, when implementing a basic actor and using webfinger_resolve_actor, I just get a WebFingerResolveFailed error, while my code generated more error information than that.

I suggest replacing the current Error value with:

pub enum ErrorKind {
    NotFound,
    RequestLimit,
    ResponseBodyLimit,
    ObjectDeleted,
    UrlVerificationError(&'static str),
    ActivityBodyDigestInvalid,
    ActivitySignatureInvalid,
    WebfingerResolveFailed,
    /// see Error::source
    Other // Here we remove the current anyhow::Error
}

pub struct Error {
    kind: Kind,
    source: Option<anyhow::Error>,
}

And implement the Error trait on top of the struct Error. That way using std::Error::source the full error information can be found.

[Nutomic]

I dont think a separate error type is the way to go. Better to change the enum variant WebfingerResolveFailed to WebfingerResolveFailed(anyhow::Error) and then replace this line with .map_err(Error::WebfingerResolveFailed). PR welcome :)

[sgued]

This would solve the issue with regards to the web finger request but not for every other types of error. I only mentioned WebFingerRequest because it is one of the first things I implemented and therefore encountered the issue but I'd be surprised if the other errors never end up "eating" the Object errors either.

Maybe another less "breaking" option would be to always log the errors that don't come from the library itself.

PR welcome

Of course I will do so once we have agreed on a satisfying solution.

[Nutomic]

You can do the same thing for other error types where this makes sense. But for example RequestLimit doesnt have any associated source error, so in that case it doesnt make sense.

[sgued]

Since https://github.com/LemmyNet/activitypub-federation-rust/pull/82 has been merged should I go for Box<dyn Error> rather than anyhow::Error?

[Nutomic]

Yes thats right, or even a more specific error if possible such as reqwest::Error.