Implement chaining error (#242)

Co-authored-by: Andronik Ordian <write@reusable.software>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

.gitlab-ci.yml

@@ -61,6 +61,13 @@ check-rust-stable-no_derive_no_std:
     - time cargo +stable check --verbose --no-default-features --features bit-vec,generic-array
     - sccache -s
 
+check-rust-stable-no_std-chain-error:
+  stage:                           check
+  <<:                              *docker-env
+  script:
+    - time cargo +stable check --verbose --no-default-features --features chain-error
+    - sccache -s
+
 check-rust-stable-no_derive_full:
   stage:                           check
   <<:                              *docker-env

Cargo.toml

@@ -33,10 +33,14 @@ bench = false
 [features]
 default = ["std"]
 derive = ["parity-scale-codec-derive"]
-std = ["serde", "bitvec/std", "byte-slice-cast/std"]
+std = ["serde", "bitvec/std", "byte-slice-cast/std", "chain-error"]
 bit-vec = ["bitvec"]
 fuzz = ["std", "arbitrary"]
 
+# Make error fully descriptive with chaining error message.
+# Should not be used in a constrained environment.
+chain-error = []
+
 # WARNING: DO _NOT_ USE THIS FEATURE IF YOU ARE WORKING ON CONSENSUS CODE!*
 #
 # Provides implementations for more data structures than just Vec and Box.

derive/src/decode.rs

@@ -25,6 +25,7 @@ pub fn quote(data: &Data, type_name: &Ident, input: &TokenStream) -> TokenStream
 		Data::Struct(ref data) => match data.fields {
 			Fields::Named(_) | Fields::Unnamed(_) => create_instance(
 				quote! { #type_name },
+				&type_name.to_string(),
 				input,
 				&data.fields,
 			),
@@ -50,6 +51,7 @@ pub fn quote(data: &Data, type_name: &Ident, input: &TokenStream) -> TokenStream
 
 				let create = create_instance(
 					quote! { #type_name :: #name },
+					&format!("{}::{}", type_name, name),
 					input,
 					&v.fields,
 				);
@@ -61,11 +63,20 @@ pub fn quote(data: &Data, type_name: &Ident, input: &TokenStream) -> TokenStream
 				}
 			});
 
-			let err_msg = format!("No such variant in enum {}", type_name);
+			let read_byte_err_msg = format!(
+				"Could not decode `{}`, failed to read variant byte",
+				type_name,
+			);
+			let invalid_variant_err_msg = format!(
+				"Could not decode `{}`, variant doesn't exist",
+				type_name,
+			);
 			quote! {
-				match #input.read_byte()? {
+				match #input.read_byte()
+					.map_err(|e| e.chain(#read_byte_err_msg))?
+				{
 					#( #recurse )*
-					_ => Err(#err_msg.into()),
+					_ => Err(#invalid_variant_err_msg.into()),
 				}
 			}
 
@@ -88,7 +99,7 @@ fn create_decode_expr(field: &Field, name: &str, input: &TokenStream) -> TokenSt
 		).to_compile_error();
 	}
 
-	let err_msg = format!("Error decoding field {}", name);
+	let err_msg = format!("Could not decode `{}`", name);
 
 	if compact {
 		let field_type = &field.ty;
@@ -98,7 +109,7 @@ fn create_decode_expr(field: &Field, name: &str, input: &TokenStream) -> TokenSt
 					<#field_type as _parity_scale_codec::HasCompact>::Type as _parity_scale_codec::Decode
 				>::decode(#input);
 				match #res {
-					Err(_) => return Err(#err_msg.into()),
+					Err(e) => return Err(e.chain(#err_msg)),
 					Ok(#res) => #res.into(),
 				}
 			}
@@ -108,7 +119,7 @@ fn create_decode_expr(field: &Field, name: &str, input: &TokenStream) -> TokenSt
 			{
 				let #res = <#encoded_as as _parity_scale_codec::Decode>::decode(#input);
 				match #res {
-					Err(_) => return Err(#err_msg.into()),
+					Err(e) => return Err(e.chain(#err_msg)),
 					Ok(#res) => #res.into(),
 				}
 			}
@@ -120,7 +131,7 @@ fn create_decode_expr(field: &Field, name: &str, input: &TokenStream) -> TokenSt
 			{
 				let #res = _parity_scale_codec::Decode::decode(#input);
 				match #res {
-					Err(_) => return Err(#err_msg.into()),
+					Err(e) => return Err(e.chain(#err_msg)),
 					Ok(#res) => #res,
 				}
 			}
@@ -130,6 +141,7 @@ fn create_decode_expr(field: &Field, name: &str, input: &TokenStream) -> TokenSt
 
 fn create_instance(
 	name: TokenStream,
+	name_str: &str,
 	input: &TokenStream,
 	fields: &Fields
 ) -> TokenStream {
@@ -137,11 +149,11 @@ fn create_instance(
 		Fields::Named(ref fields) => {
 			let recurse = fields.named.iter().map(|f| {
 				let name_ident = &f.ident;
-				let field = match name_ident {
-					Some(a) => format!("{}.{}", name, a),
-					None => format!("{}", name),
+				let field_name = match name_ident {
+					Some(a) => format!("{}::{}", name_str, a),
+					None => format!("{}", name_str), // Should never happen, fields are named.
 				};
-				let decode = create_decode_expr(f, &field, input);
+				let decode = create_decode_expr(f, &field_name, input);
 
 				quote_spanned! { f.span() =>
 					#name_ident: #decode
@@ -156,9 +168,9 @@ fn create_instance(
 		},
 		Fields::Unnamed(ref fields) => {
 			let recurse = fields.unnamed.iter().enumerate().map(|(i, f) | {
-				let name = format!("{}.{}", name, i);
+				let field_name = format!("{}.{}", name_str, i);
 
-				create_decode_expr(f, &name, input)
+				create_decode_expr(f, &field_name, input)
 			});
 
 			quote_spanned! { fields.span() =>

src/bit_vec.rs

@@ -17,9 +17,10 @@
 use bitvec::{
 	vec::BitVec, store::BitStore, order::BitOrder, slice::BitSlice, boxed::BitBox, mem::BitMemory
 };
-use crate::codec::{Encode, Decode, Input, Output, Error, decode_vec_with_len, encode_slice_no_len};
-use crate::compact::Compact;
-use crate::EncodeLike;
+use crate::{
+	EncodeLike, Encode, Decode, Input, Output, Error, Compact,
+	codec::{decode_vec_with_len, encode_slice_no_len},
+};
 
 impl<O: BitOrder, T: BitStore + Encode> Encode for BitSlice<O, T> {
 	fn encode_to<W: Output + ?Sized>(&self, dest: &mut W) {

src/codec.rs

@@ -48,73 +48,11 @@ use crate::alloc::{
 };
 use crate::compact::Compact;
 use crate::encode_like::EncodeLike;
+use crate::Error;
 
 pub(crate) const MAX_PREALLOCATION: usize = 4 * 1024;
 const A_BILLION: u32 = 1_000_000_000;
 
-/// Descriptive error type
-#[cfg(feature = "std")]
-#[derive(PartialEq, Eq, Clone, Debug)]
-pub struct Error(&'static str);
-
-/// Undescriptive error type when compiled for no std
-#[cfg(not(feature = "std"))]
-#[derive(PartialEq, Eq, Clone, Debug)]
-pub struct Error;
-
-impl Error {
-	#[cfg(feature = "std")]
-	/// Error description
-	///
-	/// This function returns an actual error str when running in `std`
-	/// environment, but `""` on `no_std`.
-	pub fn what(&self) -> &'static str {
-		self.0
-	}
-
-	#[cfg(not(feature = "std"))]
-	/// Error description
-	///
-	/// This function returns an actual error str when running in `std`
-	/// environment, but `""` on `no_std`.
-	pub fn what(&self) -> &'static str {
-		""
-	}
-}
-
-#[cfg(feature = "std")]
-impl fmt::Display for Error {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		write!(f, "{}", self.0)
-	}
-}
-
-#[cfg(not(feature = "std"))]
-impl fmt::Display for Error {
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		f.write_str("Error")
-	}
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for Error {
-	fn description(&self) -> &str {
-		self.0
-	}
-}
-
-impl From<&'static str> for Error {
-	#[cfg(feature = "std")]
-	fn from(s: &'static str) -> Error {
-		Error(s)
-	}
-
-	#[cfg(not(feature = "std"))]
-	fn from(_s: &'static str) -> Error {
-		Error
-	}
-}
-
 /// Trait that allows reading of data into a slice.
 pub trait Input {
 	/// Should return the remaining length of the input data. If no information about the input
@@ -508,9 +446,15 @@ where
 
 impl<T: Decode, E: Decode> Decode for Result<T, E> {
 	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
-		match input.read_byte()? {
-			0 => Ok(Ok(T::decode(input)?)),
-			1 => Ok(Err(E::decode(input)?)),
+		match input.read_byte()
+			.map_err(|e| e.chain("Could not result variant byte for `Result`"))?
+		{
+			0 => Ok(
+				Ok(T::decode(input).map_err(|e| e.chain("Could not Decode `Result::Ok(T)`"))?)
+			),
+			1 => Ok(
+				Err(E::decode(input).map_err(|e| e.chain("Could not decode `Result::Error(E)`"))?)
+			),
 			_ => Err("unexpected first byte decoding Result".into()),
 		}
 	}
@@ -576,9 +520,13 @@ impl<T: Encode> Encode for Option<T> {
 
 impl<T: Decode> Decode for Option<T> {
 	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
-		match input.read_byte()? {
+		match input.read_byte()
+			.map_err(|e| e.chain("Could not decode variant byte for `Option`"))?
+		{
 			0 => Ok(None),
-			1 => Ok(Some(T::decode(input)?)),
+			1 => Ok(
+				Some(T::decode(input).map_err(|e| e.chain("Could not decode `Option::Some(T)`"))?)
+			),
 			_ => Err("unexpected first byte decoding Option".into()),
 		}
 	}
@@ -1242,9 +1190,10 @@ impl Encode for Duration {
 
 impl Decode for Duration {
 	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
-		let (secs, nanos) = <(u64, u32)>::decode(input)?;
+		let (secs, nanos) = <(u64, u32)>::decode(input)
+			.map_err(|e| e.chain("Could not decode `Duration(u64, u32)`"))?;
 		if nanos >= A_BILLION {
-			Err("Number of nanoseconds should not be higher than 10^9.".into())
+			Err("Could not decode `Duration`: Number of nanoseconds should not be higher than 10^9.".into())
 		} else {
 			Ok(Duration::new(secs, nanos))
 		}
@@ -1508,10 +1457,16 @@ mod tests {
 		assert_eq!(<Vec<u8>>::decode(&mut NoLimit(&i[..])).unwrap(), vec![0u8; len]);
 
 		let i = Compact(len as u32).encode();
-		assert_eq!(<Vec<u8>>::decode(&mut NoLimit(&i[..])).err().unwrap().what(), "Not enough data to fill buffer");
+		assert_eq!(
+			<Vec<u8>>::decode(&mut NoLimit(&i[..])).err().unwrap().to_string(),
+			"Not enough data to fill buffer",
+		);
 
 		let i = Compact(1000u32).encode();
-		assert_eq!(<Vec<u8>>::decode(&mut NoLimit(&i[..])).err().unwrap().what(), "Not enough data to fill buffer");
+		assert_eq!(
+			<Vec<u8>>::decode(&mut NoLimit(&i[..])).err().unwrap().to_string(),
+			"Not enough data to fill buffer",
+		);
 	}
 
 	#[test]

src/compact.rs

@@ -17,8 +17,9 @@
 use arrayvec::ArrayVec;
 
 use crate::alloc::vec::Vec;
-use crate::codec::{Encode, Decode, Input, Output, EncodeAsRef, Error};
+use crate::codec::{Encode, Decode, Input, Output, EncodeAsRef};
 use crate::encode_like::EncodeLike;
+use crate::Error;
 #[cfg(feature = "fuzz")]
 use arbitrary::Arbitrary;
 

src/decode_all.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::codec::{Error, Decode};
+use crate::{Error, Decode};
 
 /// The error message returned when `decode_all` fails.
 pub(crate) const DECODE_ALL_ERR_MSG: &str = "Input buffer has still data left after decoding!";
@@ -56,7 +56,10 @@ mod tests {
 					);
 
 					encoded.extend(&[1, 2, 3, 4, 5, 6]);
-					assert_eq!(<$type>::decode_all(&encoded).unwrap_err().what(), DECODE_ALL_ERR_MSG);
+					assert_eq!(
+						<$type>::decode_all(&encoded).unwrap_err().to_string(),
+						"Input buffer has still data left after decoding!",
+					);
 				}
 			)*
 		};

src/depth_limit.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::codec::{Error, Decode, Input};
+use crate::{Error, Decode, Input};
 
 /// The error message returned when depth limit is reached.
 const DECODE_MAX_DEPTH_MSG: &str = "Maximum recursion depth reached when decoding";

src/encode_append.rs

@@ -15,7 +15,7 @@
 use core::{iter::ExactSizeIterator, mem};
 
 use crate::alloc::vec::Vec;
-use crate::codec::{Encode, Decode, Error};
+use crate::{Encode, Decode, Error};
 use crate::compact::{Compact, CompactLen};
 use crate::encode_like::EncodeLike;
 

src/error.rs

+// Copyright 2021 Parity Technologies
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Error type, descriptive or undescriptive depending on features.
+
+use crate::alloc::borrow::Cow;
+#[cfg(feature = "chain-error")]
+use crate::alloc::boxed::Box;
+
+/// Error type.
+///
+/// Descriptive on `std` environment, with chaining error on `chain-error` environment,
+/// underscriptive otherwise.
+#[derive(PartialEq, Eq, Clone, Debug)]
+pub struct Error {
+	#[cfg(feature = "chain-error")]
+	cause: Option<Box<Error>>,
+	#[cfg(feature = "chain-error")]
+	desc: Cow<'static, str>,
+}
+
+impl Error {
+	/// Chain error message with description.
+	///
+	/// When compiled with `chain-error` feature, the description is chained, otherwise the
+	/// description is ditched.
+	pub fn chain(self, desc: impl Into<Cow<'static, str>>) -> Self {
+		#[cfg(feature = "chain-error")]
+		{
+			Self { desc: desc.into(), cause: Some(Box::new(self)) }
+		}
+
+		#[cfg(not(feature = "chain-error"))]
+		{
+			let _ = desc;
+			self
+		}
+	}
+
+	/// Display error with indentation.
+	#[cfg(feature = "chain-error")]
+	fn display_with_indent(&self, indent: u32, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+		for _ in 0..indent {
+			f.write_str("\t")?;
+		}
+		f.write_str(&self.desc)?;
+		if let Some(cause) = &self.cause {
+			f.write_str(":")?;
+			f.write_str("\n")?;
+			cause.display_with_indent(indent + 1, f)
+		} else {
+			// Only return to new line if the error has been displayed with some indent,
+			// i.e. if the error has some causes.
+			if indent != 0 {
+				f.write_str("\n")?;
+			}
+			Ok(())
+		}
+	}
+}
+
+impl core::fmt::Display for Error {
+	fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
+		#[cfg(feature = "chain-error")]
+		{
+			self.display_with_indent(0, f)
+		}
+
+		#[cfg(not(feature = "chain-error"))]
+		{
+			f.write_str("Codec error")
+		}
+	}
+}
+
+impl From<&'static str> for Error {
+	fn from(desc: &'static str) -> Error {
+		#[cfg(feature = "chain-error")]
+		{
+			Error { desc: desc.into(), cause: None }
+		}
+
+		#[cfg(not(feature = "chain-error"))]
+		{
+			let _ = desc;
+			Error {}
+		}
+	}
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for Error {
+	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+		#[cfg(feature = "chain-error")]
+		{
+			self.cause.as_ref().map(|e| e as &(dyn std::error::Error + 'static))
+		}
+
+		#[cfg(not(feature = "chain-error"))]
+		{
+			None
+		}
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use crate::Error;
+
+	#[test]
+	fn test_full_error() {
+		let msg: &str = "final type:\n\twrap cause:\n\t\troot cause\n";
+
+		let error = Error::from("root cause").chain("wrap cause").chain("final type");
+
+		assert_eq!(&error.to_string(), msg);
+	}
+
+	#[test]
+	fn impl_std_error() {
+		use std::error::Error as _;
+
+		let error = Error::from("root cause").chain("wrap cause").chain("final type");
+		let s = error.source().unwrap();
+
+		assert_eq!(&s.to_string(), "wrap cause:\n\troot cause\n");
+	}
+}

src/generic_array.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use crate::alloc::vec::Vec;
-use crate::codec::{Encode, Decode, Input, Output, Error};
+use crate::{Encode, Decode, Input, Output, Error};
 use crate::encode_like::EncodeLike;