diff --git a/src/client.rs b/src/client.rs index d61c791685..c21fd511a6 100644 --- a/src/client.rs +++ b/src/client.rs @@ -81,7 +81,8 @@ impl Client { min_section_size, timer, ) - .map_or(State::Terminated, State::BootstrappingPeer) + .map(State::BootstrappingPeer) + .unwrap_or(State::Terminated) }, pub_id, bootstrap_config, diff --git a/src/node.rs b/src/node.rs index c4e5ae4feb..895cd2a2e6 100644 --- a/src/node.rs +++ b/src/node.rs @@ -141,17 +141,9 @@ impl NodeBuilder { StateMachine::new( move |action_sender, crust_service, timer, outbox2| { if self.first { - if let Some(state) = states::Node::first( - self.cache, - crust_service, - full_id, - min_section_size, - timer, - ) { - State::Node(state) - } else { - State::Terminated - } + states::Node::first(self.cache, crust_service, full_id, min_section_size, timer) + .map(State::Node) + .unwrap_or(State::Terminated) } else if !dev_config.allow_multiple_lan_nodes && crust_service.has_peers_on_lan() { error!( "More than one routing node found on LAN. Currently this is not supported." @@ -168,7 +160,8 @@ impl NodeBuilder { min_section_size, timer, ) - .map_or(State::Terminated, State::BootstrappingPeer) + .map(State::BootstrappingPeer) + .unwrap_or(State::Terminated) } }, pub_id, diff --git a/src/state_machine.rs b/src/state_machine.rs index 970356908f..c19d7439e4 100644 --- a/src/state_machine.rs +++ b/src/state_machine.rs @@ -157,13 +157,31 @@ impl State { ) } - fn replace_with(&mut self, f: F) + fn replace_with(&mut self, f: F) where - F: FnOnce(Self) -> Self, + F: FnOnce(Self) -> Result, + E: Debug, { let old_state = mem::replace(self, State::Terminated); - let new_state = f(old_state); - *self = new_state; + let old_state_log_ident = format!("{}", old_state); + + match f(old_state) { + Ok(new_state) => *self = new_state, + Err(error) => error!( + "{} - Failed state transition: {:?}", + old_state_log_ident, error + ), + } + } +} + +impl Display for State { + fn fmt(&self, formatter: &mut Formatter) -> fmt::Result { + state_dispatch!( + *self, + ref state => write!(formatter, "{}", state), + Terminated => write!(formatter, "Terminated") + ) } } @@ -575,6 +593,6 @@ impl StateMachine { impl Display for StateMachine { fn fmt(&self, formatter: &mut Formatter) -> fmt::Result { - self.state.fmt(formatter) + write!(formatter, "{:?}", self.state) } } diff --git a/src/states/bootstrapping_peer.rs b/src/states/bootstrapping_peer.rs index 0509ad29ac..8e91186fe7 100644 --- a/src/states/bootstrapping_peer.rs +++ b/src/states/bootstrapping_peer.rs @@ -74,7 +74,7 @@ impl BootstrappingPeer { full_id: FullId, min_section_size: usize, timer: Timer, - ) -> Option { + ) -> Result { match target_state { TargetState::Client { .. } => { let _ = crust_service.start_bootstrap(HashSet::new(), CrustUser::Client); @@ -82,11 +82,12 @@ impl BootstrappingPeer { TargetState::RelocatingNode | TargetState::ProvingNode { .. } => { if let Err(error) = crust_service.start_listening_tcp() { error!("Failed to start listening: {:?}", error); - return None; + return Err(error.into()); } } } - Some(Self { + + Ok(Self { action_sender, cache: cache, crust_service, @@ -99,19 +100,25 @@ impl BootstrappingPeer { }) } - pub fn into_target_state(self, proxy_pub_id: PublicId, outbox: &mut EventBox) -> State { + pub fn into_target_state( + self, + proxy_pub_id: PublicId, + outbox: &mut EventBox, + ) -> Result { match self.target_state { - TargetState::Client { msg_expiry_dur } => State::Client(Client::from_bootstrapping( - ClientDetails { - crust_service: self.crust_service, - full_id: self.full_id, - min_section_size: self.min_section_size, - msg_expiry_dur, - proxy_pub_id, - timer: self.timer, - }, - outbox, - )), + TargetState::Client { msg_expiry_dur } => { + Ok(State::Client(Client::from_bootstrapping( + ClientDetails { + crust_service: self.crust_service, + full_id: self.full_id, + min_section_size: self.min_section_size, + msg_expiry_dur, + proxy_pub_id, + timer: self.timer, + }, + outbox, + ))) + } TargetState::RelocatingNode => { let details = RelocatingNodeDetails { action_sender: self.action_sender, @@ -123,12 +130,12 @@ impl BootstrappingPeer { timer: self.timer, }; - if let Some(node) = RelocatingNode::from_bootstrapping(details) { - State::RelocatingNode(node) - } else { - outbox.send_event(Event::RestartRequired); - State::Terminated - } + RelocatingNode::from_bootstrapping(details) + .map(State::RelocatingNode) + .map_err(|err| { + outbox.send_event(Event::RestartRequired); + err + }) } TargetState::ProvingNode { old_full_id, @@ -147,7 +154,9 @@ impl BootstrappingPeer { timer: self.timer, }; - State::ProvingNode(ProvingNode::from_bootstrapping(details, outbox)) + Ok(State::ProvingNode(ProvingNode::from_bootstrapping( + details, outbox, + ))) } } } @@ -434,7 +443,8 @@ mod tests { min_section_size, timer, ) - .map_or(State::Terminated, State::BootstrappingPeer) + .map(State::BootstrappingPeer) + .unwrap_or(State::Terminated) }, pub_id, Some(config), diff --git a/src/states/establishing_node.rs b/src/states/establishing_node.rs index 3da0a14086..95f71042d2 100644 --- a/src/states/establishing_node.rs +++ b/src/states/establishing_node.rs @@ -92,7 +92,7 @@ impl EstablishingNode { crust_service: details.crust_service, full_id: details.full_id, gen_pfx_info: details.gen_pfx_info, - msg_backlog: vec![], + msg_backlog: details.msg_backlog, notified_nodes: details.notified_nodes, parsec_map, peer_mgr: details.peer_mgr, @@ -101,18 +101,14 @@ impl EstablishingNode { poke_timer_token, }; - node.init(details.msg_backlog, outbox)?; + node.init(outbox)?; Ok(node) } - fn init( - &mut self, - msg_backlog: Vec, - outbox: &mut EventBox, - ) -> Result<(), RoutingError> { + fn init(&mut self, outbox: &mut EventBox) -> Result<(), RoutingError> { debug!("{} - State changed to EstablishingNode.", self); - for msg in msg_backlog { + for msg in self.msg_backlog.drain(..).collect_vec() { let _ = self.dispatch_routing_message(msg, outbox)?; } @@ -124,7 +120,7 @@ impl EstablishingNode { sec_info: SectionInfo, old_pfx: Prefix, outbox: &mut EventBox, - ) -> State { + ) -> Result { let details = NodeDetails { ack_mgr: self.ack_mgr, cache: self.cache, @@ -140,10 +136,7 @@ impl EstablishingNode { timer: self.timer, }; - match Node::from_establishing_node(details, sec_info, old_pfx, outbox) { - Ok(node) => State::Node(node), - Err(_) => State::Terminated, - } + Node::from_establishing_node(details, sec_info, old_pfx, outbox).map(State::Node) } fn dispatch_routing_message( diff --git a/src/states/node.rs b/src/states/node.rs index f6569ea6ec..93a2cbcf64 100644 --- a/src/states/node.rs +++ b/src/states/node.rs @@ -142,12 +142,12 @@ impl Node { full_id: FullId, min_section_size: usize, timer: Timer, - ) -> Option { + ) -> Result { let dev_config = config_handler::get_config().dev.unwrap_or_default(); let public_id = *full_id.public_id(); let gen_pfx_info = GenesisPfxInfo { - first_info: create_first_section_info(public_id).ok()?, + first_info: create_first_section_info(public_id)?, latest_info: SectionInfo::default(), }; let parsec_map = ParsecMap::new(full_id.clone(), &gen_pfx_info); @@ -171,13 +171,16 @@ impl Node { let mut node = Self::new(details, true); - if let Err(error) = node.crust_service.start_listening_tcp() { - error!("{} - Failed to start listening: {:?}", node, error); - None - } else { - debug!("{} - State changed to Node.", node); - info!("{} - Started a new network as a seed node.", node); - Some(node) + match node.crust_service.start_listening_tcp() { + Ok(()) => { + debug!("{} - State changed to Node.", node); + info!("{} - Started a new network as a seed node.", node); + Ok(node) + } + Err(error) => { + error!("{} - Failed to start listening: {:?}", node, error); + Err(error.into()) + } } } diff --git a/src/states/proving_node.rs b/src/states/proving_node.rs index 423986b62a..01b506cfc4 100644 --- a/src/states/proving_node.rs +++ b/src/states/proving_node.rs @@ -150,7 +150,7 @@ impl ProvingNode { self, gen_pfx_info: GenesisPfxInfo, outbox: &mut EventBox, - ) -> State { + ) -> Result { let details = EstablishingNodeDetails { ack_mgr: self.ack_mgr, cache: self.cache, @@ -165,10 +165,7 @@ impl ProvingNode { timer: self.timer, }; - match EstablishingNode::from_proving_node(details, outbox) { - Ok(node) => State::EstablishingNode(node), - Err(_) => State::Terminated, - } + EstablishingNode::from_proving_node(details, outbox).map(State::EstablishingNode) } fn dispatch_routing_message( diff --git a/src/states/relocating_node.rs b/src/states/relocating_node.rs index 57af7af637..a670b7be10 100644 --- a/src/states/relocating_node.rs +++ b/src/states/relocating_node.rs @@ -14,7 +14,7 @@ use crate::{ ack_manager::{Ack, AckManager}, cache::Cache, chain::SectionInfo, - error::{Result, RoutingError}, + error::RoutingError, event::Event, id::{FullId, PublicId}, messages::{DirectMessage, HopMessage, MessageContent, RoutingMessage}, @@ -66,7 +66,7 @@ pub struct RelocatingNode { } impl RelocatingNode { - pub fn from_bootstrapping(details: RelocatingNodeDetails) -> Option { + pub fn from_bootstrapping(details: RelocatingNodeDetails) -> Result { let relocation_timer_token = details.timer.schedule(RELOCATE_TIMEOUT); let mut node = Self { action_sender: details.action_sender, @@ -81,12 +81,15 @@ impl RelocatingNode { timer: details.timer, }; - if let Err(error) = node.relocate() { - error!("{} Failed to start relocation: {:?}", node, error); - None - } else { - debug!("{} State changed to RelocatingNode.", node); - Some(node) + match node.relocate() { + Ok(()) => { + debug!("{} State changed to RelocatingNode.", node); + Ok(node) + } + Err(error) => { + error!("{} Failed to start relocation: {:?}", node, error); + Err(error) + } } } @@ -97,7 +100,7 @@ impl RelocatingNode { new_full_id: FullId, our_section: (Prefix, BTreeSet), outbox: &mut EventBox, - ) -> State { + ) -> Result { let service = Self::start_new_crust_service( self.crust_service, *new_full_id.public_id(), @@ -108,7 +111,8 @@ impl RelocatingNode { old_full_id: self.full_id, our_section: our_section, }; - if let Some(peer) = BootstrappingPeer::new( + + match BootstrappingPeer::new( self.action_sender, self.cache, target_state, @@ -117,10 +121,11 @@ impl RelocatingNode { self.min_section_size, self.timer, ) { - State::BootstrappingPeer(peer) - } else { - outbox.send_event(Event::RestartRequired); - State::Terminated + Ok(peer) => Ok(State::BootstrappingPeer(peer)), + Err(error) => { + outbox.send_event(Event::RestartRequired); + Err(error) + } } } @@ -183,7 +188,7 @@ impl RelocatingNode { Transition::Stay } - fn relocate(&mut self) -> Result<()> { + fn relocate(&mut self) -> Result<(), RoutingError> { let request_content = MessageContent::Relocate { message_id: MessageId::new(), }; @@ -283,7 +288,7 @@ impl Base for RelocatingNode { msg: DirectMessage, _: PublicId, _: &mut EventBox, - ) -> Result { + ) -> Result { debug!("{} - Unhandled direct message: {:?}", self, msg); Ok(Transition::Stay) } @@ -293,7 +298,7 @@ impl Base for RelocatingNode { hop_msg: HopMessage, pub_id: PublicId, _: &mut EventBox, - ) -> Result { + ) -> Result { if self.proxy_pub_id != pub_id { return Err(RoutingError::UnknownConnection(pub_id)); } @@ -324,7 +329,7 @@ impl Bootstrapped for RelocatingNode { src_section: Option, route: u8, expires_at: Option, - ) -> Result<()> { + ) -> Result<(), RoutingError> { self.send_routing_message_via_proxy(routing_msg, src_section, route, expires_at) } @@ -340,7 +345,7 @@ impl Bootstrapped for RelocatingNode { impl BootstrappedNotEstablished for RelocatingNode { const SEND_ACK: bool = true; - fn get_proxy_public_id(&self, proxy_name: &XorName) -> Result<&PublicId> { + fn get_proxy_public_id(&self, proxy_name: &XorName) -> Result<&PublicId, RoutingError> { proxied::get_proxy_public_id(self, &self.proxy_pub_id, proxy_name) } }