feat: use thiserror for errors

Cargo.toml

@@ -10,6 +10,7 @@ repository = "https://github.com/maidsafe/sn_data_types"
 version = "0.11.40"
 
 [dependencies]
+thiserror="1.0.23"
 bincode = "1.2.1"
 multibase = "~0.8.0"
 hex_fmt = "~0.3.0"
@@ -32,6 +33,7 @@ rand_core = "~0.5.1"
   features = [ "derive" ]
 
 [dev-dependencies]
+anyhow="1.0.36" 
 hex = "~0.3.2"
 rand_xorshift = "~0.2.0"
 proptest = "0.10.1"

src/errors.rs

@@ -10,12 +10,13 @@
 use serde::{Deserialize, Serialize};
 use std::{
     collections::BTreeMap,
-    error,
-    fmt::{self, Debug, Display, Formatter},
+    fmt::{self, Debug, Formatter},
     result,
 };
 
-/// A specialised `Result` type.
+use thiserror::Error;
+
+/// A specialised `Result` type for safecoin.
 pub type Result<T> = result::Result<T, Error>;
 
 /// Error debug struct
@@ -32,193 +33,130 @@ impl<'a, T> Debug for ErrorDebug<'a, T> {
 }
 
 /// Main error type for the crate.
-#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
+#[derive(Error, Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 pub enum Error {
-    /// Access is denied for a given requester
+    /// Access denied for supplied PublicKey
+    #[error("Access Denied")]
     AccessDenied,
     /// Serialization error
+    #[error("Serialisation error: {0}")]
     Bincode(String),
-    /// Login packet does not exist
-    NoSuchLoginPacket,
-    /// Attempt to store a login packet at an already occupied address
-    LoginPacketExists,
     /// Requested data not found
+    #[error("Requested data not found")]
     NoSuchData,
-    /// Attempt to create a mutable data when data with such a name already exists
+    /// Provided data already exists on the network
+    #[error("Data provided already exists")]
     DataExists,
-    /// Requested entry not found
+    /// Entry could not be found on the data
+    #[error("Requested entry not found")]
     NoSuchEntry,
-    /// Exceeded a limit on a number of entries
+    /// Exceeds limit on entrites for the given data type
+    #[error("Exceeded a limit on a number of entries")]
     TooManyEntries,
-    /// Some entry actions are not valid.
+    /// Supplied actions are not valid
+    #[error("Some entry actions are not valid")]
     InvalidEntryActions(BTreeMap<Vec<u8>, EntryError>),
     /// Key does not exist
+    #[error("Key does not exist")]
     NoSuchKey,
     /// Duplicate Entries in this push
+    #[error("Duplicate entries provided")]
     DuplicateEntryKeys,
     /// The list of owner keys is invalid
+    #[error("Invalid owner keys")]
     InvalidOwners,
     /// No Policy has been set to the data
+    #[error("No policy has been set for this data")]
     PolicyNotSet,
     /// Invalid version for performing a given mutating operation. Contains the
     /// current data version.
+    #[error("Invalid version provided: {0}")]
     InvalidSuccessor(u64),
     /// Invalid version for performing a given mutating operation. Contains the
     /// current owners version.
+    #[error("Invalid owners version provided: {0}")]
     InvalidOwnersSuccessor(u64),
     /// Invalid mutating operation as it causality dependency is currently not satisfied
+    #[error("Operation is not causally ready. Ensure you have the full history of operations.")]
     OpNotCausallyReady,
+    /// Invalid version for performing a given mutating operation. Contains the
+    /// current permissions version.
+    #[error("Invalid permission version provided: {0}")]
+    InvalidPermissionsSuccessor(u64),
     /// Invalid Operation such as a POST on ImmutableData
+    #[error("Invalid operation")]
     InvalidOperation,
     /// Mismatch between key type and signature type.
+    #[error("Sign key and signature type do not match")]
     SigningKeyTypeMismatch,
     /// Failed signature validation.
+    #[error("Invalid signature")]
     InvalidSignature,
     /// Received a request with a duplicate MessageId
