Simplify runtime api error handling (#8114)

* Ahh

* Work work work

* Fix all the compilation errors

* Fix test

* More fixes...

substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs

@@ -187,14 +187,15 @@ fn generate_native_call_generators(decl: &ItemTrait) -> Result<TokenStream> {
 	result.push(quote!(
 		#[cfg(any(feature = "std", test))]
 		fn convert_between_block_types
-			<I: #crate_::Encode, R: #crate_::Decode>(
-				input: &I, error_desc: &'static str,
-			) -> std::result::Result<R, String>
+			<I: #crate_::Encode, R: #crate_::Decode, F: FnOnce(#crate_::codec::Error) -> #crate_::ApiError>(
+				input: &I,
+				map_error: F,
+			) -> std::result::Result<R, #crate_::ApiError>
 		{
 			<R as #crate_::DecodeLimit>::decode_with_depth_limit(
 				#crate_::MAX_EXTRINSIC_DEPTH,
 				&mut &#crate_::Encode::encode(input)[..],
-			).map_err(|e| format!("{} {}", error_desc, e))
+			).map_err(map_error)
 		}
 	));
 
@@ -202,19 +203,26 @@ fn generate_native_call_generators(decl: &ItemTrait) -> Result<TokenStream> {
 	for fn_ in fns {
 		let params = extract_parameter_names_types_and_borrows(&fn_, AllowSelfRefInParameters::No)?;
 		let trait_fn_name = &fn_.ident;
+		let function_name_str = fn_.ident.to_string();
 		let fn_name = generate_native_call_generator_fn_name(&fn_.ident);
 		let output = return_type_replace_block_with_node_block(fn_.output.clone());
 		let output_ty = return_type_extract_type(&output);
-		let output = quote!( std::result::Result<#output_ty, String> );
+		let output = quote!( std::result::Result<#output_ty, #crate_::ApiError> );
 
 		// Every type that is using the `Block` generic parameter, we need to encode/decode,
 		// to make it compatible between the runtime/node.
 		let conversions = params.iter().filter(|v| type_is_using_block(&v.1)).map(|(n, t, _)| {
-			let name_str = format!(
-				"Could not convert parameter `{}` between node and runtime:", quote!(#n)
-			);
+			let param_name = quote!(#n).to_string();
+
 			quote!(
-				let #n: #t = convert_between_block_types(&#n, #name_str)?;
+				let #n: #t = convert_between_block_types(
+					&#n,
+					|e| #crate_::ApiError::FailedToConvertParameter {
+						function: #function_name_str,
+						parameter: #param_name,
+						error: e,
+					},
+				)?;
 			)
 		});
 		// Same as for the input types, we need to check if we also need to convert the output,
@@ -223,7 +231,10 @@ fn generate_native_call_generators(decl: &ItemTrait) -> Result<TokenStream> {
 			quote!(
 				convert_between_block_types(
 					&res,
-					"Could not convert return value from runtime to node!"
+					|e| #crate_::ApiError::FailedToConvertReturnValue {
+						function: #function_name_str,
+						error: e,
+					},
 				)
 			)
 		} else {
@@ -399,10 +410,10 @@ fn generate_call_api_at_calls(decl: &ItemTrait) -> Result<TokenStream> {
 			#[cfg(any(feature = "std", test))]
 			pub fn #fn_name<
 				R: #crate_::Encode + #crate_::Decode + PartialEq,
-				NC: FnOnce() -> std::result::Result<R, String> + std::panic::UnwindSafe,
+				NC: FnOnce() -> std::result::Result<R, #crate_::ApiError> + std::panic::UnwindSafe,
 				Block: #crate_::BlockT,
 				T: #crate_::CallApiAt<Block>,
-				C: #crate_::Core<Block, Error = T::Error>,
+				C: #crate_::Core<Block>,
 			>(
 				call_runtime_at: &T,
 				core_api: &C,
@@ -416,7 +427,7 @@ fn generate_call_api_at_calls(decl: &ItemTrait) -> Result<TokenStream> {
 				native_call: Option<NC>,
 				context: #crate_::ExecutionContext,
 				recorder: &Option<#crate_::ProofRecorder<Block>>,
-			) -> std::result::Result<#crate_::NativeOrEncoded<R>, T::Error> {
+			) -> std::result::Result<#crate_::NativeOrEncoded<R>, #crate_::ApiError> {
 				let version = call_runtime_at.runtime_version_at(at)?;
 				use #crate_::InitializeBlock;
 				let initialize_block = if #skip_initialize_block {
@@ -621,7 +632,7 @@ impl<'a> ToClientSideDecl<'a> {
 					context: #crate_::ExecutionContext,
 					params: Option<( #( #param_types ),* )>,
 					params_encoded: Vec<u8>,
-				) -> std::result::Result<#crate_::NativeOrEncoded<#ret_type>, Self::Error>;
+				) -> std::result::Result<#crate_::NativeOrEncoded<#ret_type>, #crate_::ApiError>;
 			}
 		)
 	}
@@ -647,7 +658,7 @@ impl<'a> ToClientSideDecl<'a> {
 		let params2 = params.clone();
 		let ret_type = return_type_extract_type(&method.sig.output);
 
-		fold_fn_decl_for_client_side(&mut method.sig, &self.block_id);
+		fold_fn_decl_for_client_side(&mut method.sig, &self.block_id, &self.crate_);
 
 		let name_impl = generate_method_runtime_api_impl_name(&self.trait_, &method.sig.ident);
 		let crate_ = self.crate_;
@@ -705,7 +716,12 @@ impl<'a> ToClientSideDecl<'a> {
 							},
 							#crate_::NativeOrEncoded::Encoded(r) => {
 								<#ret_type as #crate_::Decode>::decode(&mut &r[..])
-									.map_err(|err| { #crate_::ApiError::new(#function_name, err).into() })
+									.map_err(|err|
+										#crate_::ApiError::FailedToDecodeReturnValue {
+											function: #function_name,
+											error: err,
+										}
+									)
 							}
 						}
 					)
@@ -728,12 +744,10 @@ impl<'a> Fold for ToClientSideDecl<'a> {
 
 		if is_core_trait {
 			// Add all the supertraits we want to have for `Core`.
-			let crate_ = &self.crate_;
 			input.supertraits = parse_quote!(
 				'static
 				+ Send
 				+ Sync
-				+ #crate_::ApiErrorExt
 			);
 		} else {
 			// Add the `Core` runtime api as super trait.
@@ -803,12 +817,12 @@ fn generate_runtime_info_impl(trait_: &ItemTrait, version: u64) -> TokenStream {
 		let bounds = &t.bounds;
 
 		quote! { #ident #colon_token #bounds }
-	}).chain(std::iter::once(quote! { __Sr_Api_Error__ }));
+	});
 
 	let ty_generics = trait_.generics.type_params().map(|t| {
 		let ident = &t.ident;
 		quote! { #ident }
-	}).chain(std::iter::once(quote! { Error = __Sr_Api_Error__ }));
+	});
 
 	quote!(
 		#[cfg(any(feature = "std", test))]

substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs

@@ -233,16 +233,6 @@ fn generate_runtime_api_base_structures() -> Result<TokenStream> {
 					C::StateBackend: #crate_::StateBackend<#crate_::HashFor<Block>>,
 		{}
 
-		#[cfg(any(feature = "std", test))]
-		impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> #crate_::ApiErrorExt
-			for RuntimeApiImpl<Block, C>
-				where
-					// Rust bug: https://github.com/rust-lang/rust/issues/24159
-					C::StateBackend: #crate_::StateBackend<#crate_::HashFor<Block>>,
-		{
-			type Error = C::Error;
-		}
-
 		#[cfg(any(feature = "std", test))]
 		impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> #crate_::ApiExt<Block> for
 			RuntimeApiImpl<Block, C>
@@ -269,16 +259,20 @@ fn generate_runtime_api_base_structures() -> Result<TokenStream> {
 			fn has_api<A: #crate_::RuntimeApiInfo + ?Sized>(
 				&self,
 				at: &#crate_::BlockId<Block>,
-			) -> std::result::Result<bool, C::Error> where Self: Sized {
-				self.call.runtime_version_at(at).map(|v| v.has_api_with(&A::ID, |v| v == A::VERSION))
+			) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized {
+				self.call
+					.runtime_version_at(at)
+					.map(|v| v.has_api_with(&A::ID, |v| v == A::VERSION))
 			}
 
 			fn has_api_with<A: #crate_::RuntimeApiInfo + ?Sized, P: Fn(u32) -> bool>(
 				&self,
 				at: &#crate_::BlockId<Block>,
 				pred: P,
-			) -> std::result::Result<bool, C::Error> where Self: Sized {
-				self.call.runtime_version_at(at).map(|v| v.has_api_with(&A::ID, pred))
+			) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized {
+				self.call
+					.runtime_version_at(at)
+					.map(|v| v.has_api_with(&A::ID, pred))
 			}
 
 			fn record_proof(&mut self) {
@@ -306,7 +300,7 @@ fn generate_runtime_api_base_structures() -> Result<TokenStream> {
 				>>,
 				parent_hash: Block::Hash,
 			) -> std::result::Result<
-				#crate_::StorageChanges<Self::StateBackend, Block>,
+				#crate_::StorageChanges<C::StateBackend, Block>,
 				String
 			> where Self: Sized {
 				self.initialized_block.borrow_mut().take();
@@ -513,7 +507,7 @@ impl<'a> Fold for ApiRuntimeImplToApiRuntimeApiImpl<'a> {
 
 			// Generate the correct return type.
 			input.sig.output = parse_quote!(
-				-> std::result::Result<#crate_::NativeOrEncoded<#ret_type>, RuntimeApiImplCall::Error>
+				-> std::result::Result<#crate_::NativeOrEncoded<#ret_type>, #crate_::ApiError>
 			);
 
 			// Generate the new method implementation that calls into the runtime.
@@ -554,7 +548,7 @@ impl<'a> Fold for ApiRuntimeImplToApiRuntimeApiImpl<'a> {
 			)
 		};
 
-		let mut input =	fold::fold_impl_item_method(self, input);
+		let mut input = fold::fold_impl_item_method(self, input);
 		// We need to set the block, after we modified the rest of the ast, otherwise we would
 		// modify our generated block as well.
 		input.block = block;

substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs

@@ -27,7 +27,7 @@ use proc_macro2::{Span, TokenStream};
 use quote::{quote, quote_spanned};
 
 use syn::{
-	spanned::Spanned, parse_macro_input, Ident, Type, ItemImpl, ImplItem, TypePath, parse_quote,
+	spanned::Spanned, parse_macro_input, Ident, Type, ItemImpl, TypePath, parse_quote,
 	parse::{Parse, ParseStream, Result, Error}, fold::{self, Fold}, Attribute, Pat,
 };
 
@@ -61,29 +61,14 @@ impl Parse for RuntimeApiImpls {
 	}
 }
 
-/// Implement the `ApiExt` trait, `ApiErrorExt` trait and the `Core` runtime api.
+/// Implement the `ApiExt` trait and the `Core` runtime api.
 fn implement_common_api_traits(
-	error_type: Option<Type>,
 	block_type: TypePath,
 	self_ty: Type,
 ) -> Result<TokenStream> {
 	let crate_ = generate_crate_access(HIDDEN_INCLUDES_ID);
 
-	let error_type = error_type
-		.map(|e| quote!(#e))
-		.unwrap_or_else(|| quote!( #crate_::ApiError ) );
-
-	// Quote using the span from `error_type` to generate nice error messages when the type is
-	// not implementing a trait or similar.
-	let api_error_ext = quote_spanned! { error_type.span() =>
-		impl #crate_::ApiErrorExt for #self_ty {
-			type Error = #error_type;
-		}
-	};
-
 	Ok(quote!(
-		#api_error_ext
-
 		impl #crate_::ApiExt<#block_type> for #self_ty {
 			type StateBackend = #crate_::InMemoryBackend<#crate_::HashFor<#block_type>>;
 
@@ -97,7 +82,7 @@ fn implement_common_api_traits(
 			fn has_api<A: #crate_::RuntimeApiInfo + ?Sized>(
 				&self,
 				_: &#crate_::BlockId<#block_type>,
-			) -> std::result::Result<bool, #error_type> where Self: Sized {
+			) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized {
 				Ok(true)
 			}
 
@@ -105,7 +90,7 @@ fn implement_common_api_traits(
 				&self,
 				_: &#crate_::BlockId<#block_type>,
 				pred: P,
-			) -> std::result::Result<bool, #error_type> where Self: Sized {
+			) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized {
 				Ok(pred(A::VERSION))
 			}
 
@@ -140,7 +125,7 @@ fn implement_common_api_traits(
 				_: #crate_::ExecutionContext,
 				_: Option<()>,
 				_: Vec<u8>,
-			) -> std::result::Result<#crate_::NativeOrEncoded<#crate_::RuntimeVersion>, #error_type> {
+			) -> std::result::Result<#crate_::NativeOrEncoded<#crate_::RuntimeVersion>, #crate_::ApiError> {
 				unimplemented!("Not required for testing!")
 			}
 
@@ -150,7 +135,7 @@ fn implement_common_api_traits(
 				_: #crate_::ExecutionContext,
 				_: Option<#block_type>,
 				_: Vec<u8>,
-			) -> std::result::Result<#crate_::NativeOrEncoded<()>, #error_type> {
+			) -> std::result::Result<#crate_::NativeOrEncoded<()>, #crate_::ApiError> {
 				unimplemented!("Not required for testing!")
 			}
 
@@ -160,7 +145,7 @@ fn implement_common_api_traits(
 				_: #crate_::ExecutionContext,
 				_: Option<&<#block_type as #crate_::BlockT>::Header>,
 				_: Vec<u8>,
-			) -> std::result::Result<#crate_::NativeOrEncoded<()>, #error_type> {
+			) -> std::result::Result<#crate_::NativeOrEncoded<()>, #crate_::ApiError> {
 				unimplemented!("Not required for testing!")
 			}
 		}
