fix: display reqwest::Error source (#3832)

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.

Author: Eric Swanson

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 # UNRELEASED
 
+# fix: display error cause of some http-related errors
+
+Some commands that download http resources, for example `dfx extension install`, will
+once again display any error cause.
+
 # 0.22.0
 
 ### asset uploads: retry some HTTP errors returned by the replica

src/dfx-core/src/error/extension.rs

@@ -1,5 +1,4 @@
 #![allow(dead_code)]
-use crate::error::reqwest::WrappedReqwestError;
 use crate::error::structured_file::StructuredFileError;
 use thiserror::Error;
 
@@ -84,7 +83,7 @@ pub enum NewExtensionManagerError {
 #[derive(Error, Debug)]
 pub enum DownloadAndInstallExtensionToTempdirError {
     #[error(transparent)]
-    ExtensionDownloadFailed(WrappedReqwestError),
+    ExtensionDownloadFailed(reqwest::Error),
 
     #[error("Cannot get extensions directory")]
     EnsureExtensionDirExistsFailed(#[source] crate::error::fs::FsError),
@@ -141,10 +140,10 @@ pub enum GetDependenciesError {
     ParseUrl(#[from] url::ParseError),
 
     #[error(transparent)]
-    Get(WrappedReqwestError),
+    Get(reqwest::Error),
 
     #[error(transparent)]
-    ParseJson(WrappedReqwestError),
+    ParseJson(reqwest::Error),
 }
 
 #[derive(Error, Debug)]

src/dfx-core/src/error/reqwest.rs

@@ -1,28 +1,16 @@
-use crate::http::retryable::Retryable;
 use reqwest::StatusCode;
-use thiserror::Error;
 
-// reqwest::Error's fmt::Display appends the error descriptions of all sources.
-// For this reason, it is not marked as #[source] here, so that we don't
-// display the error descriptions of all sources repeatedly.
-#[derive(Error, Debug)]
-#[error("{}", .0)]
-pub struct WrappedReqwestError(pub reqwest::Error);
-
-impl Retryable for WrappedReqwestError {
-    fn is_retryable(&self) -> bool {
-        let err = &self.0;
-        err.is_timeout()
-            || err.is_connect()
-            || matches!(
-                err.status(),
-                Some(
-                    StatusCode::INTERNAL_SERVER_ERROR
-                        | StatusCode::BAD_GATEWAY
-                        | StatusCode::SERVICE_UNAVAILABLE
-                        | StatusCode::GATEWAY_TIMEOUT
-                        | StatusCode::TOO_MANY_REQUESTS
-                )
+pub fn is_retryable(err: &reqwest::Error) -> bool {
+    err.is_timeout()
+        || err.is_connect()
+        || matches!(
+            err.status(),
+            Some(
+                StatusCode::INTERNAL_SERVER_ERROR
+                    | StatusCode::BAD_GATEWAY
+                    | StatusCode::SERVICE_UNAVAILABLE
+                    | StatusCode::GATEWAY_TIMEOUT
+                    | StatusCode::TOO_MANY_REQUESTS
             )
-    }
+        )
 }

src/dfx-core/src/extension/manager/install.rs

@@ -3,7 +3,6 @@ use crate::error::extension::{
     GetExtensionArchiveNameError, GetExtensionDownloadUrlError, GetHighestCompatibleVersionError,
     InstallExtensionError,
 };
-use crate::error::reqwest::WrappedReqwestError;
 use crate::extension::{
     manager::ExtensionManager, manifest::ExtensionDependencies, url::ExtensionJsonUrl,
 };
@@ -96,11 +95,10 @@ impl ExtensionManager {
             .await
             .map_err(DownloadAndInstallExtensionToTempdirError::ExtensionDownloadFailed)?;
 
-        let bytes = response.bytes().await.map_err(|e| {
-            DownloadAndInstallExtensionToTempdirError::ExtensionDownloadFailed(WrappedReqwestError(
-                e,
-            ))
-        })?;
+        let bytes = response
+            .bytes()
+            .await
+            .map_err(DownloadAndInstallExtensionToTempdirError::ExtensionDownloadFailed)?;
 
         crate::fs::composite::ensure_dir_exists(&self.dir)
             .map_err(DownloadAndInstallExtensionToTempdirError::EnsureExtensionDirExistsFailed)?;

src/dfx-core/src/extension/manifest/dependencies.rs

@@ -1,5 +1,4 @@
 use crate::error::extension::{DfxOnlySupportedDependency, GetDependenciesError};
-use crate::error::reqwest::WrappedReqwestError;
 use crate::extension::url::ExtensionJsonUrl;
 use crate::http::get::get_with_retries;
 use crate::json::structure::VersionReqWithJsonSchema;
@@ -36,9 +35,7 @@ impl ExtensionDependencies {
             .await
             .map_err(GetDependenciesError::Get)?;
 
-        resp.json()
-            .await
-            .map_err(|e| GetDependenciesError::ParseJson(WrappedReqwestError(e)))
+        resp.json().await.map_err(GetDependenciesError::ParseJson)
     }
 
     pub fn find_highest_compatible_version(

src/dfx-core/src/http/get.rs

@@ -1,5 +1,3 @@
-use crate::error::reqwest::WrappedReqwestError;
-use crate::http::retryable::Retryable;
 use backoff::exponential::ExponentialBackoff;
 use backoff::future::retry;
 use backoff::SystemClock;
@@ -9,15 +7,14 @@ use url::Url;
 pub async fn get_with_retries(
     url: Url,
     retry_policy: ExponentialBackoff<SystemClock>,
-) -> Result<Response, WrappedReqwestError> {
+) -> Result<Response, reqwest::Error> {
     let operation = || async {
         let response = reqwest::get(url.clone())
             .await
-            .and_then(|resp| resp.error_for_status())
-            .map_err(WrappedReqwestError);
+            .and_then(|resp| resp.error_for_status());
         match response {
             Ok(doc) => Ok(doc),
-            Err(e) if e.is_retryable() => Err(backoff::Error::transient(e)),
+            Err(e) if crate::error::reqwest::is_retryable(&e) => Err(backoff::Error::transient(e)),
             Err(e) => Err(backoff::Error::permanent(e)),
         }
     };

src/dfx-core/src/http/mod.rs

@@ -1,2 +1 @@
 pub mod get;
-pub mod retryable;