Revert "feat!: limit error surface"

This reverts commit b17a536.
maidsafe · Jul 24, 2024 · 6bf355c · 6bf355c
1 parent 7ce955e
commit 6bf355c
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 39 deletions.
diff --git a/sn_client/src/api.rs b/sn_client/src/api.rs
@@ -57,13 +57,6 @@ 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> {
@@ -121,9 +114,7 @@ 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()
-            .map_err(NetworkError::MdnsBuildBehaviourError)?;
+        let (network, mut network_event_receiver, swarm_driver) = network_builder.build_client()?;
         info!("Client constructed network and swarm_driver");
 
         // If the events broadcaster is not provided by the caller, then we create a new one.
@@ -459,7 +450,7 @@ impl Client {
     ) -> Result<SignedRegister> {
         let key = NetworkAddress::from_register_address(address).to_record_key();
         let get_quorum = if is_verifying {
-            Quorum::N(QUORUM_2)
+            Quorum::N(NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?)
         } else {
             Quorum::One
         };
@@ -660,7 +651,9 @@ impl Client {
 
         let verification = if verify_store {
             let verification_cfg = GetRecordCfg {
-                get_quorum: Quorum::N(QUORUM_2),
+                get_quorum: Quorum::N(
+                    NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?,
+                ),
                 retry_strategy,
                 target_record: None, // Not used since we use ChunkProof
                 expected_holders: Default::default(),
@@ -775,7 +768,7 @@ impl Client {
                 address.clone(),
                 random_nonce,
                 expected_proof,
-                Quorum::N(QUORUM_2),
+                Quorum::N(NonZeroUsize::new(2).ok_or(Error::NonZeroUsizeWasInitialisedAsZero)?),
                 None,
             )
             .await

diff --git a/sn_client/src/error.rs b/sn_client/src/error.rs
@@ -90,6 +90,9 @@ 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),
 

diff --git a/sn_networking/src/driver.rs b/sn_networking/src/driver.rs
@@ -117,13 +117,6 @@ 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 {
@@ -311,7 +304,10 @@ impl NetworkBuilder {
             // 1mb packet size
             .set_max_packet_size(MAX_PACKET_SIZE)
             // How many nodes _should_ store data.
-            .set_replication_factor(REPLICATION_FACTOR)
+            .set_replication_factor(
+                NonZeroUsize::new(CLOSE_GROUP_SIZE)
+                    .ok_or_else(|| NetworkError::InvalidCloseGroupSize)?,
+            )
             .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)
@@ -380,12 +376,7 @@ impl NetworkBuilder {
     }
 
     /// Same as `build_node` API but creates the network components in client mode
-    pub fn build_client(
-        self,
-    ) -> std::result::Result<
-        (Network, mpsc::Receiver<NetworkEvent>, SwarmDriver),
-        MdnsBuildBehaviourError,
-    > {
+    pub fn build_client(self) -> Result<(Network, mpsc::Receiver<NetworkEvent>, SwarmDriver)> {
         // 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
@@ -397,7 +388,10 @@ 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(REPLICATION_FACTOR);
+            .set_replication_factor(
+                NonZeroUsize::new(CLOSE_GROUP_SIZE)
+                    .ok_or_else(|| NetworkError::InvalidCloseGroupSize)?,
+            );
 
         let (network, net_event_recv, driver) = self.build(
             kad_cfg,
@@ -421,10 +415,7 @@ impl NetworkBuilder {
         req_res_protocol: ProtocolSupport,
         identify_version: String,
         #[cfg(feature = "upnp")] upnp: bool,
-    ) -> std::result::Result<
-        (Network, mpsc::Receiver<NetworkEvent>, SwarmDriver),
-        MdnsBuildBehaviourError,
-    > {
+    ) -> Result<(Network, mpsc::Receiver<NetworkEvent>, SwarmDriver)> {
         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"))]
@@ -894,7 +885,3 @@ impl SwarmDriver {
         Ok(())
     }
 }
-
-#[derive(Debug, thiserror::Error)]
-#[error("building the mDNS behaviour failed: {0}")]
-pub struct MdnsBuildBehaviourError(#[from] std::io::Error);
diff --git a/sn_networking/src/error.rs b/sn_networking/src/error.rs
@@ -24,8 +24,6 @@ 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
@@ -157,6 +155,9 @@ 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,
 
@@ -184,7 +185,7 @@ pub enum NetworkError {
     OutgoingResponseDropped(Response),
 
     #[error("Error setting up behaviour: {0}")]
-    MdnsBuildBehaviourError(#[from] MdnsBuildBehaviourError),
+    BahviourErr(String),
 }
 
 #[cfg(test)]