@@ -230,9 +215,6 @@ struct FoldRuntimeApiImpl<'a> {
 	block_type: &'a TypePath,
 	/// The identifier of the trait being implemented.
 	impl_trait: &'a Ident,
-	/// Stores the error type that is being found in the trait implementation as associated type
-	/// with the name `Error`.
-	error_type: &'a mut Option<Type>,
 }
 
 impl<'a> Fold for FoldRuntimeApiImpl<'a> {
@@ -300,7 +282,7 @@ impl<'a> Fold for FoldRuntimeApiImpl<'a> {
 
 				// Generate the correct return type.
 				input.sig.output = parse_quote!(
-					-> std::result::Result<#crate_::NativeOrEncoded<#ret_type>, Self::Error>
+					-> std::result::Result<#crate_::NativeOrEncoded<#ret_type>, #crate_::ApiError>
 				);
 			}
 
@@ -336,51 +318,12 @@ impl<'a> Fold for FoldRuntimeApiImpl<'a> {
 		input.block = block;
 		input
 	}
-
-	fn fold_impl_item(&mut self, input: ImplItem) -> ImplItem {
-		match input {
-			ImplItem::Type(ty) => {
-				if ty.ident == "Error" {
-					if let Some(error_type) = self.error_type {
-						if *error_type != ty.ty {
-							let mut error = Error::new(
-								ty.span(),
-								"Error type can not change between runtime apis",
-							);
-							let error_first = Error::new(
-								error_type.span(),
-								"First error type was declared here."
-							);
-
-							error.combine(error_first);
-
-							ImplItem::Verbatim(error.to_compile_error())
-						} else {
-							ImplItem::Verbatim(Default::default())
-						}
-					} else {
-						*self.error_type = Some(ty.ty);
-						ImplItem::Verbatim(Default::default())
-					}
-				} else {
-					let error = Error::new(
-						ty.span(),
-						"Only associated type with name `Error` is allowed",
-					);
-					ImplItem::Verbatim(error.to_compile_error())
-				}
-			},
-			o => fold::fold_impl_item(self, o),
-		}
-	}
 }
 
 /// Result of [`generate_runtime_api_impls`].
 struct GeneratedRuntimeApiImpls {
 	/// All the runtime api implementations.
 	impls: TokenStream,
-	/// The error type that should be used by the runtime apis.
-	error_type: Option<Type>,
 	/// The block type that is being used by the runtime apis.
 	block_type: TypePath,
 	/// The type the traits are implemented for.
@@ -393,7 +336,6 @@ struct GeneratedRuntimeApiImpls {
 /// extracts the error type, self type and the block type.
 fn generate_runtime_api_impls(impls: &[ItemImpl]) -> Result<GeneratedRuntimeApiImpls> {
 	let mut result = Vec::with_capacity(impls.len());
-	let mut error_type = None;
 	let mut global_block_type: Option<TypePath> = None;
 	let mut self_ty: Option<Box<Type>> = None;
 
@@ -451,7 +393,6 @@ fn generate_runtime_api_impls(impls: &[ItemImpl]) -> Result<GeneratedRuntimeApiI
 		let mut visitor = FoldRuntimeApiImpl {
 			block_type,
 			impl_trait: &impl_trait.ident,
-			error_type: &mut error_type,
 		};
 
 		result.push(visitor.fold_item_impl(impl_.clone()));
@@ -459,7 +400,6 @@ fn generate_runtime_api_impls(impls: &[ItemImpl]) -> Result<GeneratedRuntimeApiI
 
 	Ok(GeneratedRuntimeApiImpls {
 		impls: quote!( #( #result )* ),
-		error_type,
 		block_type: global_block_type.expect("There is a least one runtime api; qed"),
 		self_ty: *self_ty.expect("There is at least one runtime api; qed"),
 	})
@@ -475,9 +415,9 @@ pub fn mock_impl_runtime_apis_impl(input: proc_macro::TokenStream) -> proc_macro
 
 fn mock_impl_runtime_apis_impl_inner(api_impls: &[ItemImpl]) -> Result<TokenStream> {
 	let hidden_includes = generate_hidden_includes(HIDDEN_INCLUDES_ID);
-	let GeneratedRuntimeApiImpls { impls, error_type, block_type, self_ty } =
+	let GeneratedRuntimeApiImpls { impls, block_type, self_ty } =
 		generate_runtime_api_impls(api_impls)?;
-	let api_traits = implement_common_api_traits(error_type, block_type, self_ty)?;
+	let api_traits = implement_common_api_traits(block_type, self_ty)?;
 
 	Ok(quote!(
 		#hidden_includes

substrate/primitives/api/proc-macro/src/utils.rs

@@ -99,6 +99,7 @@ pub fn replace_wild_card_parameter_names(input: &mut Signature) {
 pub fn fold_fn_decl_for_client_side(
 	input: &mut Signature,
 	block_id: &TokenStream,
+	crate_: &TokenStream,
 ) {
 	replace_wild_card_parameter_names(input);
 
@@ -109,7 +110,7 @@ pub fn fold_fn_decl_for_client_side(
 	// Wrap the output in a `Result`
 	input.output = {
 		let ty = return_type_extract_type(&input.output);
-		parse_quote!( -> std::result::Result<#ty, Self::Error> )
+		parse_quote!( -> std::result::Result<#ty, #crate_::ApiError> )
 	};
 }