From 1b5abc7508ce25b48ce45aab6110b2935f13782a Mon Sep 17 00:00:00 2001 From: James Wilson Date: Fri, 28 Jul 2023 16:12:36 +0100 Subject: [PATCH 1/5] derive_more, Display+Debug, add missed impl Error's --- CHANGELOG.md | 5 ++ scale-decode/Cargo.toml | 1 + scale-decode/src/error/mod.rs | 90 +++++++++--------------- scale-decode/src/visitor/mod.rs | 119 ++++++++++---------------------- 4 files changed, 75 insertions(+), 140 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a87fde..8629a69 100644 --- a/CHANGELOG.md +++ b/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`. + ## 0.8.0 - Add support for `no_std` (+alloc) builds ([#26](https://github.com/paritytech/scale-decode/pull/26)). Thankyou @haerdib! diff --git a/scale-decode/Cargo.toml b/scale-decode/Cargo.toml index 8d57486..b8482b7 100644 --- a/scale-decode/Cargo.toml +++ b/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"] } diff --git a/scale-decode/src/error/mod.rs b/scale-decode/src/error/mod.rs index 7a810f6..f45b806 100644 --- a/scale-decode/src/error/mod.rs +++ b/scale-decode/src/error/mod.rs @@ -29,14 +29,26 @@ 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) -> 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)))) } /// Retrieve more information about what went wrong. pub fn kind(&self) -> &ErrorKind { @@ -83,17 +95,21 @@ impl From 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, @@ -101,6 +117,7 @@ pub enum ErrorKind { 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, @@ -108,82 +125,41 @@ pub enum ErrorKind { 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 for ErrorKind { - fn from(err: DecodeError) -> ErrorKind { - ErrorKind::VisitorDecodeError(err) - } -} - -impl From 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), } +/// 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; +impl CustomError for T {} +/// Anything implementing this trait can be used in [`ErrorKind::Custom`]. #[cfg(not(feature = "std"))] -type CustomError = Box; +pub trait CustomError: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static {} +#[cfg(not(feature = "std"))] +impl 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 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: diff --git a/scale-decode/src/visitor/mod.rs b/scale-decode/src/visitor/mod.rs index 2815f70..1f6fc17 100644 --- a/scale-decode/src/visitor/mod.rs +++ b/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), /// 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), /// 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 for DecodeError { - fn from(err: codec::Error) -> Self { - Self::CodecError(err) - } -} - -impl From for DecodeError { - fn from(err: BitSequenceError) -> Self { - Self::BitSequenceError(err) - } -} - -impl From 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 { /// Skip any manual decoding and return the visitor instead. From 5ced630839d229b4c911489c4f808556df27e773 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Fri, 28 Jul 2023 16:13:20 +0100 Subject: [PATCH 2/5] bump to 0.9 --- Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d32da08..6ae96e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ members = [ ] [workspace.package] -version = "0.8.0" +version = "0.9.0" authors = ["Parity Technologies "] 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" } From 0e80ca098b6cb0af4db6a017af68d43a12b12ced Mon Sep 17 00:00:00 2001 From: James Wilson Date: Fri, 28 Jul 2023 16:25:25 +0100 Subject: [PATCH 3/5] Add Error::custom_string, too --- scale-decode/src/error/mod.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/scale-decode/src/error/mod.rs b/scale-decode/src/error/mod.rs index f45b806..f8672fc 100644 --- a/scale-decode/src/error/mod.rs +++ b/scale-decode/src/error/mod.rs @@ -50,6 +50,15 @@ impl Error { 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 { &self.kind @@ -117,7 +126,9 @@ pub enum ErrorKind { 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}")] + #[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, @@ -140,13 +151,13 @@ pub enum ErrorKind { #[cfg(feature = "std")] pub trait CustomError: std::error::Error + Send + Sync + 'static {} #[cfg(feature = "std")] -impl CustomError for T {} +impl CustomError for T {} /// Anything implementing this trait can be used in [`ErrorKind::Custom`]. #[cfg(not(feature = "std"))] pub trait CustomError: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static {} #[cfg(not(feature = "std"))] -impl CustomError for T {} +impl CustomError for T {} #[cfg(test)] mod test { From e70634dba33d422d22e66e256dec71a8755eddae Mon Sep 17 00:00:00 2001 From: James Wilson Date: Mon, 31 Jul 2023 09:57:08 +0100 Subject: [PATCH 4/5] Surround values in single quotes for clarity Co-authored-by: Squirrel --- scale-decode/src/error/mod.rs | 2 +- scale-decode/src/visitor/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scale-decode/src/error/mod.rs b/scale-decode/src/error/mod.rs index f8672fc..ebb90f6 100644 --- a/scale-decode/src/error/mod.rs +++ b/scale-decode/src/error/mod.rs @@ -112,7 +112,7 @@ pub enum ErrorKind { #[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")] + #[display(fmt = "Number '{value}' is out of range")] NumberOutOfRange { /// A string representation of the numeric value that was out of range. value: String, diff --git a/scale-decode/src/visitor/mod.rs b/scale-decode/src/visitor/mod.rs index 1f6fc17..ac8b72e 100644 --- a/scale-decode/src/visitor/mod.rs +++ b/scale-decode/src/visitor/mod.rs @@ -289,7 +289,7 @@ pub enum DecodeError { CannotDecodeCompactIntoType(scale_info::Type), /// Failure to decode bytes into a string. #[from] - #[display(fmt = "Could not decode string: {_0}")] + #[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")] From a42a45d8dce9bac3f19f51af2e716cd1b4e506c9 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Mon, 31 Jul 2023 10:19:37 +0100 Subject: [PATCH 5/5] Undo single quotes; values aren't "arbitrary" enough to need them. --- scale-decode/src/error/mod.rs | 2 +- scale-decode/src/visitor/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scale-decode/src/error/mod.rs b/scale-decode/src/error/mod.rs index ebb90f6..f8672fc 100644 --- a/scale-decode/src/error/mod.rs +++ b/scale-decode/src/error/mod.rs @@ -112,7 +112,7 @@ pub enum ErrorKind { #[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")] + #[display(fmt = "Number {value} is out of range")] NumberOutOfRange { /// A string representation of the numeric value that was out of range. value: String, diff --git a/scale-decode/src/visitor/mod.rs b/scale-decode/src/visitor/mod.rs index ac8b72e..1f6fc17 100644 --- a/scale-decode/src/visitor/mod.rs +++ b/scale-decode/src/visitor/mod.rs @@ -289,7 +289,7 @@ pub enum DecodeError { CannotDecodeCompactIntoType(scale_info::Type), /// Failure to decode bytes into a string. #[from] - #[display(fmt = "Could not decode string: '{_0}'")] + #[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")]