Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Clone, Eq and PartialEq for Error. #218

Merged
merged 2 commits into from
Nov 19, 2021
Merged

Conversation

qwandor
Copy link
Contributor

@qwandor qwandor commented Nov 16, 2021

This fixes #217.

@qwandor qwandor changed the title Implement Eq and PartialEq for Error. Implement Clone, Eq and PartialEq for Error. Nov 16, 2021
@Keats
Copy link
Owner

Keats commented Nov 17, 2021

Can I just get more details on when you would use eq on the error instead of on the error kind? And when is PartialEq not enough?

@qwandor
Copy link
Contributor Author

qwandor commented Nov 17, 2021

I'd like to be able to #[derive(Clone, Eq, PartialEq)] my own error types which wrap jsonwebtoken::Error. In particular I have something like this:

#[error(Debug, Clone, Eq, PartialEq, thiserror::Error)]
pub enum MyError {
    #[error("JWT error: {0}")]
    Jwt(jsonwebtokenerror::Error),
    #[error("Some other error...")]
    SomeOtherError(...),
    ...
}

For the derive macros to work, the various wrapped error types need to implement Clone and Eq themselves. All the error types provided by other libraries I use do, it's just jsonwebtoken which doesn't.

As for PartialEq vs. Eq, I guess PartialEq is 'enough' in that it implements all the methods I need, but there's no reason not to implement Eq and it's correct to do so. It's just a marker trait which says that your PartialEq implementation is reflexive, symmetric and transitive, which it already is.

serde_json::Error doesn't implement Clone, so wrapped it in an Arc.
@Keats Keats merged commit 2c89e18 into Keats:next Nov 19, 2021
Keats pushed a commit that referenced this pull request Feb 2, 2022
* Implement Eq and PartialEq for Error.

* Implement Clone for Error.

serde_json::Error doesn't implement Clone, so wrapped it in an Arc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants