diff --git a/ic-agent/Cargo.toml b/ic-agent/Cargo.toml index b100e009..beebef4e 100644 --- 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 index 5604cb98..820ea9e5 100644 --- 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 { 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 { fetcher: Arc, 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 index 1c0d0b39..d45a480e 100644 --- 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; + async fn check(&self, node: &Node) -> Result; } /// 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 { + async fn check(&self, node: &Node) -> Result { // 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 index 482dfe89..baf9d185 100644 --- 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 { + pub fn new(domain: &str) -> Result { 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 { 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 index d9516bb5..88fa372f 100644 --- 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>; + async fn fetch(&self, url: Url) -> Result, 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> { + async fn fetch(&self, url: Url) -> Result, 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 index 3773c158..92de4c78 100644 --- 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> { + async fn fetch(&self, _url: Url) -> Result, 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 { + async fn check(&self, node: &Node) -> Result { let nodes = self.healthy_nodes.load_full(); let latency = match nodes.contains(node) { true => Some(Duration::from_secs(1)),