hyper: `hyper::Error` prints its first cause in its Display impl, causes duplicate printing in many scenarios

Logging the error often involves crawling the chain.

slog supports a #err modifier that crawls the chain automatically and prints a message like description: source1: source2: source3.

tracing crawls the chain automatically for any field of type dyn Error + 'static that is logged and prints it like name=description, name.sources=[source1, source2, source3].

anyhow::Error and eyre::Report crawl the chain automatically when printed with {:#} or {:?}, the former printing like description: source1: source2: source3 and the latter printing in a multi-line format.

These are just what I know off the top of my head. Printing the whole chain is extremely common when logging errors.

And I don’t see how this is in conflict with what you want. Your rationale only matters for the Display impl, it does not justify the current behavior of source(). You want users who just print the error and not the whole chain to see the immediate cause, I get that. And you can still do that, just fix source() to skip that immediate cause and instead print the next one (if any). With that changed, everyone who prints just the error will see the same behavior as today, and anyone who prints the chain will stop seeing duplicated causes.

Note my last paragraph. The existence of message() as it is today requires me to write my own Error wrapper just to get proper behavior. And it also doesn’t justify the fact that printing the error chain duplicates the source.

To summarize the two reasonable options here:

1. Stop printing the cause in Display. Leave that up to the user to choose to do.
2. Or change source() to skip the immediate cause and return cause.source() instead. Additionally, change message() to return an impl Error + Send + Sync + 'static instead that retains the current cause as its source. That way I can just pass that to my error reporting infrastructure and I will get proper handling of the error chain.

@davidpdrsn I just looked myself as well and ConnectError does appear to be the only one, though is_connect() doesn’t guarantee ConnectError.

In any case, your solution will lose the message “tcp connect error”.

Unfortunately I can’t wrap this error in another error type that formats the ConnectError but skips its first source, unless I want to wrap the hyper::Error in something like struct Wrapper { inner: Arc<hyper::Error>, cause: Option<Wrapper2> } where the second wrapper indirects into the source (the reason being that source() returns a reference and so I can’t construct the wrapper on the spot, which means I need to pre-construct it with access to the error). This is very awkward. It would be easier if hyper::Error::message() didn’t keep a borrow on the error (which it doesn’t even need, it’s actually returning a static string, but the declared types don’t reflect that).

Edit: I suppose I overlooked the idea of just pre-formatting the message into a String and holding onto that, thus allowing us to wrap the hyper::Error for the ConnectError workaround up front without using Arc. Still, this is super awkward.

Edit 2: Oh, even better, my wrapper can just contain another wrapper around the error and access the message through that second wrapper, and that second wrapper can then do the ConnectInfo workaround. Still, super hacky.

Sorry for the confusion, I was in fact advocating for the way hyper currently is. I think the solution is instead that the errors working group and error reporting crates figure out a way to indicate they are going to crawl the source chain and print each out. As I mentioned, I believe it is more common for users to just print the Display of the error and be done with it, so I that’s why I propose it should be the default.

I explored that some in https://github.com/seanmonstar/errors/issues/1. Re-reading that issue, I see it disagrees slightly with my current opinion on what is default. But the rest is still good.

I provide the rationale in this comment here. The short answer is that I believe the more common case is users that are simply debug!("{}", error) printing, and not using one of the fancier error reporting crates. Those users would lose out if the behavior were changed.

The basic requirement … And yet it is. This is unequivocally a bug.

Please give the benefit of the doubt. We all want what’s best, even when what’s best isn’t crystal clear yet.