Skip to content

Implement Clone, Eq and PartialEq for Error.#218

Merged
Keats merged 2 commits into
Keats:nextfrom
qwandor:traits
Nov 19, 2021
Merged

Implement Clone, Eq and PartialEq for Error.#218
Keats merged 2 commits into
Keats:nextfrom
qwandor:traits

Conversation

@qwandor

@qwandor qwandor commented Nov 16, 2021

Copy link
Copy Markdown
Contributor

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

Keats commented Nov 17, 2021

Copy link
Copy Markdown
Owner

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

qwandor commented Nov 17, 2021

Copy link
Copy Markdown
Contributor Author

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