From 10a0e3b6295431ccfbde4d3c0f4c4e1312b08251 Mon Sep 17 00:00:00 2001 From: John Smith Date: Thu, 15 Dec 2022 20:52:24 -0500 Subject: [PATCH] bug fixes --- veilid-core/src/network_manager/mod.rs | 6 ++- .../src/routing_table/routing_table_inner.rs | 4 +- .../src/rpc_processor/rpc_find_node.rs | 19 +++------- veilid-core/src/veilid_api/debug.rs | 3 ++ veilid-core/src/veilid_api/error.rs | 13 +++++++ veilid-core/src/veilid_api/routing_context.rs | 2 + veilid-tools/src/network_result.rs | 38 +++++++++++++------ 7 files changed, 58 insertions(+), 27 deletions(-) diff --git a/veilid-core/src/network_manager/mod.rs b/veilid-core/src/network_manager/mod.rs index 2ba43538..60d26957 100644 --- a/veilid-core/src/network_manager/mod.rs +++ b/veilid-core/src/network_manager/mod.rs @@ -1682,7 +1682,7 @@ impl NetworkManager { // } inconsistent - } else { + } else if matches!(public_internet_network_class, NetworkClass::OutboundOnly) { // If we are currently outbound only, we don't have any public dial info // but if we are starting to see consistent socket address from multiple reporting peers // then we may be become inbound capable, so zap the network class so we can re-detect it and any public dial info @@ -1710,6 +1710,10 @@ impl NetworkManager { } } consistent + } else { + // If we are a webapp we never do this. + // If we have invalid network class, then public address detection is already going to happen via the network_class_discovery task + false }; if needs_public_address_detection { diff --git a/veilid-core/src/routing_table/routing_table_inner.rs b/veilid-core/src/routing_table/routing_table_inner.rs index 8338be29..b867bc03 100644 --- a/veilid-core/src/routing_table/routing_table_inner.rs +++ b/veilid-core/src/routing_table/routing_table_inner.rs @@ -799,12 +799,12 @@ impl RoutingTableInner { pub fn transform_to_peer_info( &self, routing_domain: RoutingDomain, - own_peer_info: PeerInfo, + own_peer_info: &PeerInfo, k: DHTKey, v: Option>, ) -> PeerInfo { match v { - None => own_peer_info, + None => own_peer_info.clone(), Some(entry) => entry.with(self, |_rti, e| e.make_peer_info(k, routing_domain).unwrap()), } } diff --git a/veilid-core/src/rpc_processor/rpc_find_node.rs b/veilid-core/src/rpc_processor/rpc_find_node.rs index cd4e62b5..098c9714 100644 --- a/veilid-core/src/rpc_processor/rpc_find_node.rs +++ b/veilid-core/src/rpc_processor/rpc_find_node.rs @@ -92,18 +92,16 @@ impl RPCProcessor { // add node information for the requesting node to our routing table let routing_table = self.routing_table(); - let own_peer_info = routing_table.get_own_peer_info(RoutingDomain::PublicInternet); - let has_valid_own_node_info = own_peer_info.is_some(); + let Some(own_peer_info) = routing_table.get_own_peer_info(RoutingDomain::PublicInternet) else { + // Our own node info is not yet available, drop this request. + return Ok(NetworkResult::service_unavailable()); + }; // find N nodes closest to the target node in our routing table let filter = Box::new( move |rti: &RoutingTableInner, _k: DHTKey, v: Option>| { - rti.filter_has_valid_signed_node_info( - RoutingDomain::PublicInternet, - has_valid_own_node_info, - v, - ) + rti.filter_has_valid_signed_node_info(RoutingDomain::PublicInternet, true, v) }, ) as RoutingTableEntryFilter; let filters = VecDeque::from([filter]); @@ -113,12 +111,7 @@ impl RPCProcessor { filters, // transform |rti, k, v| { - rti.transform_to_peer_info( - RoutingDomain::PublicInternet, - own_peer_info.as_ref().unwrap().clone(), - k, - v, - ) + rti.transform_to_peer_info(RoutingDomain::PublicInternet, &own_peer_info, k, v) }, ); diff --git a/veilid-core/src/veilid_api/debug.rs b/veilid-core/src/veilid_api/debug.rs index 724cf6d8..2349a97c 100644 --- a/veilid-core/src/veilid_api/debug.rs +++ b/veilid-core/src/veilid_api/debug.rs @@ -541,6 +541,9 @@ impl VeilidAPI { NetworkResult::Timeout => { return Ok("Timeout".to_owned()); } + NetworkResult::ServiceUnavailable => { + return Ok("ServiceUnavailable".to_owned()); + } NetworkResult::NoConnection(e) => { return Ok(format!("NoConnection({})", e)); } diff --git a/veilid-core/src/veilid_api/error.rs b/veilid-core/src/veilid_api/error.rs index 6d6f1a25..aece21d4 100644 --- a/veilid-core/src/veilid_api/error.rs +++ b/veilid-core/src/veilid_api/error.rs @@ -8,6 +8,14 @@ macro_rules! apibail_timeout { }; } +#[allow(unused_macros)] +#[macro_export] +macro_rules! apibail_try_again { + () => { + return Err(VeilidAPIError::try_again()) + }; +} + #[allow(unused_macros)] #[macro_export] macro_rules! apibail_generic { @@ -95,6 +103,8 @@ pub enum VeilidAPIError { AlreadyInitialized, #[error("Timeout")] Timeout, + #[error("TryAgain")] + TryAgain, #[error("Shutdown")] Shutdown, #[error("Key not found: {key}")] @@ -131,6 +141,9 @@ impl VeilidAPIError { pub fn timeout() -> Self { Self::Timeout } + pub fn try_again() -> Self { + Self::TryAgain + } pub fn shutdown() -> Self { Self::Shutdown } diff --git a/veilid-core/src/veilid_api/routing_context.rs b/veilid-core/src/veilid_api/routing_context.rs index 4e712330..3cffd867 100644 --- a/veilid-core/src/veilid_api/routing_context.rs +++ b/veilid-core/src/veilid_api/routing_context.rs @@ -153,6 +153,7 @@ impl RoutingContext { let answer = match rpc_processor.rpc_call_app_call(dest, request).await { Ok(NetworkResult::Value(v)) => v, Ok(NetworkResult::Timeout) => apibail_timeout!(), + Ok(NetworkResult::ServiceUnavailable) => apibail_try_again!(), Ok(NetworkResult::NoConnection(e)) | Ok(NetworkResult::AlreadyExists(e)) => { apibail_no_connection!(e); } @@ -181,6 +182,7 @@ impl RoutingContext { match rpc_processor.rpc_call_app_message(dest, message).await { Ok(NetworkResult::Value(())) => {} Ok(NetworkResult::Timeout) => apibail_timeout!(), + Ok(NetworkResult::ServiceUnavailable) => apibail_try_again!(), Ok(NetworkResult::NoConnection(e)) | Ok(NetworkResult::AlreadyExists(e)) => { apibail_no_connection!(e); } diff --git a/veilid-tools/src/network_result.rs b/veilid-tools/src/network_result.rs index 55145cf5..0ddb3973 100644 --- a/veilid-tools/src/network_result.rs +++ b/veilid-tools/src/network_result.rs @@ -68,6 +68,7 @@ impl NetworkResultResultExt for NetworkResult> { fn into_result_network_result(self) -> Result, E> { match self { NetworkResult::Timeout => Ok(NetworkResult::::Timeout), + NetworkResult::ServiceUnavailable => Ok(NetworkResult::::ServiceUnavailable), NetworkResult::NoConnection(e) => Ok(NetworkResult::::NoConnection(e)), NetworkResult::AlreadyExists(e) => Ok(NetworkResult::::AlreadyExists(e)), NetworkResult::InvalidMessage(s) => Ok(NetworkResult::::InvalidMessage(s)), @@ -160,6 +161,7 @@ impl FoldedNetworkResultExt for io::Result> { #[must_use] pub enum NetworkResult { Timeout, + ServiceUnavailable, NoConnection(io::Error), AlreadyExists(io::Error), InvalidMessage(String), @@ -170,6 +172,9 @@ impl NetworkResult { pub fn timeout() -> Self { Self::Timeout } + pub fn service_unavailable() -> Self { + Self::ServiceUnavailable + } pub fn no_connection(e: io::Error) -> Self { Self::NoConnection(e) } @@ -201,6 +206,7 @@ impl NetworkResult { pub fn map X>(self, f: F) -> NetworkResult { match self { Self::Timeout => NetworkResult::::Timeout, + Self::ServiceUnavailable => NetworkResult::::ServiceUnavailable, Self::NoConnection(e) => NetworkResult::::NoConnection(e), Self::AlreadyExists(e) => NetworkResult::::AlreadyExists(e), Self::InvalidMessage(s) => NetworkResult::::InvalidMessage(s), @@ -210,6 +216,10 @@ impl NetworkResult { pub fn into_result(self) -> Result { match self { Self::Timeout => Err(io::Error::new(io::ErrorKind::TimedOut, "Timed out")), + Self::ServiceUnavailable => Err(io::Error::new( + io::ErrorKind::NotFound, + "Service unavailable", + )), Self::NoConnection(e) => Err(e), Self::AlreadyExists(e) => Err(e), Self::InvalidMessage(s) => Err(io::Error::new( @@ -230,21 +240,11 @@ impl From> for Option { } } -// impl Clone for NetworkResult { -// fn clone(&self) -> Self { -// match self { -// Self::Timeout => Self::Timeout, -// Self::NoConnection(e) => Self::NoConnection(e.clone()), -// Self::InvalidMessage(s) => Self::InvalidMessage(s.clone()), -// Self::Value(t) => Self::Value(t.clone()), -// } -// } -// } - impl Debug for NetworkResult { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { Self::Timeout => write!(f, "Timeout"), + Self::ServiceUnavailable => write!(f, "ServiceUnavailable"), Self::NoConnection(e) => f.debug_tuple("NoConnection").field(e).finish(), Self::AlreadyExists(e) => f.debug_tuple("AlreadyExists").field(e).finish(), Self::InvalidMessage(s) => f.debug_tuple("InvalidMessage").field(s).finish(), @@ -257,6 +257,7 @@ impl Display for NetworkResult { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { Self::Timeout => write!(f, "Timeout"), + Self::ServiceUnavailable => write!(f, "ServiceUnavailable"), Self::NoConnection(e) => write!(f, "NoConnection({})", e.kind()), Self::AlreadyExists(e) => write!(f, "AlreadyExists({})", e.kind()), Self::InvalidMessage(s) => write!(f, "InvalidMessage({})", s), @@ -275,6 +276,7 @@ macro_rules! network_result_try { ($r: expr) => { match $r { NetworkResult::Timeout => return Ok(NetworkResult::Timeout), + NetworkResult::ServiceUnavailable => return Ok(NetworkResult::ServiceUnavailable), NetworkResult::NoConnection(e) => return Ok(NetworkResult::NoConnection(e)), NetworkResult::AlreadyExists(e) => return Ok(NetworkResult::AlreadyExists(e)), NetworkResult::InvalidMessage(s) => return Ok(NetworkResult::InvalidMessage(s)), @@ -287,6 +289,10 @@ macro_rules! network_result_try { $f; return Ok(NetworkResult::Timeout); } + NetworkResult::ServiceUnavailable => { + $f; + return Ok(NetworkResult::ServiceUnavailable); + } NetworkResult::NoConnection(e) => { $f; return Ok(NetworkResult::NoConnection(e)); @@ -340,6 +346,16 @@ macro_rules! network_result_value_or_log { ); $f } + NetworkResult::ServiceUnavailable => { + log_network_result!( + "{} at {}@{}:{}", + "ServiceUnavailable".cyan(), + file!(), + line!(), + column!() + ); + $f + } NetworkResult::NoConnection(e) => { log_network_result!( "{}({}) at {}@{}:{}",