Refactor errors (#158)

* validate error module refactored

 - use thiserror and anomaly
 - rename ValidationError -> validate::Error
 - rename ValidationErrorKind -> Kind

* cleanup Cargo.toml: remove failure and unused feature:
 - we do not use anomalys serializer feature

cleanup Cargo.toml: remove failure and unused feature:
 - we do not use anomalys serializer feature
 - remove last place which used failure
 (looks like we accidentally imported `failure::_core::fmt::Debug`
  instead of just `std::fmt::Debug`

* Address all review comments (thanks @xla)

* These are actually validate::Error too

tendermint/Cargo.toml

@@ -33,7 +33,6 @@ circle-ci = { repository = "interchainio/tendermint-rs" }
 [dependencies]
 bytes = "0.5"
 chrono = { version = "0.4", features = ["serde"] }
-failure = "0.1"
 anomaly = "0.2"
 thiserror = "1"
 getrandom = "0.1"

tendermint/src/abci/data.rs

@@ -1,4 +1,4 @@
-use crate::{Error, ErrorKind};
+use crate::{Error, Kind};
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::{
     fmt::{self, Display},
@@ -42,7 +42,7 @@ impl FromStr for Data {
         // Accept either upper or lower case hex
         let bytes = hex::decode_upper(s)
             .or_else(|_| hex::decode(s))
-            .map_err(|_| ErrorKind::Parse)?;
+            .map_err(|_| Kind::Parse)?;
 
         Ok(Data(bytes))
     }

tendermint/src/abci/gas.rs

@@ -5,7 +5,7 @@
 //!
 //! <https://tendermint.com/docs/spec/abci/apps.html#gas>
 
-use crate::{Error, ErrorKind};
+use crate::{Error, Kind};
 use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
     fmt::{self, Display},
@@ -45,7 +45,7 @@ impl FromStr for Gas {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Error> {
-        Ok(Self::from(s.parse::<u64>().map_err(|_| ErrorKind::Parse)?))
+        Ok(Self::from(s.parse::<u64>().map_err(|_| Kind::Parse)?))
     }
 }
 

tendermint/src/abci/log.rs

@@ -1,4 +1,4 @@
-use crate::{Error, ErrorKind};
+use crate::{Error, Kind};
 use serde::{Deserialize, Serialize};
 use std::fmt::{self, Display};
 
@@ -9,7 +9,7 @@ pub struct Log(String);
 impl Log {
     /// Parse the log data as JSON, returning a `serde_json::Value`
     pub fn parse_json(&self) -> Result<serde_json::Value, Error> {
-        serde_json::from_str(&self.0).map_err(|_| ErrorKind::Parse.into())
+        serde_json::from_str(&self.0).map_err(|_| Kind::Parse.into())
     }
 }
 

tendermint/src/abci/transaction/hash.rs

@@ -1,6 +1,6 @@
 //! Transaction hashes
 
-use crate::error::{Error, ErrorKind};
+use crate::error::{Error, Kind};
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
     fmt::{self, Debug, Display},
@@ -64,10 +64,10 @@ impl FromStr for Hash {
         // Accept either upper or lower case hex
         let bytes = hex::decode_upper(s)
             .or_else(|_| hex::decode(s))
-            .map_err(|_| ErrorKind::Parse)?;
+            .map_err(|_| Kind::Parse)?;
 
         if bytes.len() != LENGTH {
-            return Err(ErrorKind::Parse.into());
+            return Err(Kind::Parse.into());
         }
 
         let mut result_bytes = [0u8; LENGTH];

tendermint/src/account.rs

@@ -1,6 +1,6 @@
 //! Tendermint accounts
 
-use crate::error::{Error, ErrorKind};
+use crate::error::{Error, Kind};
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
 use sha2::{Digest, Sha256};
 use signatory::{ecdsa::curve::secp256k1, ed25519};
@@ -86,10 +86,10 @@ impl FromStr for Id {
         // Accept either upper or lower case hex
         let bytes = hex::decode_upper(s)
             .or_else(|_| hex::decode(s))
-            .map_err(|_| ErrorKind::Parse)?;
+            .map_err(|_| Kind::Parse)?;
 
         if bytes.len() != LENGTH {
-            return Err(ErrorKind::Parse.into());
+            return Err(Kind::Parse.into());
         }
 
         let mut result_bytes = [0u8; LENGTH];

tendermint/src/amino_types/block_id.rs

@@ -1,4 +1,4 @@
-use super::validate::{ConsensusMessage, ValidationError, ValidationErrorKind::*};
+use super::validate::{ConsensusMessage, Kind::InvalidHashSize, Kind::NegativeTotal};
 use crate::block::parts;
 use crate::{
     block,
@@ -44,7 +44,7 @@ impl From<&block::Id> for BlockId {
 }
 
 impl ConsensusMessage for BlockId {
-    fn validate_basic(&self) -> Result<(), ValidationError> {
+    fn validate_basic(&self) -> Result<(), Error> {
         // Hash can be empty in case of POLBlockID in Proposal.
         if !self.hash.is_empty() && self.hash.len() != SHA256_HASH_SIZE {
             return Err(InvalidHashSize.into());
@@ -103,7 +103,7 @@ impl PartsSetHeader {
 }
 
 impl ConsensusMessage for PartsSetHeader {
-    fn validate_basic(&self) -> Result<(), ValidationError> {
+    fn validate_basic(&self) -> Result<(), Error> {
         if self.total < 0 {
             return Err(NegativeTotal.into());
         }

tendermint/src/amino_types/proposal.rs

@@ -4,12 +4,14 @@ use super::{
     remote_error::RemoteError,
     signature::{SignableMsg, SignedMsgType},
     time::TimeMsg,
-    validate::{ConsensusMessage, ValidationError, ValidationErrorKind::*},
+    validate::{
+        self, ConsensusMessage, Kind::InvalidMessageType, Kind::MissingConsensusMessage,
+        Kind::NegativeHeight, Kind::NegativePOLRound, Kind::NegativeRound,
+    },
 };
 use crate::{
     block::{self, ParseId},
-    chain, consensus,
-    error::Error,
+    chain, consensus, error,
 };
 use bytes::BufMut;
 use once_cell::sync::Lazy;
@@ -38,7 +40,7 @@ pub struct Proposal {
 
 // TODO(tony): custom derive proc macro for this e.g. `derive(ParseBlockHeight)`
 impl block::ParseHeight for Proposal {
-    fn parse_block_height(&self) -> Result<block::Height, Error> {
+    fn parse_block_height(&self) -> Result<block::Height, error::Error> {
         block::Height::try_from(self.height)
     }
 }
@@ -73,13 +75,13 @@ struct CanonicalProposal {
 }
 
 impl chain::ParseId for CanonicalProposal {
-    fn parse_chain_id(&self) -> Result<chain::Id, Error> {
+    fn parse_chain_id(&self) -> Result<chain::Id, error::Error> {
         self.chain_id.parse()
     }
 }
 
 impl block::ParseHeight for CanonicalProposal {
-    fn parse_block_height(&self) -> Result<block::Height, Error> {
+    fn parse_block_height(&self) -> Result<block::Height, error::Error> {
         block::Height::try_from(self.height)
     }
 }
@@ -133,7 +135,7 @@ impl SignableMsg for SignProposalRequest {
             prop.signature = sig.as_ref().to_vec();
         }
     }
-    fn validate(&self) -> Result<(), ValidationError> {
+    fn validate(&self) -> Result<(), validate::Error> {
         match self.proposal {
             Some(ref p) => p.validate_basic(),
             None => Err(MissingConsensusMessage.into()),
@@ -172,7 +174,7 @@ impl SignableMsg for SignProposalRequest {
 }
 
 impl ConsensusMessage for Proposal {
-    fn validate_basic(&self) -> Result<(), ValidationError> {
+    fn validate_basic(&self) -> Result<(), validate::Error> {
         if self.msg_type != SignedMsgType::Proposal.to_u32() {
             return Err(InvalidMessageType.into());
         }

tendermint/src/amino_types/signature.rs

@@ -1,4 +1,4 @@
-use super::validate::ValidationError;
+use super::validate;
 use crate::{chain, consensus};
 use bytes::BufMut;
 use prost_amino::{DecodeError, EncodeError};
@@ -15,7 +15,7 @@ pub trait SignableMsg {
 
     /// Set the Ed25519 signature on the underlying message
     fn set_signature(&mut self, sig: &ed25519::Signature);
-    fn validate(&self) -> Result<(), ValidationError>;
+    fn validate(&self) -> Result<(), validate::Error>;
     fn consensus_state(&self) -> Option<consensus::State>;
     fn height(&self) -> Option<i64>;
     fn msg_type(&self) -> Option<SignedMsgType>;

tendermint/src/amino_types/validate.rs

@@ -1,42 +1,30 @@
-use failure::*;
+use thiserror::Error;
 
 pub trait ConsensusMessage {
-    fn validate_basic(&self) -> Result<(), ValidationError>;
+    fn validate_basic(&self) -> Result<(), Error>;
 }
 
-pub struct ValidationError(ValidationErrorKind);
+pub type Error = anomaly::BoxError;
 
 /// Kinds of validation errors
-#[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)]
-pub enum ValidationErrorKind {
-    #[fail(display = "invalid Type")]
+#[derive(Copy, Clone, Eq, PartialEq, Debug, Error)]
+pub enum Kind {
+    #[error("invalid Type")]
     InvalidMessageType,
-    #[fail(display = "consensus message is missing")]
+    #[error("consensus message is missing")]
     MissingConsensusMessage,
-    #[fail(display = "negative height")]
+    #[error("negative height")]
     NegativeHeight,
-    #[fail(display = "negative round")]
+    #[error("negative round")]
     NegativeRound,
-    #[fail(display = "negative POLRound (exception: -1)")]
+    #[error("negative POLRound (exception: -1)")]
     NegativePOLRound,
-    #[fail(display = "negative ValidatorIndex")]
+    #[error("negative ValidatorIndex")]
     NegativeValidatorIndex,
-    #[fail(display = "expected ValidatorAddress size to be 20 bytes")]
+    #[error("expected ValidatorAddress size to be 20 bytes")]
     InvalidValidatorAddressSize,
-    #[fail(display = "Wrong hash: expected Hash size to be 32 bytes")]
+    #[error("Wrong hash: expected Hash size to be 32 bytes")]
     InvalidHashSize,
-    #[fail(display = "negative total")]
+    #[error("negative total")]
     NegativeTotal,
 }
-
-impl ToString for ValidationError {
-    fn to_string(&self) -> String {
-        self.0.to_string()
-    }
-}
-
-impl From<ValidationErrorKind> for ValidationError {
-    fn from(kind: ValidationErrorKind) -> Self {
-        ValidationError(kind)
-    }
-}

tendermint/src/amino_types/vote.rs

@@ -4,7 +4,8 @@ use super::{
     remote_error::RemoteError,
     signature::SignableMsg,
     time::TimeMsg,
-    validate::{ConsensusMessage, ValidationError, ValidationErrorKind::*},
+    validate,
+    validate::{ConsensusMessage, Kind::*},
     SignedMsgType,
 };
 use crate::amino_types::PartsSetHeader;
@@ -178,7 +179,7 @@ impl SignableMsg for SignVoteRequest {
             vt.signature = sig.as_ref().to_vec();
         }
     }
-    fn validate(&self) -> Result<(), ValidationError> {
+    fn validate(&self) -> Result<(), validate::Error> {
         match self.vote {
             Some(ref v) => v.validate_basic(),
             None => Err(MissingConsensusMessage.into()),
@@ -215,7 +216,7 @@ impl SignableMsg for SignVoteRequest {
 }
 
 impl ConsensusMessage for Vote {
-    fn validate_basic(&self) -> Result<(), ValidationError> {
+    fn validate_basic(&self) -> Result<(), validate::Error> {
         if self.msg_type().is_none() {
             return Err(InvalidMessageType.into());
         }

tendermint/src/block/height.rs

@@ -1,4 +1,4 @@
-use crate::error::{Error, ErrorKind};
+use crate::error::{Error, Kind};
 use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
     convert::TryFrom,
@@ -50,7 +50,7 @@ impl TryFrom<i64> for Height {
         if n >= 0 {
             Ok(Height(n as u64))
         } else {
-            Err(ErrorKind::OutOfRange.into())
+            Err(Kind::OutOfRange.into())
         }
     }
 }
@@ -77,7 +77,7 @@ impl FromStr for Height {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Error> {
-        Ok(s.parse::<u64>().map_err(|_| ErrorKind::Parse)?.into())
+        Ok(s.parse::<u64>().map_err(|_| Kind::Parse)?.into())
     }
 }
 

tendermint/src/chain/id.rs

@@ -1,6 +1,6 @@
 //! Tendermint blockchain identifiers
 
-use crate::error::{Error, ErrorKind};
+use crate::error::{Error, Kind};
 use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
     cmp::Ordering,
@@ -66,13 +66,13 @@ impl FromStr for Id {
     /// Parses string to create a new chain ID
     fn from_str(name: &str) -> Result<Self, Error> {
         if name.is_empty() || name.len() > MAX_LENGTH {
-            return Err(ErrorKind::Length.into());
+            return Err(Kind::Length.into());
         }
 
         for byte in name.as_bytes() {
             match byte {
                 b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'-' | b'_' => (),
-                _ => return Err(ErrorKind::Parse.into()),
+                _ => return Err(Kind::Parse.into()),
             }
         }
 
@@ -146,15 +146,18 @@ mod tests {
 
     #[test]
     fn rejects_empty_chain_ids() {
-        assert_eq!(*"".parse::<Id>().err().unwrap().kind(), ErrorKind::Length);
+        assert_eq!(
+            *"".parse::<Id>().unwrap_err().to_string(),
+            Kind::Length.to_string()
+        );
     }
 
     #[test]
     fn rejects_overlength_chain_ids() {
         let overlong_id = String::from_utf8(vec![b'x'; MAX_LENGTH + 1]).unwrap();
         assert_eq!(
-            *overlong_id.parse::<Id>().err().unwrap().kind(),
-            ErrorKind::Length
+            *overlong_id.parse::<Id>().unwrap_err().to_string(),
+            Kind::Length.to_string()
         );
     }
 }