fix(error messages): fix error messages when downloading the config

[marcelo-gonzalez]

This was missed, but #5967 did
not interact very well with enum FileDownloadError, so this is an
attempt to fix that, and present better error messages.

The reason for getting rid of the #[from] in the HttpError variant is
that it doesn't really play nicely with the anyhow {:#} formatter and
gives us ugliness like:

ERROR neard::cli: Failed to initialize configs: Failed to download the config file from http://localhost:8002/config.json: error trying to connect: tcp connect error: Connection refused (os error 111): tcp connect error: Connection refused (os error 111): Connection refused (os error 111)

Test Plan:
Of course check that downloading the file still works, but also inject
errors and make sure the error messages look ok. For reviewer
convenience, error msgs corresponding to the variants below (except
for RemoveTemporaryFileError. Didn't get to that one... but I think
it's probably good)

HttpError:

ERROR neard::cli: Failed to initialize configs: Failed to download the config file from http://localhost:8002/config.json: error trying to connect: tcp connect error: Connection refused (os error 111)

OpenError:

ERROR neard::cli: Failed to initialize configs: Failed to download the config file from http://localhost:8002/config.json: Failed to open temporary file: No such file or directory (os error 2) at path "/asdf/.tmp2g9Pqe"

WriteError:

ERROR neard::cli: Failed to initialize configs: Failed to download the config file from http://localhost:8000/config.json: Failed to write to temporary file at /tmp/asdfa/.tmpNdy4iL: Bad file descriptor (os error 9)

RenameEror:

ERROR neard::cli: Failed to initialize configs: Failed to download the config file from http://localhost:8000/config.json: Failed to rename temporary file /tmp/asdfa/.tmpRvBBEn to /tmp/asdfa/config.json : Permission denied (os error 13)

UriError:

ERROR neard::cli: Failed to initialize configs: Failed to download the config file from http://localhost:8002:3:a:b/config.json: Invalid URI: invalid authority

Issue: #5485

[nagisa]

This should be #[error(transparent)] I think.

---

Pre-existing, but in general thiserror messages should not format in the cause error's description into its own. Instead #[source] mechanism should be utilized to specify the causation chain. This is how the ecosystem expects the errors to be, making it least likely to get duplicated output like you had described in the PR description. I have, in the past, also written a block post on error handling. While this is not directly applicable to this discussion, it kind of demonstrates what a conventional thiserror use looks like.

[marcelo-gonzalez]

Ah yeah, I actually tried this originally but it gives the same repition of info. Looks like something wrong with hyper itself?

println!("orig {}", &e);
let mut source = e.source();

while let Some(err) = source {
  println!("source {}", err);
  source = err.source();
}

this prints out:

orig error trying to connect: tcp connect error: Connection refused (os error 111)
source tcp connect error: Connection refused (os error 111)
source Connection refused (os error 111)

So just using "{0}" is the best workaround for that I can think of here. But as for the others, using #[source] instead of formatting it explicitly seems way better yeah, I like the blog post! updated the other ones to use #[source] now, and checked that the output is still good. (Last one is a little tricky since there are really two sources, but we can just pick the first one I guess)

[nagisa]

I see. I think it might be a good idea to report this to hyper.

[nagisa]

hyperium/hyper#2732