feat!: limit error surface

In spirit of streamlining our public APIs, it's important to make distinctions between errors.
Our errors are currently grouped into big error types, while many functions only return a
handful of errors, or even just one. This is inconvenient to handle.

BREAKING CHANGE: added/changed an pub enum variant

sn_client/src/api.rs

@@ -57,6 +57,13 @@ pub const CONNECTION_TIMEOUT: Duration = Duration::from_secs(30);
 /// The timeout duration for the client to receive any response from the network.
 const INACTIVITY_TIMEOUT: Duration = Duration::from_secs(30);
 
+// Init during compilation, instead of runtime error that should never happen
+// Option<T>::expect will be stabilised as const in the future (https://github.com/rust-lang/rust/issues/67441)
+const QUORUM_2: NonZeroUsize = match NonZeroUsize::new(2) {
+    Some(v) => v,
+    None => panic!("2 is not be zero"),
+};
+
 impl Client {
     /// A quick client with a random secret key and some peers.
     pub async fn quick_start(peers: Option<Vec<Multiaddr>>) -> Result<Self> {
@@ -114,7 +121,9 @@ impl Client {
         #[cfg(feature = "open-metrics")]
         network_builder.metrics_registry(Some(Registry::default()));
 
-        let (network, mut network_event_receiver, swarm_driver) = network_builder.build_client()?;
+        let (network, mut network_event_receiver, swarm_driver) = network_builder
+            .build_client()
+            .map_err(NetworkError::MdnsBuildBehaviourError)?;
         info!("Client constructed network and swarm_driver");
 
         // If the events broadcaster is not provided by the caller, then we create a new one.
@@ -450,7 +459,7 @@ impl Client {
     ) -> Result<SignedRegister> {
         let key = NetworkAddress::from_register_address(address).to_record_key();
         let get_quorum = if is_verifying {
-            Quorum::N(NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?)
+            Quorum::N(QUORUM_2)
         } else {
             Quorum::One
         };
@@ -651,9 +660,7 @@ impl Client {
 
         let verification = if verify_store {
             let verification_cfg = GetRecordCfg {
-                get_quorum: Quorum::N(
-                    NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?,
-                ),
+                get_quorum: Quorum::N(QUORUM_2),
                 retry_strategy,
                 target_record: None, // Not used since we use ChunkProof
                 expected_holders: Default::default(),
@@ -768,7 +775,7 @@ impl Client {
                 address.clone(),
                 random_nonce,
                 expected_proof,
-                Quorum::N(NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?),
+                Quorum::N(QUORUM_2),
                 None,
             )
             .await

sn_client/src/error.rs

@@ -90,9 +90,6 @@ pub enum Error {
     #[error("Total price exceed possible token amount")]
     TotalPriceTooHigh,
 
-    #[error("Logic error: NonZeroUsize was initialised as zero")]
-    NonZeroUsizeWasInitialisedAsZero,
-
     #[error("Could not connect to the network in {0:?}")]
     ConnectionTimeout(Duration),
 

sn_networking/src/driver.rs

@@ -117,6 +117,13 @@ const NETWORKING_CHANNEL_SIZE: usize = 10_000;
 /// Time before a Kad query times out if no response is received
 const KAD_QUERY_TIMEOUT_S: Duration = Duration::from_secs(10);
 
+// Init during compilation, instead of runtime error that should never happen
+// Option<T>::expect will be stabilised as const in the future (https://github.com/rust-lang/rust/issues/67441)
+const REPLICATION_FACTOR: NonZeroUsize = match NonZeroUsize::new(CLOSE_GROUP_SIZE) {
+    Some(v) => v,
+    None => panic!("CLOSE_GROUP_SIZE should not be zero"),
+};
+
 /// The various settings to apply to when fetching a record from network
 #[derive(Clone)]
 pub struct GetRecordCfg {
@@ -305,10 +312,7 @@ impl NetworkBuilder {
             // 1mb packet size
             .set_max_packet_size(MAX_PACKET_SIZE)
             // How many nodes _should_ store data.
-            .set_replication_factor(
-                NonZeroUsize::new(CLOSE_GROUP_SIZE)
-                    .ok_or_else(|| NetworkError::InvalidCloseGroupSize)?,
-            )
+            .set_replication_factor(REPLICATION_FACTOR)
             .set_query_timeout(KAD_QUERY_TIMEOUT_S)
             // Require iterative queries to use disjoint paths for increased resiliency in the presence of potentially adversarial nodes.
             .disjoint_query_paths(true)
@@ -377,7 +381,12 @@ impl NetworkBuilder {
     }
 
     /// Same as `build_node` API but creates the network components in client mode
-    pub fn build_client(self) -> Result<(Network, mpsc::Receiver<NetworkEvent>, SwarmDriver)> {
+    pub fn build_client(
+        self,
+    ) -> std::result::Result<
+        (Network, mpsc::Receiver<NetworkEvent>, SwarmDriver),
+        MdnsBuildBehaviourError,
+    > {
         // Create a Kademlia behaviour for client mode, i.e. set req/resp protocol
         // to outbound-only mode and don't listen on any address
         let mut kad_cfg = kad::Config::default(); // default query timeout is 60 secs
@@ -389,10 +398,7 @@ impl NetworkBuilder {
             // Require iterative queries to use disjoint paths for increased resiliency in the presence of potentially adversarial nodes.
             .disjoint_query_paths(true)
             // How many nodes _should_ store data.
-            .set_replication_factor(
-                NonZeroUsize::new(CLOSE_GROUP_SIZE)
-                    .ok_or_else(|| NetworkError::InvalidCloseGroupSize)?,
-            );
+            .set_replication_factor(REPLICATION_FACTOR);
 
         let (network, net_event_recv, driver) = self.build(
             kad_cfg,
@@ -416,7 +422,10 @@ impl NetworkBuilder {
         req_res_protocol: ProtocolSupport,
         identify_version: String,
         #[cfg(feature = "upnp")] upnp: bool,
-    ) -> Result<(Network, mpsc::Receiver<NetworkEvent>, SwarmDriver)> {
+    ) -> std::result::Result<
+        (Network, mpsc::Receiver<NetworkEvent>, SwarmDriver),
+        MdnsBuildBehaviourError,
+    > {
         let peer_id = PeerId::from(self.keypair.public());
         // vdash metric (if modified please notify at https://github.com/happybeing/vdash/issues):
         #[cfg(not(target_arch = "wasm32"))]
@@ -881,3 +890,7 @@ impl SwarmDriver {
         Ok(())
     }
 }
+
+#[derive(Debug, thiserror::Error)]
+#[error("building the mDNS behaviour failed: {0}")]
+pub struct MdnsBuildBehaviourError(#[from] std::io::Error);

sn_networking/src/error.rs

@@ -24,6 +24,8 @@ use thiserror::Error;
 use tokio::sync::oneshot;
 use xor_name::XorName;
 
+use crate::driver::MdnsBuildBehaviourError;
+
 pub(super) type Result<T, E = NetworkError> = std::result::Result<T, E>;
 
 /// GetRecord Query errors
@@ -155,9 +157,6 @@ pub enum NetworkError {
     #[error("Could not get enough peers ({required}) to satisfy the request, found {found}")]
     NotEnoughPeers { found: usize, required: usize },
 
-    #[error("Close group size must be a non-zero usize")]
-    InvalidCloseGroupSize,
-
     #[error("Node Listen Address was not provided during construction")]
     ListenAddressNotProvided,
 
@@ -185,7 +184,7 @@ pub enum NetworkError {
     OutgoingResponseDropped(Response),
 
     #[error("Error setting up behaviour: {0}")]
-    BahviourErr(String),
+    MdnsBuildBehaviourError(#[from] MdnsBuildBehaviourError),
 }
 
 #[cfg(test)]