Extracting the Error impl from the monolith eth2 (#7878)

Currently the `eth2` crate lib file is a large monolith of almost 3000 lines of code. As part of the bosun migration we are trying to increase code readability and modularity in the lighthouse crates initially, which then can be transferred to bosun


Co-Authored-By: hopinheimer <knmanas6@gmail.com>

Co-Authored-By: hopinheimer <48147533+hopinheimer@users.noreply.github.com>

common/eth2/src/error.rs

@@ -0,0 +1,165 @@
+//! Centralized error handling for eth2 API clients
+//!
+//! This module consolidates all error types, response processing,
+//! and recovery logic for both beacon node and validator client APIs.
+
+use pretty_reqwest_error::PrettyReqwestError;
+use reqwest::{Response, StatusCode};
+use sensitive_url::SensitiveUrl;
+use serde::{Deserialize, Serialize};
+use std::{fmt, path::PathBuf};
+
+/// Main error type for eth2 API clients
+#[derive(Debug)]
+pub enum Error {
+    /// The `reqwest` client raised an error.
+    HttpClient(PrettyReqwestError),
+    /// The `reqwest_eventsource` client raised an error.
+    SseClient(Box<reqwest_eventsource::Error>),
+    /// 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.
+    ServerIndexedMessage(IndexedErrorMessage),
+    /// The server returned an error message where the body was unable to be parsed.
+    StatusCode(StatusCode),
+    /// The supplied URL is badly formatted. It should look something like `http://127.0.0.1:5052`.
+    InvalidUrl(SensitiveUrl),
+    /// The supplied validator client secret is invalid.
+    InvalidSecret(String),
+    /// The server returned a response with an invalid signature. It may be an impostor.
+    InvalidSignatureHeader,
+    /// The server returned a response without a signature header. It may be an impostor.
+    MissingSignatureHeader,
+    /// The server returned an invalid JSON response.
+    InvalidJson(serde_json::Error),
+    /// The server returned an invalid server-sent event.
+    InvalidServerSentEvent(String),
+    /// The server sent invalid response headers.
+    InvalidHeaders(String),
+    /// The server returned an invalid SSZ response.
+    InvalidSsz(ssz::DecodeError),
+    /// An I/O error occurred while loading an API token from disk.
+    TokenReadError(PathBuf, std::io::Error),
+    /// The client has been configured without a server pubkey, but requires one for this request.
+    NoServerPubkey,
+    /// The client has been configured without an API token, but requires one for this request.
+    NoToken,
+}
+
+/// An API error serializable to JSON.
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct ErrorMessage {
+    pub code: u16,
+    pub message: String,
+    #[serde(default)]
+    pub stacktraces: Vec<String>,
+}
+
+/// An indexed API error serializable to JSON.
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct IndexedErrorMessage {
+    pub code: u16,
+    pub message: String,
+    pub failures: Vec<Failure>,
+}
+
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+pub struct Failure {
+    pub index: u64,
+    pub message: String,
+}
+
+impl Failure {
+    pub fn new(index: usize, message: String) -> Self {
+        Self {
+            index: index as u64,
+            message,
+        }
+    }
+}
+
+/// Server error response variants
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(untagged)]
+pub enum ResponseError {
+    Indexed(IndexedErrorMessage),
+    Message(ErrorMessage),
+}
+
+impl Error {
+    /// If the error has a HTTP status code, return it.
+    pub fn status(&self) -> Option<StatusCode> {
+        match self {
+            Error::HttpClient(error) => error.inner().status(),
+            Error::SseClient(error) => {
+                if let reqwest_eventsource::Error::InvalidStatusCode(status, _) = error.as_ref() {
+                    Some(*status)
+                } else {
+                    None
+                }
+            }
+            Error::ServerMessage(msg) => StatusCode::try_from(msg.code).ok(),
+            Error::ServerIndexedMessage(msg) => StatusCode::try_from(msg.code).ok(),
+            Error::StatusCode(status) => Some(*status),
+            Error::InvalidUrl(_) => None,
+            Error::InvalidSecret(_) => None,
+            Error::InvalidSignatureHeader => None,
+            Error::MissingSignatureHeader => None,
+            Error::InvalidJson(_) => None,
+            Error::InvalidSsz(_) => None,
+            Error::InvalidServerSentEvent(_) => None,
+            Error::InvalidHeaders(_) => None,
+            Error::TokenReadError(..) => None,
+            Error::NoServerPubkey | Error::NoToken => None,
+        }
+    }
+}
+
+impl From<reqwest::Error> for Error {
+    fn from(error: reqwest::Error) -> Self {
+        Error::HttpClient(error.into())
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:?}", self)
+    }
+}
+
+/// Returns `Ok(response)` if the response is a `200 OK`, `202 ACCEPTED`, or `204 NO_CONTENT`
+/// Otherwise, creates an appropriate error message.
+pub async fn ok_or_error(response: Response) -> Result<Response, Error> {
+    let status = response.status();
+
+    if status == StatusCode::OK
+        || status == StatusCode::ACCEPTED
+        || status == StatusCode::NO_CONTENT
+    {
+        Ok(response)
+    } else if let Ok(message) = response.json::<ResponseError>().await {
+        match message {
+            ResponseError::Message(message) => Err(Error::ServerMessage(message)),
+            ResponseError::Indexed(indexed) => Err(Error::ServerIndexedMessage(indexed)),
+        }
+    } else {
+        Err(Error::StatusCode(status))
+    }
+}
+
+/// Returns `Ok(response)` if the response is a success (2xx) response. Otherwise, creates an
+/// appropriate error message.
+pub async fn success_or_error(response: Response) -> Result<Response, Error> {
+    let status = response.status();
+
+    if status.is_success() {
+        Ok(response)
+    } else if let Ok(message) = response.json().await {
+        match message {
+            ResponseError::Message(message) => Err(Error::ServerMessage(message)),
+            ResponseError::Indexed(indexed) => Err(Error::ServerIndexedMessage(indexed)),
+        }
+    } else {
+        Err(Error::StatusCode(status))
+    }
+}

