Improve error handling + consistency when no_std

CHANGELOG.md

@@ -4,6 +4,11 @@ The format is based on [Keep a Changelog].
 
 [Keep a Changelog]: http://keepachangelog.com/en/1.0.0/
 
+## 0.9.0 - 2023-07-28
+
+- Improve custom error handling: custom errors now require `Debug + Display` on `no_std` or `Error` on `std`.
+  `Error::custom()` now accepts anything implementing these traits rather than depending on `Into<Error>`.
+
 ## 0.8.0
 
 - Add support for `no_std` (+alloc) builds ([#26](https://github.com/paritytech/scale-decode/pull/26)). Thankyou @haerdib!

Cargo.toml

@@ -6,7 +6,7 @@ members = [
 ]
 
 [workspace.package]
-version = "0.8.0"
+version = "0.9.0"
 authors = ["Parity Technologies <[email protected]>"]
 edition = "2021"
 license = "Apache-2.0"
@@ -16,5 +16,5 @@ keywords = ["parity", "scale", "decoding"]
 include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"]
 
 [workspace.dependencies]
-scale-decode = { version = "0.8.0", path = "scale-decode" }
-scale-decode-derive = { version = "0.8.0", path = "scale-decode-derive" }
+scale-decode = { version = "0.9.0", path = "scale-decode" }
+scale-decode-derive = { version = "0.9.0", path = "scale-decode-derive" }

scale-decode/Cargo.toml

@@ -32,6 +32,7 @@ scale-bits = { version = "0.4.0", default-features = false, features = ["scale-i
 scale-decode-derive = { workspace = true, optional = true }
 primitive-types = { version = "0.12.0", optional = true, default-features = false }
 smallvec = "1.10.0"
+derive_more = { version = "0.99.17", default-features = false, features = ["from", "display"] }
 
 [dev-dependencies]
 scale-info = { version = "2.7.0", default-features = false, features = ["bit-vec", "derive"] }

scale-decode/src/error/mod.rs

@@ -29,14 +29,35 @@ pub struct Error {
     kind: ErrorKind,
 }
 
+#[cfg(feature = "std")]
+impl std::error::Error for Error {}
+
 impl Error {
     /// Construct a new error given an error kind.
     pub fn new(kind: ErrorKind) -> Error {
         Error { context: Context::new(), kind }
     }
     /// Construct a new, custom error.
-    pub fn custom(error: impl Into<CustomError>) -> Error {
-        Error::new(ErrorKind::Custom(error.into()))
+    pub fn custom(error: impl CustomError) -> Error {
+        Error::new(ErrorKind::Custom(Box::new(error)))
+    }
+    /// Construct a custom error from a static string.
+    pub fn custom_str(error: &'static str) -> Error {
+        #[derive(derive_more::Display, Debug)]
+        pub struct StrError(pub &'static str);
+        #[cfg(feature = "std")]
+        impl std::error::Error for StrError {}
+
+        Error::new(ErrorKind::Custom(Box::new(StrError(error))))
+    }
+    /// Construct a custom error from an owned string.
+    pub fn custom_string(error: String) -> Error {
+        #[derive(derive_more::Display, Debug)]
+        pub struct StringError(String);
+        #[cfg(feature = "std")]
+        impl std::error::Error for StringError {}
+
+        Error::new(ErrorKind::Custom(Box::new(StringError(error))))
     }
     /// Retrieve more information about what went wrong.
     pub fn kind(&self) -> &ErrorKind {
@@ -83,107 +104,73 @@ impl From<DecodeError> for Error {
 }
 
 /// The underlying nature of the error.
-#[derive(Debug)]
+#[derive(Debug, derive_more::From, derive_more::Display)]
 pub enum ErrorKind {
     /// Something went wrong decoding the bytes based on the type
     /// and type registry provided.
+    #[from]
+    #[display(fmt = "Error decoding bytes given the type ID and registry provided: {_0}")]
     VisitorDecodeError(DecodeError),
     /// We cannot decode the number seen into the target type; it's out of range.
+    #[display(fmt = "Number {value} is out of range")]
     NumberOutOfRange {
         /// A string representation of the numeric value that was out of range.
         value: String,
     },
     /// We cannot find the variant we're trying to decode from in the target type.
+    #[display(fmt = "Cannot find variant {got}; expects one of {expected:?}")]
     CannotFindVariant {
         /// The variant that we are given back from the encoded bytes.
         got: String,
         /// The possible variants that we can decode into.
         expected: Vec<&'static str>,
     },
     /// The types line up, but the expected length of the target type is different from the length of the input value.
+    #[display(
+        fmt = "Cannot decode from type; expected length {expected_len} but got length {actual_len}"
+    )]
     WrongLength {
         /// Length of the type we are trying to decode from
         actual_len: usize,
         /// Length fo the type we're trying to decode into
         expected_len: usize,
     },
     /// Cannot find a field that we need to decode to our target type
+    #[display(fmt = "Field {name} does not exist in our encoded data")]
     CannotFindField {
         /// Name of the field which was not provided.
         name: String,
     },
     /// A custom error.
-    Custom(CustomError),
-}
-
-impl From<DecodeError> for ErrorKind {
-    fn from(err: DecodeError) -> ErrorKind {
-        ErrorKind::VisitorDecodeError(err)
-    }
-}
-
-impl From<CustomError> for ErrorKind {
-    fn from(err: CustomError) -> ErrorKind {
-        ErrorKind::Custom(err)
-    }
-}
-
-impl Display for ErrorKind {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            ErrorKind::VisitorDecodeError(decode_error) => {
-                write!(f, "Error decoding bytes given the type ID and registry provided: {decode_error:?}")
-            }
-            ErrorKind::NumberOutOfRange { value } => {
-                write!(f, "Number {value} is out of range")
-            }
-            ErrorKind::CannotFindVariant { got, expected } => {
-                write!(f, "Cannot find variant {got}; expects one of {expected:?}")
-            }
-            ErrorKind::WrongLength { actual_len, expected_len } => {
-                write!(f, "Cannot decode from type; expected length {expected_len} but got length {actual_len}")
-            }
-            ErrorKind::CannotFindField { name } => {
-                write!(f, "Field {name} does not exist in our encoded data")
-            }
-            ErrorKind::Custom(custom_error) => {
-                write!(f, "Custom error: {custom_error:?}")
-            }
-        }
-    }
+    #[from]
+    #[display(fmt = "Custom error: {_0}")]
+    Custom(Box<dyn CustomError>),
 }
 
+/// Anything implementing this trait can be used in [`ErrorKind::Custom`].
+#[cfg(feature = "std")]
+pub trait CustomError: std::error::Error + Send + Sync + 'static {}
 #[cfg(feature = "std")]
-type CustomError = Box<dyn std::error::Error + Send + Sync + 'static>;
+impl<T: std::error::Error + Send + Sync + 'static> CustomError for T {}
 
+/// Anything implementing this trait can be used in [`ErrorKind::Custom`].
 #[cfg(not(feature = "std"))]
-type CustomError = Box<dyn core::fmt::Debug + Send + Sync + 'static>;
+pub trait CustomError: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static {}
+#[cfg(not(feature = "std"))]
+impl<T: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static> CustomError for T {}
 
 #[cfg(test)]
 mod test {
     use super::*;
 
-    #[derive(Debug)]
+    #[derive(Debug, derive_more::Display)]
     enum MyError {
         Foo,
     }
 
-    impl Display for MyError {
-        fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-            write!(f, "{self:?}")
-        }
-    }
-
     #[cfg(feature = "std")]
     impl std::error::Error for MyError {}
 
-    #[cfg(not(feature = "std"))]
-    impl Into<CustomError> for MyError {
-        fn into(self) -> CustomError {
-            Box::new(self)
-        }
-    }
-
     #[test]
     fn custom_error() {
         // Just a compile-time check that we can ergonomically provide an arbitrary custom error:

scale-decode/src/visitor/mod.rs

@@ -18,7 +18,6 @@
 mod decode;
 pub mod types;
 
-use core::fmt::Display;
 use scale_info::form::PortableForm;
 use types::*;
 
@@ -279,136 +278,90 @@ pub trait Visitor: Sized {
 }
 
 /// An error decoding SCALE bytes.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, derive_more::From, derive_more::Display)]
 pub enum DecodeError {
     /// We ran into an error trying to decode a bit sequence.
+    #[from]
+    #[display(fmt = "Cannot decode bit sequence: {_0}")]
     BitSequenceError(BitSequenceError),
     /// The type we're trying to decode is supposed to be compact encoded, but that is not possible.
+    #[display(fmt = "Could not decode compact encoded type into {_0:?}")]
     CannotDecodeCompactIntoType(scale_info::Type<PortableForm>),
     /// Failure to decode bytes into a string.
+    #[from]
+    #[display(fmt = "Could not decode string: {_0}")]
     InvalidStr(alloc::str::Utf8Error),
     /// We could not convert the [`u32`] that we found into a valid [`char`].
+    #[display(fmt = "{_0} is expected to be a valid char, but is not")]
     InvalidChar(u32),
     /// We expected more bytes to finish decoding, but could not find them.
+    #[display(fmt = "Ran out of data during decoding")]
     NotEnoughInput,
     /// We found a variant that does not match with any in the type we're trying to decode from.
+    #[display(fmt = "Could not find variant with index {_0} in {_1:?}")]
     VariantNotFound(u8, scale_info::TypeDefVariant<PortableForm>),
     /// Some error emitted from a [`codec::Decode`] impl.
+    #[from]
     CodecError(codec::Error),
     /// We could not find the type given in the type registry provided.
+    #[display(fmt = "Cannot find type with ID {_0}")]
     TypeIdNotFound(u32),
     /// This is returned by default if a visitor function is not implemented.
+    #[display(fmt = "Unexpected type {_0}")]
     Unexpected(Unexpected),
 }
 
-impl From<codec::Error> for DecodeError {
-    fn from(err: codec::Error) -> Self {
-        Self::CodecError(err)
-    }
-}
-
-impl From<BitSequenceError> for DecodeError {
-    fn from(err: BitSequenceError) -> Self {
-        Self::BitSequenceError(err)
-    }
-}
-
-impl From<alloc::str::Utf8Error> for DecodeError {
-    fn from(err: alloc::str::Utf8Error) -> Self {
-        Self::InvalidStr(err)
-    }
-}
-
-impl Display for DecodeError {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        match self {
-            DecodeError::BitSequenceError(error) => {
-                write!(f, "Cannot decode bit sequence: {error:?}")
-            }
-            DecodeError::CannotDecodeCompactIntoType(portable_type) => {
-                write!(f, "Could not decode compact encoded type into {portable_type:?}")
-            }
-            DecodeError::InvalidStr(error) => {
-                write!(f, "Could not decode string: {error:?}")
-            }
-            DecodeError::InvalidChar(char) => {
-                write!(f, "{char} is expected to be a valid char, but is not")
-            }
-            DecodeError::NotEnoughInput => {
-                write!(f, "Ran out of data during decoding")
-            }
-            DecodeError::VariantNotFound(index, type_def_variant) => {
-                write!(f, "Could not find variant with index {index} in {type_def_variant:?}")
-            }
-            DecodeError::CodecError(error) => {
-                write!(f, "{error}")
-            }
-            DecodeError::TypeIdNotFound(id) => {
-                write!(f, "Cannot find type with ID {id}")
-            }
-            DecodeError::Unexpected(unexpected) => {
-                write!(f, "Unexpected type {unexpected}")
-            }
-        }
-    }
-}
+#[cfg(feature = "std")]
+impl std::error::Error for DecodeError {}
 
 /// This is returned by default when a visitor function isn't implemented.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, derive_more::Display)]
 #[allow(missing_docs)]
 pub enum Unexpected {
+    #[display(fmt = "bool")]
     Bool,
+    #[display(fmt = "char")]
     Char,
+    #[display(fmt = "u8")]
     U8,
+    #[display(fmt = "u16")]
     U16,
+    #[display(fmt = "u32")]
     U32,
+    #[display(fmt = "u64")]
     U64,
+    #[display(fmt = "u128")]
     U128,
+    #[display(fmt = "u256")]
     U256,
+    #[display(fmt = "i8")]
     I8,
+    #[display(fmt = "i16")]
     I16,
+    #[display(fmt = "i32")]
     I32,
+    #[display(fmt = "i64")]
     I64,
+    #[display(fmt = "i128")]
     I128,
+    #[display(fmt = "i256")]
     I256,
+    #[display(fmt = "sequence")]
     Sequence,
+    #[display(fmt = "composite")]
     Composite,
+    #[display(fmt = "tuple")]
     Tuple,
+    #[display(fmt = "str")]
     Str,
+    #[display(fmt = "variant")]
     Variant,
+    #[display(fmt = "array")]
     Array,
+    #[display(fmt = "bitsequence")]
     Bitsequence,
 }
 
-impl core::fmt::Display for Unexpected {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let s = match self {
-            Unexpected::Bool => "bool",
-            Unexpected::Char => "char",
-            Unexpected::U8 => "u8",
-            Unexpected::U16 => "u16",
-            Unexpected::U32 => "u32",
-            Unexpected::U64 => "u64",
-            Unexpected::U128 => "u128",
-            Unexpected::U256 => "u256",
-            Unexpected::I8 => "i8",
-            Unexpected::I16 => "i16",
-            Unexpected::I32 => "i32",
-            Unexpected::I64 => "i64",
-            Unexpected::I128 => "i128",
-            Unexpected::I256 => "i256",
-            Unexpected::Sequence => "sequence",
-            Unexpected::Composite => "composite",
-            Unexpected::Tuple => "tuple",
-            Unexpected::Str => "str",
-            Unexpected::Variant => "variant",
-            Unexpected::Array => "array",
-            Unexpected::Bitsequence => "bitsequence",
-        };
-        f.write_str(s)
-    }
-}
-
 /// The response from [`Visitor::unchecked_decode_as_type()`].
 pub enum DecodeAsTypeResult<V, R> {
     /// Skip any manual decoding and return the visitor instead.