From 643ee5226c52ffe846e0c707a2d4fa4d2cff2b4a Mon Sep 17 00:00:00 2001 From: jawilk Date: Sat, 1 Apr 2023 08:20:27 +0200 Subject: [PATCH] chore(net): improve network service launch error diagnostic (#2068) --- crates/net/network/src/discovery.rs | 8 ++--- crates/net/network/src/error.rs | 43 +++++++++++++++++++++++++- crates/net/network/src/lib.rs | 1 + crates/net/network/src/manager.rs | 6 ++-- crates/net/network/tests/it/main.rs | 1 + crates/net/network/tests/it/startup.rs | 42 +++++++++++++++++++++++++ 6 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 crates/net/network/tests/it/startup.rs diff --git a/crates/net/network/src/discovery.rs b/crates/net/network/src/discovery.rs index 7842c8ab9e..69f1e4202d 100644 --- a/crates/net/network/src/discovery.rs +++ b/crates/net/network/src/discovery.rs @@ -1,6 +1,6 @@ //! Discovery support for the network. -use crate::error::NetworkError; +use crate::error::{NetworkError, ServiceKind}; use futures::StreamExt; use reth_discv4::{DiscoveryUpdate, Discv4, Discv4Config}; use reth_dns_discovery::{ @@ -58,9 +58,9 @@ impl Discovery { let local_enr = NodeRecord::from_secret_key(discovery_addr, &sk); let (discv4, discv4_updates, _discv4_service) = if let Some(disc_config) = discv4_config { let (discv4, mut discv4_service) = - Discv4::bind(discovery_addr, local_enr, sk, disc_config) - .await - .map_err(NetworkError::Discovery)?; + Discv4::bind(discovery_addr, local_enr, sk, disc_config).await.map_err(|err| { + NetworkError::from_io_error(err, ServiceKind::Discovery(discovery_addr)) + })?; let discv4_updates = discv4_service.update_stream(); // spawn the service let _discv4_service = discv4_service.spawn(); diff --git a/crates/net/network/src/error.rs b/crates/net/network/src/error.rs index 2d4d4ad8ac..55d5e14a8d 100644 --- a/crates/net/network/src/error.rs +++ b/crates/net/network/src/error.rs @@ -6,7 +6,25 @@ use reth_eth_wire::{ errors::{EthHandshakeError, EthStreamError, P2PHandshakeError, P2PStreamError}, DisconnectReason, }; -use std::{fmt, io, io::ErrorKind}; +use std::{fmt, io, io::ErrorKind, net::SocketAddr}; + +/// Service kind. +#[derive(Debug, PartialEq)] +pub enum ServiceKind { + /// Listener service. + Listener(SocketAddr), + /// Discovery service. + Discovery(SocketAddr), +} + +impl fmt::Display for ServiceKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ServiceKind::Listener(addr) => write!(f, "{addr} (listener service)"), + ServiceKind::Discovery(addr) => write!(f, "{addr} (discovery service)"), + } + } +} /// All error variants for the network #[derive(Debug, thiserror::Error)] @@ -14,6 +32,14 @@ pub enum NetworkError { /// General IO error. #[error(transparent)] Io(#[from] io::Error), + /// Error when an address is already in use. + #[error("Address {kind} is already in use (os error 98)")] + AddressAlreadyInUse { + /// Service kind. + kind: ServiceKind, + /// IO error. + error: io::Error, + }, /// IO error when creating the discovery service #[error("Failed to launch discovery service: {0}")] Discovery(io::Error), @@ -24,6 +50,21 @@ pub enum NetworkError { DnsResolver(#[from] ResolveError), } +impl NetworkError { + /// Converts a `std::io::Error` to a more descriptive `NetworkError`. + pub fn from_io_error(err: io::Error, kind: ServiceKind) -> Self { + match err.kind() { + ErrorKind::AddrInUse => NetworkError::AddressAlreadyInUse { kind, error: err }, + _ => { + if let ServiceKind::Discovery(_) = kind { + return NetworkError::Discovery(err) + } + NetworkError::Io(err) + } + } + } +} + /// Abstraction over errors that can lead to a failed session #[auto_impl::auto_impl(&)] pub(crate) trait SessionError: fmt::Debug { diff --git a/crates/net/network/src/lib.rs b/crates/net/network/src/lib.rs index f149efbd6b..66ec9d1cc9 100644 --- a/crates/net/network/src/lib.rs +++ b/crates/net/network/src/lib.rs @@ -139,6 +139,7 @@ pub mod transactions; pub use builder::NetworkBuilder; pub use config::{NetworkConfig, NetworkConfigBuilder}; +pub use discovery::Discovery; pub use fetch::FetchClient; pub use manager::{NetworkEvent, NetworkManager}; pub use message::PeerRequest; diff --git a/crates/net/network/src/manager.rs b/crates/net/network/src/manager.rs index 22af3b43b2..360272056e 100644 --- a/crates/net/network/src/manager.rs +++ b/crates/net/network/src/manager.rs @@ -18,7 +18,7 @@ use crate::{ config::NetworkConfig, discovery::Discovery, - error::NetworkError, + error::{NetworkError, ServiceKind}, eth_requests::IncomingEthRequest, import::{BlockImport, BlockImportOutcome, BlockValidation}, listener::ConnectionListener, @@ -171,7 +171,9 @@ where let peers_manager = PeersManager::new(peers_config); let peers_handle = peers_manager.handle(); - let incoming = ConnectionListener::bind(listener_addr).await?; + let incoming = ConnectionListener::bind(listener_addr).await.map_err(|err| { + NetworkError::from_io_error(err, ServiceKind::Listener(listener_addr)) + })?; let listener_address = Arc::new(Mutex::new(incoming.local_address())); discovery_v4_config = discovery_v4_config.map(|mut disc_config| { diff --git a/crates/net/network/tests/it/main.rs b/crates/net/network/tests/it/main.rs index 87fbbfb96b..ccba08a3a2 100644 --- a/crates/net/network/tests/it/main.rs +++ b/crates/net/network/tests/it/main.rs @@ -1,5 +1,6 @@ mod connect; mod requests; mod session; +mod startup; fn main() {} diff --git a/crates/net/network/tests/it/startup.rs b/crates/net/network/tests/it/startup.rs new file mode 100644 index 0000000000..024cdd1730 --- /dev/null +++ b/crates/net/network/tests/it/startup.rs @@ -0,0 +1,42 @@ +use reth_discv4::{Discv4Config, DEFAULT_DISCOVERY_PORT}; +use reth_network::{ + error::{NetworkError, ServiceKind}, + Discovery, NetworkConfigBuilder, NetworkManager, +}; +use reth_provider::test_utils::NoopProvider; +use secp256k1::SecretKey; +use std::{ + io, + net::{Ipv4Addr, SocketAddr, SocketAddrV4}, +}; + +fn is_addr_in_use_kind(err: NetworkError, kind: ServiceKind) -> bool { + match err { + NetworkError::AddressAlreadyInUse { kind: k, error } => { + k == kind && error.kind() == io::ErrorKind::AddrInUse + } + _ => false, + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_listener_addr_in_use() { + let secret_key = SecretKey::new(&mut rand::thread_rng()); + let config = NetworkConfigBuilder::new(secret_key).build(NoopProvider::default()); + let _network = NetworkManager::new(config).await.unwrap(); + let config = NetworkConfigBuilder::new(secret_key).build(NoopProvider::default()); + let addr = config.discovery_addr; + let result = NetworkManager::new(config).await; + assert!(is_addr_in_use_kind(result.err().unwrap(), ServiceKind::Listener(addr))); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_discovery_addr_in_use() { + let secret_key = SecretKey::new(&mut rand::thread_rng()); + let disc_config = Discv4Config::default(); + let addr = SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, DEFAULT_DISCOVERY_PORT)); + let _discovery = Discovery::new(addr, secret_key, Some(disc_config), None).await.unwrap(); + let disc_config = Discv4Config::default(); + let result = Discovery::new(addr, secret_key, Some(disc_config), None).await; + assert!(is_addr_in_use_kind(result.err().unwrap(), ServiceKind::Discovery(addr))); +}