+    #[error("Duplicate message id received")]
     DuplicateMessageId,
     /// Network error occurring at Node level which has no bearing on clients, e.g. serialisation
     /// failure or database failure
+    #[error("Network error: {0}")]
     NetworkOther(String),
     /// While parsing, precision would be lost.
+    #[error("Lost precision on the number of coins during parsing")]
     LossOfPrecision,
     /// The coin amount would exceed
-    /// [the maximum value for `Money`](constant.MAX_MONEY_VALUE.html).
+    /// [the maximum value for `Coins`](constant.MAX_COINS_VALUE.html).
+    #[error("Overflow on number of coins (check the MAX_COINS_VALUE const)")]
     ExcessiveValue,
-    /// Failed to parse the string as [`Money`](struct.Money.html).
+    /// Failed to parse the string as [`Coins`](struct.Coins.html).
+    #[error("Failed to parse: {0}")]
     FailedToParse(String),
-    /// Transfer ID already exists.
-    TransferIdExists,
-    /// Insufficient money.
+    /// Transaction ID already exists.
+    #[error("Transaction Id already exists")]
+    TransactionIdExists,
+    /// Insufficient coins.
+    #[error("Insufficient balance to complete this operation")]
     InsufficientBalance,
     /// Inexistent balance.
+    // TODO: key/wallet/balance, what's our vocab here?
+    #[error("No such key exists")]
     NoSuchBalance,
     /// Inexistent sender balance.
+    #[error("No such sender key balance")]
     NoSuchSender,
     /// Inexistent recipient balance.
+    // TODO: this should not be possible
+    #[error("No such recipient key balance")]
     NoSuchRecipient,
     /// Coin balance already exists.
+    #[error("Key already exists")]
     BalanceExists,
     /// Expected data size exceeded.
+    #[error("Size of the structure exceeds the limit")]
     ExceededSize,
