Tidy formatting of Reqwest errors (#4336)

## Issue Addressed

NA

## Proposed Changes

Implements the `PrettyReqwestError` to wrap a `reqwest::Error` and give nicer `Debug` formatting. It also wraps the `Url` component in a `SensitiveUrl` to avoid leaking sensitive info in logs.

### Before

```
Reqwest(reqwest::Error { kind: Request, url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(9999), path: "/eth/v1/node/version", query: None, fragment: None }, source: hyper::Error(Connect, ConnectError("tcp connect error", Os { code: 61, kind: ConnectionRefused, message: "Connection refused" })) })
```

### After

```
HttpClient(url: http://localhost:9999/, kind: request, detail: error trying to connect: tcp connect error: Connection refused (os error 61))
```

## Additional Info

I've also renamed the `Reqwest` error enum variants to `HttpClient`, to give people a better chance at knowing what's going on. Reqwest is pretty odd and looks like a typo.

I've implemented it in the `eth2` and `execution_layer` crates. This should affect most logs in the VC and EE-related ones in the BN.

I think the last crate that could benefit from the is the `beacon_node/eth1` crate. I haven't updated it in this PR since its error type is not so amenable to it (everything goes into a `String`). I don't have a whole lot of time to jig around with that at the moment and I feel that this PR as it stands is a significant enough improvement to merge on its own. Leaving it as-is is fine for the time being and we can always come back for it later (or implement in-protocol deposits!).

Cargo.lock

@@ -2292,6 +2292,8 @@ dependencies = [
  "libsecp256k1",
  "lighthouse_network",
  "mediatype",
+ "mime",
+ "pretty_reqwest_error",
  "procinfo",
  "proto_array",
  "psutil",
@@ -2302,6 +2304,7 @@ dependencies = [
  "serde_json",
  "slashing_protection",
  "store",
+ "tokio",
  "types",
 ]
 
@@ -2742,6 +2745,7 @@ dependencies = [
  "lru 0.7.8",
  "mev-rs",
  "parking_lot 0.12.1",
+ "pretty_reqwest_error",
  "rand 0.8.5",
  "reqwest",
  "sensitive_url",
@@ -6204,6 +6208,14 @@ dependencies = [
  "vcpkg",
 ]
 
+[[package]]
+name = "pretty_reqwest_error"
+version = "0.1.0"
+dependencies = [
+ "reqwest",
+ "sensitive_url",
+]
+
 [[package]]
 name = "prettyplease"
 version = "0.1.25"

Cargo.toml

@@ -35,6 +35,7 @@ members = [
     "common/lru_cache",
     "common/malloc_utils",
     "common/oneshot_broadcast",
+    "common/pretty_reqwest_error",
     "common/sensitive_url",
     "common/slot_clock",
     "common/system_health",

beacon_node/builder_client/src/lib.rs

@@ -72,7 +72,7 @@ impl BuilderHttpClient {
             .await?
             .json()
             .await
-            .map_err(Error::Reqwest)
+            .map_err(Into::into)
     }
 
     /// Perform a HTTP GET request, returning the `Response` for further processing.
@@ -85,7 +85,7 @@ impl BuilderHttpClient {
         if let Some(timeout) = timeout {
             builder = builder.timeout(timeout);
         }
-        let response = builder.send().await.map_err(Error::Reqwest)?;
+        let response = builder.send().await.map_err(Error::from)?;
         ok_or_error(response).await
     }
 
@@ -114,7 +114,7 @@ impl BuilderHttpClient {
         if let Some(timeout) = timeout {
             builder = builder.timeout(timeout);
         }
-        let response = builder.json(body).send().await.map_err(Error::Reqwest)?;
+        let response = builder.json(body).send().await.map_err(Error::from)?;
         ok_or_error(response).await
     }
 

beacon_node/execution_layer/Cargo.toml

@@ -50,3 +50,4 @@ keccak-hash = "0.10.0"
 hash256-std-hasher = "0.15.2"
 triehash = "0.8.4"
 hash-db = "0.15.2"
+pretty_reqwest_error = { path = "../../common/pretty_reqwest_error" }

beacon_node/execution_layer/src/engine_api.rs

@@ -10,6 +10,7 @@ pub use ethers_core::types::Transaction;
 use ethers_core::utils::rlp::{self, Decodable, Rlp};
 use http::deposit_methods::RpcError;
 pub use json_structures::{JsonWithdrawal, TransitionConfigurationV1};
+use pretty_reqwest_error::PrettyReqwestError;
 use reqwest::StatusCode;
 use serde::{Deserialize, Serialize};
 use std::convert::TryFrom;
@@ -32,7 +33,7 @@ pub type PayloadId = [u8; 8];
 
 #[derive(Debug)]
 pub enum Error {
-    Reqwest(reqwest::Error),
+    HttpClient(PrettyReqwestError),
     Auth(auth::Error),
     BadResponse(String),
     RequestFailed(String),
@@ -67,7 +68,7 @@ impl From<reqwest::Error> for Error {
         ) {
             Error::Auth(auth::Error::InvalidToken)
         } else {
-            Error::Reqwest(e)
+            Error::HttpClient(e.into())
         }
     }
 }

common/eth2/Cargo.toml

@@ -27,6 +27,11 @@ futures = "0.3.8"
 store = { path = "../../beacon_node/store", optional = true }
 slashing_protection = { path = "../../validator_client/slashing_protection", optional = true }
 mediatype = "0.19.13"
+mime = "0.3.16"
+pretty_reqwest_error = { path = "../../common/pretty_reqwest_error" }
+
+[dev-dependencies]
+tokio = { version = "1.14.0", features = ["full"] }
 
 [target.'cfg(target_os = "linux")'.dependencies]
 psutil = { version = "3.2.2", optional = true }

common/eth2/src/lib.rs

@@ -19,6 +19,7 @@ use self::types::{Error as ResponseError, *};
 use futures::Stream;
 use futures_util::StreamExt;
 use lighthouse_network::PeerId;
+use pretty_reqwest_error::PrettyReqwestError;
 pub use reqwest;
 use reqwest::{IntoUrl, RequestBuilder, Response};
 pub use reqwest::{StatusCode, Url};
@@ -39,7 +40,7 @@ pub const CONSENSUS_VERSION_HEADER: &str = "Eth-Consensus-Version";
 #[derive(Debug)]
 pub enum Error {
     /// The `reqwest` client raised an error.
-    Reqwest(reqwest::Error),
+    HttpClient(PrettyReqwestError),
     /// The server returned an error message where the body was able to be parsed.
     ServerMessage(ErrorMessage),
     /// The server returned an error message with an array of errors.
@@ -70,7 +71,7 @@ pub enum Error {
 
 impl From<reqwest::Error> for Error {
     fn from(error: reqwest::Error) -> Self {
-        Error::Reqwest(error)
+        Error::HttpClient(error.into())
     }
 }
 
@@ -78,7 +79,7 @@ impl Error {
     /// If the error has a HTTP status code, return it.
     pub fn status(&self) -> Option<StatusCode> {
         match self {
-            Error::Reqwest(error) => error.status(),
+            Error::HttpClient(error) => error.inner().status(),
             Error::ServerMessage(msg) => StatusCode::try_from(msg.code).ok(),
             Error::ServerIndexedMessage(msg) => StatusCode::try_from(msg.code).ok(),
             Error::StatusCode(status) => Some(*status),
@@ -278,7 +279,7 @@ impl BeaconNodeHttpClient {
             .await?
             .json()
             .await
-            .map_err(Error::Reqwest)
+            .map_err(Into::into)
     }
 
     /// Perform a HTTP POST request with a custom timeout.
@@ -303,7 +304,7 @@ impl BeaconNodeHttpClient {
             .await?
             .json()
             .await
-            .map_err(Error::Reqwest)
+            .map_err(Error::from)
     }
 
     /// Generic POST function supporting arbitrary responses and timeouts.
@@ -1645,7 +1646,7 @@ impl BeaconNodeHttpClient {
             .bytes_stream()
             .map(|next| match next {
                 Ok(bytes) => EventKind::from_sse_bytes(bytes.as_ref()),
-                Err(e) => Err(Error::Reqwest(e)),
+                Err(e) => Err(Error::HttpClient(e.into())),
             }))
     }
 

common/eth2/src/lighthouse.rs

@@ -364,12 +364,12 @@ pub struct DatabaseInfo {
 impl BeaconNodeHttpClient {
     /// Perform a HTTP GET request, returning `None` on a 404 error.
     async fn get_bytes_opt<U: IntoUrl>(&self, url: U) -> Result<Option<Vec<u8>>, Error> {
-        let response = self.client.get(url).send().await.map_err(Error::Reqwest)?;
+        let response = self.client.get(url).send().await.map_err(Error::from)?;
         match ok_or_error(response).await {
             Ok(resp) => Ok(Some(
                 resp.bytes()
                     .await
-                    .map_err(Error::Reqwest)?
+                    .map_err(Error::from)?
                     .into_iter()
                     .collect::<Vec<_>>(),
             )),

common/eth2/src/lighthouse_vc/http_client.rs

@@ -170,7 +170,7 @@ impl ValidatorClientHttpClient {
             .map_err(|_| Error::InvalidSignatureHeader)?
             .to_string();
 
-        let body = response.bytes().await.map_err(Error::Reqwest)?;
+        let body = response.bytes().await.map_err(Error::from)?;
 
         let message =
             Message::parse_slice(digest(&SHA256, &body).as_ref()).expect("sha256 is 32 bytes");
@@ -222,7 +222,7 @@ impl ValidatorClientHttpClient {
             .headers(self.headers()?)
             .send()
             .await
-            .map_err(Error::Reqwest)?;
+            .map_err(Error::from)?;
         ok_or_error(response).await
     }
 
@@ -236,7 +236,7 @@ impl ValidatorClientHttpClient {
             .await?
             .json()
             .await
-            .map_err(Error::Reqwest)
+            .map_err(Error::from)
     }
 
     /// Perform a HTTP GET request, returning `None` on a 404 error.
@@ -266,7 +266,7 @@ impl ValidatorClientHttpClient {
             .json(body)
             .send()
             .await
-            .map_err(Error::Reqwest)?;
+            .map_err(Error::from)?;
         ok_or_error(response).await
     }
 
@@ -297,7 +297,7 @@ impl ValidatorClientHttpClient {
             .json(body)
             .send()
             .await
-            .map_err(Error::Reqwest)?;
+            .map_err(Error::from)?;
         let response = ok_or_error(response).await?;
         self.signed_body(response).await?;
         Ok(())
@@ -316,7 +316,7 @@ impl ValidatorClientHttpClient {
             .json(body)
             .send()
             .await
-            .map_err(Error::Reqwest)?;
+            .map_err(Error::from)?;
         ok_or_error(response).await
     }
 

common/pretty_reqwest_error/Cargo.toml

@@ -0,0 +1,10 @@
+[package]
+name = "pretty_reqwest_error"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+reqwest = { version = "0.11.0", features = ["json","stream"] }
+sensitive_url = { path = "../sensitive_url" }

common/pretty_reqwest_error/src/lib.rs

@@ -0,0 +1,62 @@
+use sensitive_url::SensitiveUrl;
+use std::error::Error as StdError;
+use std::fmt;
+
+pub struct PrettyReqwestError(reqwest::Error);
+
+impl PrettyReqwestError {
+    pub fn inner(&self) -> &reqwest::Error {
+        &self.0
+    }
+}
+
+impl fmt::Debug for PrettyReqwestError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if let Some(url) = self.0.url() {
+            if let Ok(url) = SensitiveUrl::new(url.clone()) {
+                write!(f, "url: {}", url)?;
+            } else {
+                write!(f, "url: unable_to_parse")?;
+            };
+        }
+
+        let kind = if self.0.is_builder() {
+            "builder"
+        } else if self.0.is_redirect() {
+            "redirect"
+        } else if self.0.is_status() {
+            "status"
+        } else if self.0.is_timeout() {
+            "timeout"
+        } else if self.0.is_request() {
+            "request"
+        } else if self.0.is_connect() {
+            "connect"
+        } else if self.0.is_body() {
+            "body"
+        } else if self.0.is_decode() {
+            "decode"
+        } else {
+            "unknown"
+        };
+        write!(f, ", kind: {}", kind)?;
+
+        if let Some(status) = self.0.status() {
+            write!(f, ", status_code: {}", status)?;
+        }
+
+        if let Some(ref source) = self.0.source() {
+            write!(f, ", detail: {}", source)?;
+        } else {
+            write!(f, ", source: unknown")?;
+        }
+
+        Ok(())
+    }
+}
+
+impl From<reqwest::Error> for PrettyReqwestError {
+    fn from(inner: reqwest::Error) -> Self {
+        Self(inner)
+    }
+}

common/sensitive_url/src/lib.rs

@@ -75,7 +75,7 @@ impl SensitiveUrl {
         SensitiveUrl::new(surl)
     }
 
-    fn new(full: Url) -> Result<Self, SensitiveError> {
+    pub fn new(full: Url) -> Result<Self, SensitiveError> {
         let mut redacted = full.clone();
         redacted
             .path_segments_mut()