common/eth2/src/lib.rs

@@ -7,6 +7,7 @@
 //! Eventually it would be ideal to publish this crate on crates.io, however we have some local
 //! dependencies preventing this presently.
 
+pub mod error;
 #[cfg(feature = "lighthouse")]
 pub mod lighthouse;
 #[cfg(feature = "lighthouse")]
@@ -14,14 +15,14 @@ pub mod lighthouse_vc;
 pub mod mixin;
 pub mod types;
 
+pub use self::error::{Error, ok_or_error, success_or_error};
 use self::mixin::{RequestAccept, ResponseOptional};
-use self::types::{Error as ResponseError, *};
+use self::types::*;
 use ::types::beacon_response::ExecutionOptimisticFinalizedBeaconResponse;
 use derivative::Derivative;
 use futures::Stream;
 use futures_util::StreamExt;
 use libp2p_identity::PeerId;
-use pretty_reqwest_error::PrettyReqwestError;
 pub use reqwest;
 use reqwest::{
     Body, IntoUrl, RequestBuilder, Response,
@@ -34,7 +35,6 @@ use serde::{Serialize, de::DeserializeOwned};
 use ssz::Encode;
 use std::fmt;
 use std::future::Future;
-use std::path::PathBuf;
 use std::time::Duration;
 
 pub const V1: EndpointVersion = EndpointVersion(1);
@@ -68,83 +68,6 @@ const HTTP_GET_DEPOSIT_SNAPSHOT_QUOTIENT: u32 = 4;
 const HTTP_GET_VALIDATOR_BLOCK_TIMEOUT_QUOTIENT: u32 = 4;
 const HTTP_DEFAULT_TIMEOUT_QUOTIENT: u32 = 4;
 
-#[derive(Debug)]
-pub enum Error {
-    /// The `reqwest` client raised an error.
-    HttpClient(PrettyReqwestError),
-    /// The `reqwest_eventsource` client raised an error.
-    SseClient(Box<reqwest_eventsource::Error>),
-    /// 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.
-    ServerIndexedMessage(IndexedErrorMessage),
-    /// The server returned an error message where the body was unable to be parsed.
-    StatusCode(StatusCode),
-    /// The supplied URL is badly formatted. It should look something like `http://127.0.0.1:5052`.
-    InvalidUrl(SensitiveUrl),
-    /// The supplied validator client secret is invalid.
-    InvalidSecret(String),
-    /// The server returned a response with an invalid signature. It may be an impostor.
-    InvalidSignatureHeader,
-    /// The server returned a response without a signature header. It may be an impostor.
-    MissingSignatureHeader,
-    /// The server returned an invalid JSON response.
-    InvalidJson(serde_json::Error),
-    /// The server returned an invalid server-sent event.
-    InvalidServerSentEvent(String),
-    /// The server sent invalid response headers.
-    InvalidHeaders(String),
-    /// The server returned an invalid SSZ response.
-    InvalidSsz(ssz::DecodeError),
-    /// An I/O error occurred while loading an API token from disk.
-    TokenReadError(PathBuf, std::io::Error),
-    /// The client has been configured without a server pubkey, but requires one for this request.
-    NoServerPubkey,
-    /// The client has been configured without an API token, but requires one for this request.
-    NoToken,
-}
-
-impl From<reqwest::Error> for Error {
-    fn from(error: reqwest::Error) -> Self {
-        Error::HttpClient(error.into())
-    }
-}
-
-impl Error {
-    /// If the error has a HTTP status code, return it.
-    pub fn status(&self) -> Option<StatusCode> {
-        match self {
-            Error::HttpClient(error) => error.inner().status(),
-            Error::SseClient(error) => {
-                if let reqwest_eventsource::Error::InvalidStatusCode(status, _) = error.as_ref() {
-                    Some(*status)
-                } else {
-                    None
-                }
-            }
-            Error::ServerMessage(msg) => StatusCode::try_from(msg.code).ok(),
-            Error::ServerIndexedMessage(msg) => StatusCode::try_from(msg.code).ok(),
-            Error::StatusCode(status) => Some(*status),
-            Error::InvalidUrl(_) => None,
-            Error::InvalidSecret(_) => None,
-            Error::InvalidSignatureHeader => None,
-            Error::MissingSignatureHeader => None,
-            Error::InvalidJson(_) => None,
-            Error::InvalidSsz(_) => None,
-            Error::InvalidServerSentEvent(_) => None,
-            Error::InvalidHeaders(_) => None,
-            Error::TokenReadError(..) => None,
-            Error::NoServerPubkey | Error::NoToken => None,
-        }
-    }
-}
-
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{:?}", self)
-    }
-}
-
 /// A struct to define a variety of different timeouts for different validator tasks to ensure
 /// proper fallback behaviour.
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -2928,37 +2851,3 @@ impl BeaconNodeHttpClient {
             .await
     }
 }
-
-/// Returns `Ok(response)` if the response is a `200 OK` response. Otherwise, creates an
-/// appropriate error message.
-pub async fn ok_or_error(response: Response) -> Result<Response, Error> {
-    let status = response.status();
-
-    if status == StatusCode::OK {
-        Ok(response)
-    } else if let Ok(message) = response.json().await {
-        match message {
-            ResponseError::Message(message) => Err(Error::ServerMessage(message)),
-            ResponseError::Indexed(indexed) => Err(Error::ServerIndexedMessage(indexed)),
-        }
-    } else {
-        Err(Error::StatusCode(status))
-    }
-}
-
-/// Returns `Ok(response)` if the response is a success (2xx) response. Otherwise, creates an
-/// appropriate error message.
-pub async fn success_or_error(response: Response) -> Result<Response, Error> {
-    let status = response.status();
-
-    if status.is_success() {
-        Ok(response)
-    } else if let Ok(message) = response.json().await {
-        match message {
-            ResponseError::Message(message) => Err(Error::ServerMessage(message)),
-            ResponseError::Indexed(indexed) => Err(Error::ServerIndexedMessage(indexed)),
-        }
-    } else {
-        Err(Error::StatusCode(status))
-    }
-}

common/eth2/src/lighthouse_vc/http_client.rs

@@ -1,5 +1,5 @@
 use super::types::*;
-use crate::Error;
+use crate::{Error, success_or_error};
 use reqwest::{
     IntoUrl,
     header::{HeaderMap, HeaderValue},
@@ -145,7 +145,7 @@ impl ValidatorClientHttpClient {
             .send()
             .await
             .map_err(Error::from)?;
-        ok_or_error(response).await
+        success_or_error(response).await
     }
 
     /// Perform a HTTP DELETE request, returning the `Response` for further processing.
@@ -157,7 +157,7 @@ impl ValidatorClientHttpClient {
             .send()
             .await
             .map_err(Error::from)?;
-        ok_or_error(response).await
+        success_or_error(response).await
     }
 
     async fn get<T: DeserializeOwned, U: IntoUrl>(&self, url: U) -> Result<T, Error> {
@@ -218,7 +218,7 @@ impl ValidatorClientHttpClient {
             .send()
             .await
             .map_err(Error::from)?;
-        ok_or_error(response).await
+        success_or_error(response).await
     }
 
     async fn post<T: Serialize, U: IntoUrl, V: DeserializeOwned>(
@@ -250,7 +250,7 @@ impl ValidatorClientHttpClient {
             .send()
             .await
             .map_err(Error::from)?;
-        ok_or_error(response).await?;
+        success_or_error(response).await?;
         Ok(())
     }
 
@@ -268,7 +268,7 @@ impl ValidatorClientHttpClient {
             .send()
             .await
             .map_err(Error::from)?;
-        ok_or_error(response).await
+        success_or_error(response).await
     }
 
     /// Perform a HTTP DELETE request.
@@ -681,20 +681,3 @@ impl ValidatorClientHttpClient {
         self.delete(url).await
     }
 }
-
-/// Returns `Ok(response)` if the response is a `200 OK` response or a
-/// `202 Accepted` response. Otherwise, creates an appropriate error message.
-async fn ok_or_error(response: Response) -> Result<Response, Error> {
-    let status = response.status();
-
-    if status == StatusCode::OK
-        || status == StatusCode::ACCEPTED
-        || status == StatusCode::NO_CONTENT
-    {
-        Ok(response)
-    } else if let Ok(message) = response.json().await {
-        Err(Error::ServerMessage(message))
-    } else {
-        Err(Error::StatusCode(status))
-    }
-}

common/eth2/src/types.rs

@@ -26,46 +26,8 @@ pub use types::*;
 #[cfg(feature = "lighthouse")]
 use crate::lighthouse::BlockReward;
 
-/// An API error serializable to JSON.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(untagged)]
-pub enum Error {
-    Indexed(IndexedErrorMessage),
-    Message(ErrorMessage),
-}
-
-/// An API error serializable to JSON.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-pub struct ErrorMessage {
-    pub code: u16,
-    pub message: String,
-    #[serde(default)]
-    pub stacktraces: Vec<String>,
-}
-
-/// An indexed API error serializable to JSON.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-pub struct IndexedErrorMessage {
-    pub code: u16,
-    pub message: String,
-    pub failures: Vec<Failure>,
-}
-
-/// A single failure in an index of API errors, serializable to JSON.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-pub struct Failure {
-    pub index: u64,
-    pub message: String,
-}
-
-impl Failure {
-    pub fn new(index: usize, message: String) -> Self {
-        Self {
-            index: index as u64,
-            message,
-        }
-    }
-}
+// Re-export error types from the unified error module
+pub use crate::error::{ErrorMessage, Failure, IndexedErrorMessage, ResponseError as Error};
 
 /// The version of a single API endpoint, e.g. the `v1` in `/eth/v1/beacon/blocks`.
 #[derive(Debug, Clone, Copy, PartialEq)]