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.
I have a library with its own error type, and one variant wraps
http::uri::InvalidUri. I would like that error type to derivePartialEqandEqso unit tests can compare expected and actual errors directly, butInvalidUricurrently prevents that.For example:
Today I can work around this with a manual
PartialEqimplementation in my crate, but the only special case is this wrappedhttperror. The main thing I want is boring unit tests that compare errors normally.Would it be reasonable for
InvalidUriandInvalidUriPartsto implementPartialEqandEq?The small patch would be:
ErrorKindalready implementsPartialEqandEq, 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/Eqseems like a smaller commitment thanClone, but it is still a commitment: users could compare twoInvalidUrivalues 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,
ErrorKindstays 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, andToStrError. I am starting withInvalidUribecause that is the case I ran into, andInvalidUriPartsis just a wrapper around it.