Migrate Comms/DHT to thiserror (#1951)

Merge pull request #1951

Migrate Comms/DHT to thiserror

base_layer/wallet_ffi/src/error.rs

@@ -285,15 +285,12 @@ impl From<NodeIdentityError> for LibWalletError {
                 code: 701,
                 message: format!("{:?}", n),
             },
-            NodeIdentityError::NodeIdError(NodeIdError::OutOfBounds) => Self {
-                code: 702,
-                message: format!("{:?}", n),
-            },
-            NodeIdentityError::PoisonedAccess => Self {
+            // 702 NodeIdError::OutOfBounds no longer occurs
+            NodeIdentityError::AddressLockPoisoned => Self {
                 code: 703,
                 message: format!("{:?}", n),
             },
-            NodeIdentityError::NodeIdError(NodeIdError::DigestError) => Self {
+            NodeIdentityError::NodeIdError(NodeIdError::InvalidDigestOutputSize) => Self {
                 code: 704,
                 message: format!("{:?}", n),
             },

comms/Cargo.toml

@@ -21,7 +21,6 @@ chrono = { version = "0.4.6", features = ["serde"] }
 cidr = "0.1.0"
 clear_on_drop = "=0.2.3"
 data-encoding = "2.2.0"
-derive-error = "0.0.4"
 digest = "0.8.0"
 futures =  { version = "^0.3", features = ["async-await"]}
 lazy_static = "1.3.0"
@@ -35,6 +34,7 @@ rand = "0.7.2"
 serde = "1.0.90"
 serde_derive = "1.0.90"
 snow = {version="=0.6.2", features=["default-resolver"]}
+thiserror = "1.0.19"
 tokio = {version="~0.2.19", features=["blocking", "time", "tcp", "dns", "sync", "stream", "signal"]}
 tokio-util = {version="0.2.0", features=["codec"]}
 tower= "0.3.1"

comms/dht/Cargo.toml

@@ -22,7 +22,6 @@ tari_storage = { version = "^0.1", path = "../../infrastructure/storage"}
 bitflags = "1.2.0"
 bytes = "0.4.12"
 chrono = "0.4.9"
-derive-error = "0.0.4"
 diesel = {version="1.4", features = ["sqlite", "serde_json", "chrono", "numeric"]}
 diesel_migrations =  "1.4"
 digest = "0.8.1"
@@ -40,6 +39,7 @@ ttl_cache = "0.5.1"
 
 # tower-filter dependencies
 pin-project = "0.4"
+thiserror = "1.0.19"
 
 [dev-dependencies]
 tari_test_utils = { version = "^0.0", path = "../../infrastructure/test_utils"}

comms/dht/src/actor.rs

@@ -36,7 +36,6 @@ use crate::{
     DhtConfig,
 };
 use chrono::{DateTime, Utc};
-use derive_error::Error;
 use futures::{
     channel::{mpsc, mpsc::SendError, oneshot},
     future,
@@ -54,32 +53,37 @@ use tari_comms::{
 };
 use tari_shutdown::ShutdownSignal;
 use tari_utilities::message_format::{MessageFormat, MessageFormatError};
+use thiserror::Error;
 use tokio::task;
 use ttl_cache::TtlCache;
 
 const LOG_TARGET: &str = "comms::dht::actor";
 
 #[derive(Debug, Error)]
 pub enum DhtActorError {
-    /// MPSC channel is disconnected
+    #[error("MPSC channel is disconnected")]
     ChannelDisconnected,
-    /// MPSC sender was unable to send because the channel buffer is full
+    #[error("MPSC sender was unable to send because the channel buffer is full")]
     SendBufferFull,
-    /// Reply sender canceled the request
+    #[error("Reply sender canceled the request")]
     ReplyCanceled,
-    PeerManagerError(PeerManagerError),
-    #[error(msg_embedded, no_from, non_std)]
-    SendFailed(String),
-    DiscoveryError(DhtDiscoveryError),
-    BlockingJoinError(tokio::task::JoinError),
-    StorageError(StorageError),
-    #[error(no_from)]
+    #[error("PeerManagerError: {0}")]
+    PeerManagerError(#[from] PeerManagerError),
+    #[error("Failed to broadcast join message: {0}")]
+    FailedToBroadcastJoinMessage(String),
+    #[error("DiscoveryError: {0}")]
+    DiscoveryError(#[from] DhtDiscoveryError),
+    #[error("StorageError: {0}")]
+    StorageError(#[from] StorageError),
+    #[error("StoredValueFailedToDeserialize: {0}")]
     StoredValueFailedToDeserialize(MessageFormatError),
-    #[error(no_from)]
+    #[error("FailedToSerializeValue: {0}")]
     FailedToSerializeValue(MessageFormatError),
-    ConnectionManagerError(ConnectionManagerError),
-    ConnectivityError(ConnectivityError),
-    /// Connectivity event stream closed
+    #[error("ConnectionManagerError: {0}")]
+    ConnectionManagerError(#[from] ConnectionManagerError),
+    #[error("ConnectivityError: {0}")]
+    ConnectivityError(#[from] ConnectivityError),
+    #[error("Connectivity event stream closed")]
     ConnectivityEventStreamClosed,
 }
 
@@ -350,7 +354,9 @@ impl DhtActor {
                 message,
             )
             .await
-            .map_err(|err| DhtActorError::SendFailed(format!("Failed to send join message: {}", err)))?;
+            .map_err(|err| {
+                DhtActorError::FailedToBroadcastJoinMessage(format!("Failed to send join message: {}", err))
+            })?;
 
         Ok(())
     }

comms/dht/src/connectivity/mod.rs

@@ -24,7 +24,6 @@
 mod test;
 
 use crate::{DhtActorError, DhtConfig, DhtRequester};
-use derive_error::Error;
 use futures::StreamExt;
 use log::*;
 use std::{sync::Arc, time::Instant};
@@ -36,17 +35,19 @@ use tari_comms::{
     PeerManager,
 };
 use tari_shutdown::ShutdownSignal;
+use thiserror::Error;
 use tokio::{task, task::JoinHandle, time};
 
 const LOG_TARGET: &str = "comms::dht::connectivity";
 
 #[derive(Debug, Error)]
 pub enum DhtConnectivityError {
-    ConnectivityError(ConnectivityError),
-    PeerManagerError(PeerManagerError),
-    /// Failed to send network Join message
-    #[error(no_from)]
-    SendJoinFailed(DhtActorError),
+    #[error("ConnectivityError: {0}")]
+    ConnectivityError(#[from] ConnectivityError),
+    #[error("PeerManagerError: {0}")]
+    PeerManagerError(#[from] PeerManagerError),
+    #[error("Failed to send network Join message: {0}")]
+    SendJoinFailed(#[from] DhtActorError),
 }
 
 /// # DHT Connectivity Actor

comms/dht/src/dht.rs

@@ -39,7 +39,6 @@ use crate::{
     DhtActorError,
     DhtConfig,
 };
-use derive_error::Error;
 use futures::{channel::mpsc, future, Future};
 use log::*;
 use std::sync::Arc;
@@ -50,6 +49,7 @@ use tari_comms::{
     pipeline::PipelineError,
 };
 use tari_shutdown::ShutdownSignal;
+use thiserror::Error;
 use tower::{layer::Layer, Service, ServiceBuilder};
 
 const LOG_TARGET: &str = "comms::dht";
@@ -60,10 +60,12 @@ const DHT_SAF_SERVICE_CHANNEL_SIZE: usize = 100;
 
 #[derive(Debug, Error)]
 pub enum DhtInitializationError {
-    /// Database initialization failed
-    DatabaseMigrationFailed(StorageError),
-    StoreAndForwardInitializationError(StoreAndForwardError),
-    DhtActorInitializationError(DhtActorError),
+    #[error("Database initialization failed: {0}")]
+    DatabaseMigrationFailed(#[from] StorageError),
+    #[error("StoreAndForwardInitializationError: {0}")]
+    StoreAndForwardInitializationError(#[from] StoreAndForwardError),
+    #[error("DhtActorInitializationError: {0}")]
+    DhtActorInitializationError(#[from] DhtActorError),
 }
 
 /// Responsible for starting the DHT actor, building the DHT middleware stack and as a factory

comms/dht/src/discovery/error.rs

@@ -21,31 +21,34 @@
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use crate::outbound::DhtOutboundError;
-use derive_error::Error;
 use futures::channel::mpsc::SendError;
 use tari_comms::{connection_manager::ConnectionManagerError, peer_manager::PeerManagerError};
+use thiserror::Error;
 
 #[derive(Debug, Error)]
 pub enum DhtDiscoveryError {
-    /// The reply channel was canceled
+    #[error("The reply channel was canceled")]
     ReplyCanceled,
-    DhtOutboundError(DhtOutboundError),
-    /// Received public key in peer discovery response which does not match the requested public key
+    #[error("DhtOutboundError: {0}")]
+    DhtOutboundError(#[from] DhtOutboundError),
+    #[error("Received public key in peer discovery response which does not match the requested public key")]
     DiscoveredPeerMismatch,
-    /// Received an invalid `NodeId`
+    #[error("Received an invalid `NodeId`")]
     InvalidNodeId,
-    /// MPSC channel is disconnected
+    #[error("MPSC channel is disconnected")]
     ChannelDisconnected,
-    /// MPSC sender was unable to send because the channel buffer is full
+    #[error("MPSC sender was unable to send because the channel buffer is full")]
     SendBufferFull,
-    /// The discovery request timed out
+    #[error("The discovery request timed out")]
     DiscoveryTimeout,
-    /// Failed to send discovery message
+    #[error("Failed to send discovery message")]
     DiscoverySendFailed,
-    PeerManagerError(PeerManagerError),
-    #[error(msg_embedded, non_std, no_from)]
+    #[error("PeerManagerError: {0}")]
+    PeerManagerError(#[from] PeerManagerError),
+    #[error("InvalidPeerMultiaddr: {0}")]
     InvalidPeerMultiaddr(String),
-    ConnectionManagerError(ConnectionManagerError),
+    #[error("ConnectionManagerError: {0}")]
+    ConnectionManagerError(#[from] ConnectionManagerError),
 }
 
 impl DhtDiscoveryError {

comms/dht/src/envelope.rs

@@ -21,36 +21,35 @@
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use bitflags::bitflags;
-use derive_error::Error;
+use bytes::Bytes;
 use serde::{Deserialize, Serialize};
 use std::{
     convert::{TryFrom, TryInto},
     fmt,
     fmt::Display,
 };
-use tari_comms::{peer_manager::NodeId, types::CommsPublicKey};
+use tari_comms::{message::MessageTag, peer_manager::NodeId, types::CommsPublicKey};
 use tari_utilities::{ByteArray, ByteArrayError};
+use thiserror::Error;
 
 // Re-export applicable protos
 pub use crate::proto::envelope::{dht_header::Destination, DhtEnvelope, DhtHeader, DhtMessageType, Network};
-use bytes::Bytes;
-use tari_comms::message::MessageTag;
 
 #[derive(Debug, Error)]
 pub enum DhtMessageError {
-    /// Invalid node destination
+    #[error("Invalid node destination")]
     InvalidDestination,
-    /// Invalid origin public key
+    #[error("Invalid origin public key")]
     InvalidOrigin,
-    /// Invalid or unrecognised DHT message type
+    #[error("Invalid or unrecognised DHT message type")]
     InvalidMessageType,
-    /// Invalid or unrecognised network type
+    #[error("Invalid or unrecognised network type")]
     InvalidNetwork,
-    /// Invalid or unrecognised DHT message flags
+    #[error("Invalid or unrecognised DHT message flags")]
     InvalidMessageFlags,
-    /// Invalid ephemeral public key
+    #[error("Invalid ephemeral public key")]
     InvalidEphemeralPublicKey,
-    /// Header was omitted from the message
+    #[error("Header was omitted from the message")]
     HeaderOmitted,
 }
 

comms/dht/src/inbound/decryption.rs

@@ -26,7 +26,6 @@ use crate::{
     inbound::message::{DecryptedDhtMessage, DhtInboundMessage},
     proto::envelope::OriginMac,
 };
-use derive_error::Error;
 use futures::{task::Context, Future};
 use log::*;
 use prost::Message;
@@ -39,23 +38,24 @@ use tari_comms::{
     utils::signature,
 };
 use tari_utilities::ByteArray;
+use thiserror::Error;
 use tower::{layer::Layer, Service, ServiceExt};
 
 const LOG_TARGET: &str = "comms::middleware::decryption";
 
 #[derive(Error, Debug)]
 enum DecryptionError {
-    /// Failed to validate origin MAC signature
+    #[error("Failed to validate origin MAC signature")]
     OriginMacInvalidSignature,
-    /// Origin MAC contained an invalid public key
+    #[error("Origin MAC contained an invalid public key")]
     OriginMacInvalidPublicKey,
-    /// Origin MAC not provided for encrypted message
+    #[error("Origin MAC not provided for encrypted message")]
     OriginMacNotProvided,
-    /// Failed to decrypt origin MAC
+    #[error("Failed to decrypt origin MAC")]
     OriginMacDecryptedFailed,
-    /// Failed to decode clear-text origin MAC
+    #[error("Failed to decode clear-text origin MAC")]
     OriginMacClearTextDecodeFailed,
-    /// Failed to decrypt message body
+    #[error("Failed to decrypt message body")]
     MessageBodyDecryptionFailed,
 }
 
@@ -213,7 +213,7 @@ where S: Service<DecryptedDhtMessage, Response = (), Error = PipelineError>
         body: &[u8],
     ) -> Result<(), DecryptionError>
     {
-        if signature::verify(public_key, signature, body).unwrap_or(false) {
+        if signature::verify(public_key, signature, body) {
             Ok(())
         } else {
             Err(DecryptionError::OriginMacInvalidSignature)

comms/dht/src/inbound/error.rs

@@ -21,24 +21,25 @@
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 use crate::{discovery::DhtDiscoveryError, outbound::DhtOutboundError};
-use derive_error::Error;
-use prost::DecodeError;
 use tari_comms::{message::MessageError, peer_manager::PeerManagerError};
+use thiserror::Error;
 
 #[derive(Debug, Error)]
 pub enum DhtInboundError {
-    MessageError(MessageError),
-    PeerManagerError(PeerManagerError),
-    DhtOutboundError(DhtOutboundError),
-    /// Failed to decode message
-    DecodeError(DecodeError),
-    /// Message body invalid
+    #[error("MessageError: {0}")]
+    MessageError(#[from] MessageError),
+    #[error("PeerManagerError: {0}")]
+    PeerManagerError(#[from] PeerManagerError),
+    #[error("DhtOutboundError: {0}")]
+    DhtOutboundError(#[from] DhtOutboundError),
+    #[error("Message body invalid")]
     InvalidMessageBody,
-    /// Node ID is invalid
+    #[error("Node ID is invalid")]
     InvalidNodeId,
-    /// All given addresses were invalid
+    #[error("All given addresses were invalid")]
     InvalidAddresses,
-    DhtDiscoveryError(DhtDiscoveryError),
-    #[error(msg_embedded, no_from, non_std)]
+    #[error("DhtDiscoveryError: {0}")]
+    DhtDiscoveryError(#[from] DhtDiscoveryError),
+    #[error("OriginRequired: {0}")]
     OriginRequired(String),
 }