Skip to content

fix: Display for Error shouldn't include source#2199

Merged
seanmonstar merged 1 commit intomasterfrom
error-display-without-source
Mar 22, 2024
Merged

fix: Display for Error shouldn't include source#2199
seanmonstar merged 1 commit intomasterfrom
error-display-without-source

Conversation

@seanmonstar
Copy link
Copy Markdown
Owner

This was supposed to be in 0.12 and forgotten. Getting it into 0.12.1 just a couple days after should be early enough.

@seanmonstar seanmonstar force-pushed the error-display-without-source branch from ddf7016 to 517f9da Compare March 22, 2024 12:31
@seanmonstar seanmonstar force-pushed the error-display-without-source branch from 517f9da to b58a6ca Compare March 22, 2024 12:32
@seanmonstar seanmonstar merged commit 3d78fcb into master Mar 22, 2024
@seanmonstar seanmonstar deleted the error-display-without-source branch March 22, 2024 12:44
legoktm added a commit to freedomofpress/securedrop-client that referenced this pull request May 17, 2024
There's a small change in how reqwest errors are printed (see
<seanmonstar/reqwest#2199>), so handle that in
our code and update one test accordingly.

Import a number of audits and trust markers, a few audits will still be
needed.
legoktm added a commit to freedomofpress/securedrop-client that referenced this pull request May 17, 2024
There's a small change in how reqwest errors are printed (see
<seanmonstar/reqwest#2199>), so handle that in
our code and update one test accordingly.

Import a number of audits and trust markers, a few audits will still be
needed.

Fixes #1985.
ghost pushed a commit to dfinity/sdk that referenced this pull request Jul 15, 2024
Since reqwest 0.12.1 (seanmonstar/reqwest#2199),
fmt::Display for reqwest::Error no longer includes the source.

The whole reason for WrappedReqwestError was to avoid displaying the source
twice. But since the above change, the source isn't displayed at all.

After this change, dfx will once again display the error source(s) for
request::Error, but on separate lines ("Caused by:") like other error sources.
@ghost ghost mentioned this pull request Jul 15, 2024
3 tasks
ghost pushed a commit to dfinity/sdk that referenced this pull request Jul 15, 2024
Since reqwest 0.12.1 (seanmonstar/reqwest#2199),
fmt::Display for reqwest::Error no longer includes the source.

The whole reason for WrappedReqwestError was to avoid displaying the source
twice. But since the above change, the source isn't displayed at all.

After this change, dfx will once again display the error source(s) for
request::Error, but on separate lines ("Caused by:") like other error sources.
wfchandler added a commit to oxidecomputer/oxide.rs that referenced this pull request Oct 9, 2024
reqwest#2199[0] removed the error source from its Display impl, causing
us to lose context on why a request failed.

Manually iterate over error sources and append them to output message to
restore the original behavior. The first source is the intial error that
we have wrapped, so that must be skipped in order to prevent duplicating
the message.

[0] seanmonstar/reqwest#2199
Nutomic pushed a commit to Nutomic/reqwest that referenced this pull request Nov 7, 2024
rikonor pushed a commit to dfinity/sdk that referenced this pull request Feb 3, 2025
Since reqwest 0.12.1 (seanmonstar/reqwest#2199),
fmt::Display for reqwest::Error no longer includes the source.

The whole reason for WrappedReqwestError was to avoid displaying the source
twice. But since the above change, the source isn't displayed at all.

After this change, dfx will once again display the error source(s) for
request::Error, but on separate lines ("Caused by:") like other error sources.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant