chore: remove anyhow

dfinity · Jul 17, 2024 · df7b557 · df7b557
1 parent a493beb
commit df7b557
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 22 deletions.
diff --git a/ic-agent/Cargo.toml b/ic-agent/Cargo.toml
@@ -79,7 +79,6 @@ tower = { version = "0.4.13", optional = true }
 async-trait = "0.1.80"
 tracing = "0.1.40"
 arc-swap = "1.7.1"
-anyhow = "1.0.86"
 simple_moving_average = "1.0.2"
 tracing-subscriber = "0.3.18"
 tokio-util = { version = "0.7.11", features = ["full"] }

diff --git a/ic-agent/src/agent/http_transport/dynamic_routing/dynamic_route_provider.rs b/ic-agent/src/agent/http_transport/dynamic_routing/dynamic_route_provider.rs
@@ -8,6 +8,7 @@ use std::{
 use arc_swap::ArcSwap;
 use candid::Principal;
 use reqwest::Client;
+use thiserror::Error;
 use tokio::{
     runtime::Handle,
     sync::{mpsc, watch},
@@ -71,6 +72,20 @@ pub struct DynamicRouteProvider<S> {
     token: CancellationToken,
 }
 
+/// An error that occurred when the DynamicRouteProvider service was running.
+#[derive(Error, Debug)]
+pub enum DynamicRouteProviderError {
+    /// An error when fetching topology of the API nodes.
+    #[error("An error when fetching API nodes: {0}")]
+    NodesFetchError(String),
+    /// An error when checking API node's health.
+    #[error("An error when checking API node's health: {0}")]
+    HealthCheckError(String),
+    /// An invalid domain name provided.
+    #[error("Provided domain name is invalid: {0}")]
+    InvalidDomainName(String),
+}
+
 /// A builder for the `DynamicRouteProvider`.
 pub struct DynamicRouteProviderBuilder<S> {
     fetcher: Arc<dyn Fetch>,

diff --git a/ic-agent/src/agent/http_transport/dynamic_routing/health_check.rs b/ic-agent/src/agent/http_transport/dynamic_routing/health_check.rs
@@ -1,4 +1,3 @@
-use anyhow::bail;
 use async_trait::async_trait;
 use http::{Method, StatusCode};
 use reqwest::{Client, Request};
@@ -13,6 +12,7 @@ use tracing::{debug, error, info, warn};
 use url::Url;
 
 use crate::agent::http_transport::dynamic_routing::{
+    dynamic_route_provider::DynamicRouteProviderError,
     messages::{FetchedNodes, NodeHealthState},
     node::Node,
     snapshot::routing_snapshot::RoutingSnapshot,
@@ -25,7 +25,7 @@ const CHANNEL_BUFFER: usize = 128;
 #[async_trait]
 pub trait HealthCheck: Send + Sync + Debug {
     /// Checks the health of the node.
-    async fn check(&self, node: &Node) -> anyhow::Result<HealthCheckStatus>;
+    async fn check(&self, node: &Node) -> Result<HealthCheckStatus, DynamicRouteProviderError>;
 }
 
 /// A struct representing the health check status of the node.
@@ -72,15 +72,19 @@ const HEALTH_CHECKER: &str = "HealthChecker";
 
 #[async_trait]
 impl HealthCheck for HealthChecker {
-    async fn check(&self, node: &Node) -> anyhow::Result<HealthCheckStatus> {
+    async fn check(&self, node: &Node) -> Result<HealthCheckStatus, DynamicRouteProviderError> {
         // API boundary node exposes /health endpoint and should respond with 204 (No Content) if it's healthy.
-        let url = Url::parse(&format!("https://{}/health", node.domain()))?;
+        let url = Url::parse(&format!("https://{}/health", node.domain())).unwrap();
 
         let mut request = Request::new(Method::GET, url.clone());
         *request.timeout_mut() = Some(self.timeout);
 
         let start = Instant::now();
-        let response = self.http_client.execute(request).await?;
+        let response = self.http_client.execute(request).await.map_err(|err| {
+            DynamicRouteProviderError::HealthCheckError(format!(
+                "Failed to execute GET request to {url}: {err}"
+            ))
+        })?;
         let latency = start.elapsed();
 
         if response.status() != StatusCode::NO_CONTENT {
@@ -89,7 +93,7 @@ impl HealthCheck for HealthChecker {
                 response.status()
             );
             error!(err_msg);
-            bail!(err_msg);
+            return Err(DynamicRouteProviderError::HealthCheckError(err_msg));
         }
 
         Ok(HealthCheckStatus::new(Some(latency)))

diff --git a/ic-agent/src/agent/http_transport/dynamic_routing/node.rs b/ic-agent/src/agent/http_transport/dynamic_routing/node.rs
@@ -1,7 +1,8 @@
 use url::Url;
 
 use crate::agent::ApiBoundaryNode;
-use anyhow::anyhow;
+
+use super::dynamic_route_provider::DynamicRouteProviderError;
 
 /// Represents a node in the dynamic routing.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -11,9 +12,11 @@ pub struct Node {
 
 impl Node {
     /// Creates a new `Node` instance from the domain name.
-    pub fn new(domain: &str) -> anyhow::Result<Self> {
+    pub fn new(domain: &str) -> Result<Self, DynamicRouteProviderError> {
         if !is_valid_domain(domain) {
-            return Err(anyhow!("Invalid domain name {domain}"));
+            return Err(DynamicRouteProviderError::InvalidDomainName(
+                domain.to_string(),
+            ));
         }
         Ok(Self {
             domain: domain.to_string(),
@@ -41,7 +44,7 @@ impl From<&Node> for Url {
 }
 
 impl TryFrom<&ApiBoundaryNode> for Node {
-    type Error = anyhow::Error;
+    type Error = DynamicRouteProviderError;
 
     fn try_from(value: &ApiBoundaryNode) -> Result<Self, Self::Error> {
         Node::new(&value.domain)

diff --git a/ic-agent/src/agent/http_transport/dynamic_routing/nodes_fetch.rs b/ic-agent/src/agent/http_transport/dynamic_routing/nodes_fetch.rs
@@ -1,4 +1,3 @@
-use anyhow::Context;
 use async_trait::async_trait;
 use candid::Principal;
 use reqwest::Client;
@@ -11,9 +10,12 @@ use url::Url;
 use crate::agent::{
     http_transport::{
         dynamic_routing::{
-            health_check::HEALTH_MANAGER_ACTOR, messages::FetchedNodes, node::Node,
-            snapshot::routing_snapshot::RoutingSnapshot, type_aliases::AtomicSwap,
-            type_aliases::SenderWatch,
+            dynamic_route_provider::DynamicRouteProviderError,
+            health_check::HEALTH_MANAGER_ACTOR,
+            messages::FetchedNodes,
+            node::Node,
+            snapshot::routing_snapshot::RoutingSnapshot,
+            type_aliases::{AtomicSwap, SenderWatch},
         },
         reqwest_transport::ReqwestTransport,
     },
@@ -26,7 +28,7 @@ const NODES_FETCH_ACTOR: &str = "NodesFetchActor";
 #[async_trait]
 pub trait Fetch: Sync + Send + Debug {
     /// Fetches the nodes from the topology.
-    async fn fetch(&self, url: Url) -> anyhow::Result<Vec<Node>>;
+    async fn fetch(&self, url: Url) -> Result<Vec<Node>, DynamicRouteProviderError>;
 }
 
 /// A struct representing the fetcher of the nodes from the topology.
@@ -48,16 +50,29 @@ impl NodesFetcher {
 
 #[async_trait]
 impl Fetch for NodesFetcher {
-    async fn fetch(&self, url: Url) -> anyhow::Result<Vec<Node>> {
+    async fn fetch(&self, url: Url) -> Result<Vec<Node>, DynamicRouteProviderError> {
         let transport = ReqwestTransport::create_with_client(url, self.http_client.clone())
-            .with_context(|| "Failed to build transport: {err}")?;
+            .map_err(|err| {
+                DynamicRouteProviderError::NodesFetchError(format!(
+                    "Failed to build transport: {err}"
+                ))
+            })?;
         let agent = Agent::builder()
             .with_transport(transport)
             .build()
-            .with_context(|| "Failed to build an agent: {err}")?;
+            .map_err(|err| {
+                DynamicRouteProviderError::NodesFetchError(format!(
+                    "Failed to build the agent: {err}"
+                ))
+            })?;
         let api_bns = agent
             .fetch_api_boundary_nodes_by_subnet_id(self.subnet_id)
-            .await?;
+            .await
+            .map_err(|err| {
+                DynamicRouteProviderError::NodesFetchError(format!(
+                    "Failed to fetch API nodes: {err}"
+                ))
+            })?;
         // If some API BNs have invalid domain names, they are discarded.
         let nodes = api_bns
             .iter()

diff --git a/ic-agent/src/agent/http_transport/dynamic_routing/test_utils.rs b/ic-agent/src/agent/http_transport/dynamic_routing/test_utils.rs
@@ -8,6 +8,7 @@ use url::Url;
 
 use crate::agent::http_transport::{
     dynamic_routing::{
+        dynamic_route_provider::DynamicRouteProviderError,
         health_check::{HealthCheck, HealthCheckStatus},
         node::Node,
         nodes_fetch::Fetch,
@@ -60,7 +61,7 @@ pub struct NodesFetcherMock {
 
 #[async_trait]
 impl Fetch for NodesFetcherMock {
-    async fn fetch(&self, _url: Url) -> anyhow::Result<Vec<Node>> {
+    async fn fetch(&self, _url: Url) -> Result<Vec<Node>, DynamicRouteProviderError> {
         let nodes = (*self.nodes.load_full()).clone();
         Ok(nodes)
     }
@@ -97,7 +98,7 @@ impl Default for NodeHealthCheckerMock {
 
 #[async_trait]
 impl HealthCheck for NodeHealthCheckerMock {
-    async fn check(&self, node: &Node) -> anyhow::Result<HealthCheckStatus> {
+    async fn check(&self, node: &Node) -> Result<HealthCheckStatus, DynamicRouteProviderError> {
         let nodes = self.healthy_nodes.load_full();
         let latency = match nodes.contains(node) {
             true => Some(Duration::from_secs(1)),