-
Notifications
You must be signed in to change notification settings - Fork 74
Expose reqwest's inner error in the fetch function
#424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We were converting the `reqwest::Error` type directly to `span::Error`
and the rendering chain stopped at that. This ignored the
inner (source) error which contained the most useful information to
figure out what went wrong.
This gets us from:
Error: error: failed to fetch: error: error decoding response body
to:
Error: error: failed to fetch: error: error decoding response body: missing field `discord` at line 16049 column 1
|
You can reproduce this on the following commit: It will return this error: Error: error: failed to fetch: error: error decoding response body
--> /home/shadower/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/spanned-0.6.1/src/error.rs:130:23
|
130 | span: Span::here(),
| ^^^^^^^^^^^^^
|
--> crates/rust-project-goals/src/team.rs:17:27
|
17 | Err(e) => Err(Error::str(format!("failed to fetch: {e:?}"))),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|That message tells you nothing about what's wrong. With this PR, we get the following instead: Error: error: failed to fetch: error: error decoding response body: missing field `discord` at line 16049 column 1
--> crates/rust-project-goals/src/team.rs:133:13
|
133 | spanned::Error::str(format!("{e}{source_message}"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
--> crates/rust-project-goals/src/team.rs:17:27
|
17 | Err(e) => Err(Error::str(format!("failed to fetch: {e:?}"))),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|You can see that the first line tells you a bit more about the issue (there's a missing field in the JSON it's trying to convert) and give you slightly better stacktrace (pointing you at the |
|
I don't love the spans here, they seem to add more noise than clarity. I also feel constructing a Maybe there's a way to make sure |
The code is shorter and less hacky. The end-user message looks much better too.
|
Ok, I think this is much better both in terms of the code and the message displayed to the user: $ cargo rpg updates 2025h2 --output-file out
Error: error: failed to fetch: error: missing field `discord` at line 16049 column 1
--> crates/rust-project-goals/src/team.rs:129:35
|
129 | .map(|json_error| spanned::Error::str(format!("{json_error}")))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
--> crates/rust-project-goals/src/team.rs:17:27
|
17 | Err(e) => Err(Error::str(format!("failed to fetch: {e:?}"))),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
|
Okay, I'm much happier with this version. Maybe there's a better way to handle this, but we just create a Happy to merge this unless there are objections. |
|
Nice, the output error looks much better. |
We were converting the
reqwest::Errortype directly tospan::Errorand the rendering chain stopped at that. This ignored the inner (source) error which contained the most useful information to figure out what went wrong.This gets us from:
to: