Repository navigation
Implement PartialEq/Eq for InvalidUri? #849
Description
I have a library with its own error type, and one variant wraps http::uri::InvalidUri. I would like that error type to derive PartialEq and Eq so unit tests can compare expected and actual errors directly, but InvalidUri currently prevents that.
For example:
#[derive(Debug, PartialEq, Eq)]
enum Error {
InvalidUri(http::uri::InvalidUri),
}Today I can work around this with a manual PartialEq implementation in my crate, but the only special case is this wrapped http error. The main thing I want is boring unit tests that compare errors normally.
Would it be reasonable for InvalidUri and InvalidUriParts to implement PartialEq and Eq?
The small patch would be:
#[derive(Debug, Eq, PartialEq)]
pub struct InvalidUri(ErrorKind);
#[derive(Debug, Eq, PartialEq)]
pub struct InvalidUriParts(InvalidUri);ErrorKind already implements PartialEq and Eq, and this would keep it private. I am not trying to expose or match on the URI error kind.
I also realize this is still a public contract. I saw the previous discussions around error trait impls, especially #128 and #204 around Clone, and I understand the concern that trait impls can constrain future error internals. PartialEq/Eq seems like a smaller commitment than Clone, but it is still a commitment: users could compare two InvalidUri values and observe whether they are considered the same broad error.
My take is that this tradeoff is reasonable for these URI errors because the comparison can stay coarse, ErrorKind stays private, and the concrete downstream benefit is allowing library error types to derive equality for tests. I may be underestimating the compatibility cost, though, so I would appreciate maintainer feedback.
I am opening this as an issue even though the possible patch is small, because the useful part is the API decision. If the answer is "no, this is too much public contract for URI errors," that decision would be useful to have recorded for future searches.
One scope question: the same motivation may apply to other opaque conversion errors in this crate, such as InvalidMethod, InvalidStatusCode, InvalidHeaderName, InvalidHeaderValue, and ToStrError. I am starting with InvalidUri because that is the case I ran into, and InvalidUriParts is just a wrapper around it.
Activity
seanmonstar commentedon Jul 6, 2026
Just curious, what about
assert_matches!(), or oldassert!(matches!()).joshka commentedon Jul 6, 2026
Those both work fine, but the error itself is also nested. So you end up having to propagate the workaround everywhere rather than just assert_eq!
It's not a huge blocker in terms of this code can't be written. Just a thing that feels like it should be supported to avoid undifferentiated code across all places that embed InvalidUri in other errors. the exact code is:
https://github.com/joshka/webfinger-rs/blob/7e3b90b432855deca8408bfd11fab78a54abc0e2/webfinger-rs/src/types/resource.rs#L38
joshka commentedon Jul 12, 2026
@seanmonstar is there any push back on this? Obv. the impl is extremely simple, and I've done as much due diligence as possible in understanding the underlying background here. This sort of feels like it's either LGTM-shipit (#850) or a message saying that I didn't think about this unknown unknown...
Even if "I'm not sure, please do some work and dig into x for me" would set a direction here.
seanmonstar commentedon Jul 13, 2026
Hm, I see what you're looking for. I did a little more thinking, and read up on where this has come up before. I still lean towards not merging.
I think in general, Rust
Errors shouldn't implementPartialEq. They might have internal details that don't make sense to compare. Like positional data, or a source chain. Even if we don't have those details yet, committing to a public API can restrict us from internal refactoring.(A public, exhaustive, flat
enumcould choose differently. But those look more like akind(). These are opaque error structs.)Similar to in that bincode issue, if testing is the main reason, I think
matches!,kind(), oris_*()patterns fit better.joshka commentedon Jul 14, 2026
Thanks for considering this. I looked further into the concern that implementing equality could constrain future internal changes.
One relevant standard-library precedent is
TryReserveError. It originally derivedPartialEqandEq, and Rust later deliberately changed it into an opaque struct because the private error details were still unsettled (rust-lang/rust#87408). The stabilization proposal explicitly left open both adding support for future allocator details and reducing the error to a unit struct, while the public type was stabilized with its equality implementations intact (rust-lang/rust#87993).I did not find an explicit discussion of equality in those reviews, so I would not claim that this settles the question for
InvalidUri. It does show that std has stabilized equality for an opaque error whose private representation was deliberately left open to substantial change. Together withUtf8Errorimplementing equality despite carrying positional information, that makes me less concerned that these possible future details are necessarily blocking.My practical motivation remains that
InvalidUriis a leaf error from a foundational crate. Including it in a downstream error enum prevents that enum from derivingPartialEq, moving the implementation work into each downstream crate. My take is that this compositional benefit outweighs the limited compatibility constraint here, particularly if equality is defined without exposing the private kind or making future diagnostic details significant.That said, the downstream workaround is reasonable, and I understand the preference not to add the contract. I’m happy to leave this undone and close the PR.