-    /// Unexpected error.
-    Unexpected(String),
-}
-
-impl<T: Into<String>> From<T> for Error {
-    fn from(err: T) -> Self {
-        Error::NetworkOther(err.into())
-    }
-}
-
-impl Display for Error {
-    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        match *self {
-            Error::AccessDenied => write!(f, "Access denied"),
-            Error::Bincode(_) => write!(f, "Serialization error"),
-            Error::NoSuchLoginPacket => write!(f, "Login packet does not exist"),
-            Error::LoginPacketExists => write!(f, "Login packet already exists at this location"),
-            Error::NoSuchData => write!(f, "Requested data not found"),
-            Error::DataExists => write!(f, "Data given already exists"),
-            Error::NoSuchEntry => write!(f, "Requested entry not found"),
-            Error::TooManyEntries => write!(f, "Exceeded a limit on a number of entries"),
-            Error::InvalidEntryActions(ref errors) => {
-                write!(f, "Entry actions are invalid: {:?}", errors)
-            }
-            Error::NoSuchKey => write!(f, "Key does not exists"),
-            Error::DuplicateEntryKeys => write!(f, "Duplicate keys in this push"),
-            Error::InvalidOwners => write!(f, "The list of owner keys is invalid"),
-            Error::PolicyNotSet => write!(f, "No Policy has been set to the data"),
-            Error::InvalidOperation => write!(f, "Requested operation is not allowed"),
-            Error::InvalidSuccessor(_) => {
-                write!(f, "Data given is not a valid successor of stored data")
-            }
-            Error::InvalidOwnersSuccessor(_) => {
-                write!(f, "Data given is not a valid successor of stored data")
-            }
-            Error::OpNotCausallyReady => write!(
-                f,
-                "Data operation depends on a different causal state than the current"
-            ),
-            Error::SigningKeyTypeMismatch => {
-                write!(f, "Mismatch between key type and signature type")
-            }
-            Error::InvalidSignature => write!(f, "Failed signature validation"),
-            Error::NetworkOther(ref error) => write!(f, "Error on Node network: {}", error),
-            Error::LossOfPrecision => {
-                write!(f, "Lost precision on the amount of money during parsing")
-            }
-            Error::ExcessiveValue => write!(
-                f,
-                "Overflow on amount of money (check the MAX_MONEY_VALUE const)"
-            ),
-            Error::FailedToParse(ref error) => {
-                write!(f, "Failed to parse from a string: {}", error)
-            }
-            Error::TransferIdExists => write!(f, "Transfer with a given ID already exists"),
-            Error::InsufficientBalance => write!(f, "Not enough money to complete this operation"),
-            Error::NoSuchBalance => write!(f, "Balance does not exist"),
-            Error::NoSuchSender => write!(f, "Sender does not exist"),
-            Error::NoSuchRecipient => write!(f, "Recipient does not exist"),
-            Error::BalanceExists => write!(f, "Balance already exists"),
-            Error::DuplicateMessageId => write!(f, "MessageId already exists"),
-            Error::ExceededSize => write!(f, "Size of the structure exceeds the limit"),
-            Error::Unexpected(ref error) => write!(f, "Unexpected error: {}", error),
-        }
-    }
-}
-
-impl error::Error for Error {
-    fn description(&self) -> &str {
-        match *self {
-            Error::AccessDenied => "Access denied",
-            Error::Bincode(_) => "Serialization error",
-            Error::NoSuchLoginPacket => "Login packet does not exist",
-            Error::LoginPacketExists => "Login packet already exists at this location",
-            Error::NoSuchData => "No such data",
-            Error::DataExists => "Data exists",
-            Error::NoSuchEntry => "No such entry",
-            Error::TooManyEntries => "Too many entries",
-            Error::InvalidEntryActions(_) => "Invalid entry actions",
-            Error::NoSuchKey => "No such key",
-            Error::DuplicateEntryKeys => "Duplicate keys in this push",
-            Error::InvalidOwners => "Invalid owners",
-            Error::PolicyNotSet => "No Policy has been set to the data",
-            Error::InvalidSuccessor(_) => "Invalid data successor",
-            Error::InvalidOwnersSuccessor(_) => "Invalid owners successor",
-            Error::OpNotCausallyReady => "Operation's is currently not causally ready",
-            Error::InvalidOperation => "Invalid operation",
-            Error::SigningKeyTypeMismatch => "Key type and signature type mismatch",
-            Error::InvalidSignature => "Invalid signature",
-            Error::NetworkOther(ref error) => error,
-            Error::LossOfPrecision => "Lost precision on the amount of money during parsing",
-            Error::ExcessiveValue => {
-                "Overflow on amount of money (check the MAX_MONEY_VALUE const)"
-            }
-            Error::FailedToParse(_) => "Failed to parse entity",
-            Error::TransferIdExists => "Transfer with a given ID already exists",
-            Error::InsufficientBalance => "Not enough money to complete this operation",
-            Error::NoSuchBalance => "Balance does not exist",
-            Error::NoSuchSender => "Sender does not exist",
-            Error::NoSuchRecipient => "Recipient does not exist",
-            Error::BalanceExists => "Balance already exists",
-            Error::DuplicateMessageId => "MessageId already exists",
-            Error::ExceededSize => "Exceeded the size limit",
-            Error::Unexpected(_) => "Unexpected error",
-        }
-    }
+    /// Could not be serlialised
+    #[error("Could not deserialize as ed25519 secret key")]
+    Ed25519SecretKey,
+    /// The operation has not been signed by an actor PK and so cannot be validated.
+    #[error("CRDT operation missing actor signature")]
+    CrdtMissingOpSignature,
+    /// The data for a given policy could not be located, so CRDT operations cannot be applied
+    #[error("CRDT data is in an unexpected state. No data ffound for requested policy.")]
+    CrdtUnexpectedState,
 }
 
 pub(crate) fn convert_bincode_error(err: bincode::Error) -> Error {
     Error::Bincode(err.as_ref().to_string())
 }
 
 /// Entry error for `Error::InvalidEntryActions`.
