feat: use err result to send errors back to client

Previously the mechanism for handling errors in `handle_valid_service_msg` was to return an `Ok`
result with an empty list of commands, or if you wanted to send an error response back to the
client, to include a command in that `Ok` result.

A new `CmdProcessingClientRespondError` variant is added to `Error` to enable returning an `Err`
result when we want to return an error back to the client. When the command processing code
encounters this error variant, it will process the commands included in the error, which will be the
information to send back to the client.

The intention is also to return `Err` results from `handle_valid_service_msg` when any errors are
encountered, not just for the limited few we want to send back to the client.

sn_node/src/comm/mod.rs

@@ -53,7 +53,7 @@ pub(crate) struct Comm {
 /// Commands for interacting with Comm.
 #[allow(unused)]
 #[derive(Debug, Clone)]
-pub(crate) enum Cmd {
+pub enum Cmd {
     #[cfg(feature = "back-pressure")]
     /// Set message rate for peer to the desired msgs per second
     Regulate { peer: Peer, msgs_per_s: f64 },

sn_node/src/node/error.rs

@@ -8,12 +8,12 @@
 
 use super::Prefix;
 
-use crate::node::handover::Error as HandoverError;
+use crate::node::{flow_ctrl::cmds::Cmd, handover::Error as HandoverError};
 
 use sn_dbc::Error as DbcError;
 use sn_interface::{
     messaging::data::Error as ErrorMsg,
-    types::{convert_dt_error_to_error_msg, DataAddress, Peer, PublicKey},
+    types::{convert_dt_error_to_error_msg, Peer, PublicKey, ReplicatedDataAddress as DataAddress},
 };
 
 use secured_linked_list::error::Error as SecuredLinkedListError;
@@ -203,6 +203,15 @@ pub enum Error {
     GenesisDbcError(String),
     #[error("DbcError: {0}")]
     DbcError(#[from] DbcError),
+    /// An error occurred while processing a command and we want to send a response back to the
+    /// client.
+    ///
+    /// It's not really a type of error as such, but rather a marker for the command processing
+    /// code that we want to send an acknowledgement back to the client. It enables command
+    /// processing (or message handling) to return errors for more robust unit testing. Previously
+    /// if there was an error we would return back an `Ok` result with an empty list of commands.
+    #[error("Error processing command. Response will be sent back to client.")]
+    CmdProcessingClientRespondError(Vec<Cmd>),
 }
 
 impl From<qp2p::ClientEndpointError> for Error {

sn_node/src/node/flow_ctrl/cmd_ctrl.rs

@@ -193,13 +193,25 @@ impl CmdCtrl {
                         .await;
                 }
                 Err(error) => {
+                    if let Error::CmdProcessingClientRespondError(ref cmds) = error {
+                        for cmd in cmds.clone() {
+                            match cmd_process_api.send((cmd, Some(id))).await {
+                                Ok(_) => {
+                                    //no issues
+                                }
+                                Err(error) => {
+                                    error!("Could not enqueue child Cmd: {:?}", error);
+                                }
+                            }
+                        }
+                    }
                     node_event_sender
                         .send(Event::CmdProcessing(CmdProcessEvent::Failed {
                             id,
                             priority,
                             time: SystemTime::now(),
                             cmd_string,
-                            error: format!("{:?}", error),
+                            error: format!("{:?}", &error.to_string()),
                         }))
                         .await;
                 }

sn_node/src/node/flow_ctrl/cmds.rs

@@ -102,7 +102,7 @@ impl CmdJob {
     }
 }
 
-/// Internal cmds for a node.
+/// Commands for a node.
 ///
 /// Cmds are used to connect different modules, allowing
 /// for a better modularization of the code base.
@@ -111,7 +111,7 @@ impl CmdJob {
 /// In other words, it enables enhanced flow control.
 #[allow(clippy::large_enum_variant)]
 #[derive(Debug, Clone)]
-pub(crate) enum Cmd {
+pub enum Cmd {
     /// Cleanup node's PeerLinks, removing any unsused, unconnected peers
     CleanupPeerLinks,
     /// Validate `wire_msg` from `sender`.

sn_node/src/node/messaging/mod.rs

@@ -35,14 +35,14 @@ use std::collections::BTreeSet;
 
 #[derive(Debug, Clone)]
 #[allow(clippy::large_enum_variant)]
-pub(crate) enum OutgoingMsg {
+pub enum OutgoingMsg {
     System(SystemMsg),
     Service(ServiceMsg),
     DstAggregated((BlsShareAuth, Bytes)),
 }
 
 #[derive(Debug, Clone)]
-pub(crate) enum Peers {
+pub enum Peers {
     Single(Peer),
     Multiple(BTreeSet<Peer>),
 }

sn_node/src/node/proposal.rs

@@ -14,8 +14,8 @@ use sn_interface::{
 };
 
 #[allow(clippy::large_enum_variant)]
-#[derive(Clone, Debug, PartialEq)]
-pub(crate) enum Proposal {
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub enum Proposal {
     VoteNodeOffline(NodeState),
     SectionInfo(SectionAuthorityProvider),
     NewElders(SectionAuth<SectionAuthorityProvider>),