From 81efcc4e52d498aa4640a297addfb45f5194add2 Mon Sep 17 00:00:00 2001
From: ropottnik <simonertl.1386@gmail.com>
Date: Fri, 15 Jan 2021 13:38:21 +0100
Subject: [PATCH] improve benchmarking error output (#7863)

* add concat Vec<u8> function and use it for better error logging in add_benchmark! macro

* refactor benchmark error reporting to use format! and RuntimeString
---
 substrate/frame/benchmarking/src/lib.rs       | 59 ++++++++++++++++++-
 substrate/frame/benchmarking/src/utils.rs     |  3 +-
 .../primitives/runtime/src/runtime_string.rs  | 16 +++++
 .../frame/benchmarking-cli/src/command.rs     |  2 +-
 4 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/substrate/frame/benchmarking/src/lib.rs b/substrate/frame/benchmarking/src/lib.rs
index 6db8674a3d..0657aea5d6 100644
--- a/substrate/frame/benchmarking/src/lib.rs
+++ b/substrate/frame/benchmarking/src/lib.rs
@@ -955,6 +955,39 @@ macro_rules! impl_benchmark_test {
 	};
 }
 
+/// show error message and debugging info for the case of an error happening
+/// during a benchmark
+pub fn show_benchmark_debug_info(
+	instance_string: &[u8],
+	benchmark: &[u8],
+	lowest_range_values: &sp_std::prelude::Vec<u32>,
+	highest_range_values: &sp_std::prelude::Vec<u32>,
+	steps: &sp_std::prelude::Vec<u32>,
+	repeat: &u32,
+	verify: &bool,
+	error_message: &str,
+) -> sp_runtime::RuntimeString {
+	sp_runtime::format_runtime_string!(
+		"\n* Pallet: {}\n\
+		* Benchmark: {}\n\
+		* Lowest_range_values: {:?}\n\
+		* Highest_range_values: {:?}\n\
+		* Steps: {:?}\n\
+		* Repeat: {:?}\n\
+		* Verify: {:?}\n\
+		* Error message: {}",
+		sp_std::str::from_utf8(instance_string)
+		.expect("it's all just strings ran through the wasm interface. qed"),
+		sp_std::str::from_utf8(benchmark)
+		.expect("it's all just strings ran through the wasm interface. qed"),
+		lowest_range_values,
+		highest_range_values,
+		steps,
+		repeat,
+		verify,
+		error_message,
+	)
+}
 
 /// This macro adds pallet benchmarks to a `Vec<BenchmarkBatch>` object.
 ///
@@ -1050,7 +1083,18 @@ macro_rules! add_benchmark {
 							*repeat,
 							whitelist,
 							*verify,
-						)?,
+						).map_err(|e| { 
+							$crate::show_benchmark_debug_info(
+								instance_string,
+								benchmark,
+								lowest_range_values,
+								highest_range_values,
+								steps,
+								repeat,
+								verify,
+								e,
+							)
+						})?,
 					});
 				}
 			} else {
@@ -1066,7 +1110,18 @@ macro_rules! add_benchmark {
 						*repeat,
 						whitelist,
 						*verify,
-					)?,
+					).map_err(|e| { 
+						$crate::show_benchmark_debug_info(
+							instance_string,
+							benchmark,
+							lowest_range_values,
+							highest_range_values,
+							steps,
+							repeat,
+							verify,
+							e,
+						)
+					})?,
 				});
 			}
 		}
diff --git a/substrate/frame/benchmarking/src/utils.rs b/substrate/frame/benchmarking/src/utils.rs
index 945141345c..1574e47454 100644
--- a/substrate/frame/benchmarking/src/utils.rs
+++ b/substrate/frame/benchmarking/src/utils.rs
@@ -20,7 +20,6 @@
 use codec::{Encode, Decode};
 use sp_std::{vec::Vec, prelude::Box};
 use sp_io::hashing::blake2_256;
-use sp_runtime::RuntimeString;
 use sp_storage::TrackedStorageKey;
 
 /// An alphabet of possible parameters to use for benchmarking.
@@ -90,7 +89,7 @@ sp_api::decl_runtime_apis! {
 	/// Runtime api for benchmarking a FRAME runtime.
 	pub trait Benchmark {
 		/// Dispatch the given benchmark.
-		fn dispatch_benchmark(config: BenchmarkConfig) -> Result<Vec<BenchmarkBatch>, RuntimeString>;
+		fn dispatch_benchmark(config: BenchmarkConfig) -> Result<Vec<BenchmarkBatch>, sp_runtime::RuntimeString>;
 	}
 }
 
diff --git a/substrate/primitives/runtime/src/runtime_string.rs b/substrate/primitives/runtime/src/runtime_string.rs
index df57def219..e315de430c 100644
--- a/substrate/primitives/runtime/src/runtime_string.rs
+++ b/substrate/primitives/runtime/src/runtime_string.rs
@@ -32,6 +32,22 @@ pub enum RuntimeString {
 	Owned(Vec<u8>),
 }
 
+/// Convenience macro to use the format! interface to get a `RuntimeString::Owned`
+#[macro_export]
+macro_rules! format_runtime_string {
+	($($args:tt)*) => {{
+		#[cfg(feature = "std")]
+		{
+			sp_runtime::RuntimeString::Owned(format!($($args)*))
+		}
+		#[cfg(not(feature = "std"))]
+		{
+			sp_runtime::RuntimeString::Owned(sp_std::alloc::format!($($args)*).as_bytes().to_vec())
+		}
+	}};
+}
+
+
 impl From<&'static str> for RuntimeString {
 	fn from(data: &'static str) -> Self {
 		Self::Borrowed(data)
diff --git a/substrate/utils/frame/benchmarking-cli/src/command.rs b/substrate/utils/frame/benchmarking-cli/src/command.rs
index 8a6a39f045..57b9a592f0 100644
--- a/substrate/utils/frame/benchmarking-cli/src/command.rs
+++ b/substrate/utils/frame/benchmarking-cli/src/command.rs
@@ -174,7 +174,7 @@ impl BenchmarkCmd {
 					}
 				}
 			},
-			Err(error) => eprintln!("Error: {:?}", error),
+			Err(error) => eprintln!("Error: {}", error),
 		}
 
 		Ok(())