-#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
+#[derive(Error, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
 pub enum EntryError {
     /// Entry does not exists.
+    #[error("Entry does not exist")]
     NoSuchEntry,
     /// Entry already exists. Contains the current entry Key.
+    #[error("Entry already exists {0}")]
     EntryExists(u8),
     /// Invalid version when updating an entry. Contains the current entry Key.
+    #[error("Entry version for updating the entry {0}")]
     InvalidSuccessor(u8),
 }

src/keys/keypair.rs

@@ -109,9 +109,7 @@ impl Keypair {
                 let bytes = keypair.secret.to_bytes();
                 match ed25519_dalek::SecretKey::from_bytes(&bytes) {
                     Ok(sk) => Ok(SecretKey::Ed25519(sk)),
-                    Err(_) => Err(Error::Unexpected(
-                        "Could not deserialise Ed25519 secret key".to_string(),
-                    )),
+                    Err(_) => Err(Error::Ed25519SecretKey),
                 }
             }
             Self::Bls(keypair) => Ok(SecretKey::Bls(keypair.secret.clone())),

src/messaging/account.rs

@@ -186,7 +186,7 @@ mod tests {
     use crate::{Error, Keypair};
 
     #[test]
-    fn exceed_size_limit() -> Result<(), Error> {
+    fn exceed_size_limit() -> anyhow::Result<()> {
         let our_id = Keypair::new_ed25519(&mut rand::thread_rng());
 
         let acc_data = vec![0; MAX_LOGIN_PACKET_BYTES + 1];
@@ -196,13 +196,13 @@ mod tests {
 
         match res {
             Err(Error::ExceededSize) => Ok(()),
-            Ok(_) => Err(Error::Unexpected("Unexpected success".to_string())),
-            Err(e) => Err(Error::Unexpected(format!("Unexpected error: {:?}", e))),
+            Ok(_) => Err(anyhow::anyhow!("Unexpected success".to_string())),
+            Err(e) => Err(anyhow::anyhow!("Unexpected error: {:?}", e)),
         }
     }
 
     #[test]
-    fn valid() -> Result<(), Error> {
+    fn valid() -> anyhow::Result<()> {
         let our_id = Keypair::new_ed25519(&mut rand::thread_rng());
 
         let acc_data = vec![1; 16];
@@ -220,7 +220,7 @@ mod tests {
                 assert_eq!(ad.data(), acc_data.as_slice());
                 Ok(())
             }
-            Err(e) => Err(Error::Unexpected(format!("Unexpected error: {:?}", e))),
+            Err(e) => Err(anyhow::anyhow!("Unexpected error: {:?}", e)),
         }
     }
 }

src/messaging/mod.rs

@@ -626,7 +626,7 @@ impl fmt::Debug for QueryResponse {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::{PublicBlob, Result, UnseqMap};
+    use crate::{PublicBlob, UnseqMap};
     use std::convert::{TryFrom, TryInto};
 
     #[test]
@@ -640,7 +640,7 @@ mod tests {
     }
 
     #[test]
-    fn try_from() -> Result<()> {
+    fn try_from() -> anyhow::Result<()> {
         use QueryResponse::*;
 
         let i_data = Blob::Public(PublicBlob::new(vec![1, 3, 1, 4]));
@@ -649,7 +649,7 @@ mod tests {
             i_data,
             GetBlob(Ok(i_data.clone()))
                 .try_into()
-                .map_err(|_| Error::Unexpected("Mismatched types".to_string()))?
+                .map_err(|_| anyhow::anyhow!("Mismatched types".to_string()))?
         );
         assert_eq!(
             Err(TryFromError::Response(e.clone())),
@@ -670,7 +670,7 @@ mod tests {
             m_data,
             GetMap(Ok(m_data.clone()))
                 .try_into()
-                .map_err(|_| Error::Unexpected("Mismatched types".to_string()))?
+                .map_err(|_| anyhow::anyhow!("Mismatched types".to_string()))?
         );
         assert_eq!(
             Err(TryFromError::Response(e.clone())),