chore/error: remove from str conversion and add deprecation notificat… (#7472)

* chore/error: remove from str conversion and add deprecation notifications * fixup changes * fix test looking for gone ::Msg variant * another test fix * one is duplicate, the other is not, so duplicates reported are n-1 * darn spaces Co-authored-by: Andronik Ordian <write@reusable.software> * remove pointless doc comments of error variants without any value * low hanging fruits (for a tall person) * moar error type variants * avoid the storage modules for now They are in need of a refactor, and the pain is rather large removing all String error and DefaultError occurences. * chore remove pointless error generic * fix test for mocks, add a bunch of non_exhaustive * max line width * test fixes due to error changes * fin * error outputs... again * undo stderr adjustments * Update client/consensus/slots/src/lib.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * remove closure clutter Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * more error types * introduce ApiError * extract Mock error * ApiError refactor * even more error types * the last for now * chore unused deps * another extraction * reduce should panic, due to extended error messages * error test happiness * shift error lines by one * doc tests * white space Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Into -> From Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * remove pointless codec Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * avoid pointless self import Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Bernhard Schuster <bernhard@parity.io> Co-authored-by: Andronik Ordian <write@reusable.software> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
2026-06-13 22:11:06 +00:00 · 2020-11-27 19:37:53 +01:00
parent 6722a83ba6
commit 8c7d217091
48 changed files with 500 additions and 350 deletions
@@ -137,7 +137,9 @@ where
 		)?;
 		let state = self.backend.state_at(*id)?;
 		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
-		let runtime_code = self.check_override(state_runtime_code.runtime_code()?, id)?;
+		let runtime_code = state_runtime_code.runtime_code()
+			.map_err(sp_blockchain::Error::RuntimeCode)?;
+		let runtime_code = self.check_override(runtime_code, id)?;

 		let return_data = StateMachine::new(
 			&state,
@@ -211,7 +213,10 @@ where
 				let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&trie_state);
 				// It is important to extract the runtime code here before we create the proof
 				// recorder.
-				let runtime_code = self.check_override(state_runtime_code.runtime_code()?, at)?;
+				
+				let runtime_code = state_runtime_code.runtime_code()
+					.map_err(sp_blockchain::Error::RuntimeCode)?;
+				let runtime_code = self.check_override(runtime_code, at)?;

 				let backend = sp_state_machine::ProvingBackend::new_with_recorder(
 					trie_state,
@@ -236,7 +241,9 @@ where
 			},
 			None => {
 				let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
-				let runtime_code = self.check_override(state_runtime_code.runtime_code()?, at)?;
+				let runtime_code = state_runtime_code.runtime_code()
+					.map_err(sp_blockchain::Error::RuntimeCode)?;
+				let runtime_code = self.check_override(runtime_code, at)?;

 				let mut state_machine = StateMachine::new(
 					&state,
@@ -273,7 +280,9 @@ where
 			None,
 		);
 		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
-		self.executor.runtime_version(&mut ext, &state_runtime_code.runtime_code()?)
+		let runtime_code = state_runtime_code.runtime_code()
+			.map_err(sp_blockchain::Error::RuntimeCode)?;
+		self.executor.runtime_version(&mut ext, &runtime_code)
 			.map_err(|e| sp_blockchain::Error::VersionInvalid(format!("{:?}", e)).into())
 	}

@@ -284,6 +293,9 @@ where
 		method: &str,
 		call_data: &[u8]
 	) -> Result<(Vec<u8>, StorageProof), sp_blockchain::Error> {
+		let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_state);
+		let runtime_code = state_runtime_code.runtime_code()
+			.map_err(sp_blockchain::Error::RuntimeCode)?;
 		sp_state_machine::prove_execution_on_trie_backend::<_, _, NumberFor<Block>, _, _>(
 			trie_state,
 			overlay,
@@ -291,7 +303,7 @@ where
 			self.spawn_handle.clone(),
 			method,
 			call_data,
-			&sp_state_machine::backend::BackendRuntimeCode::new(trie_state).runtime_code()?,
+			&runtime_code,
 		)
 		.map_err(Into::into)
 	}
@@ -297,7 +297,8 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		config: ClientConfig,
 	) -> sp_blockchain::Result<Self> {
 		if backend.blockchain().header(BlockId::Number(Zero::zero()))?.is_none() {
-			let genesis_storage = build_genesis_storage.build_storage()?;
+			let genesis_storage = build_genesis_storage.build_storage()
+				.map_err(sp_blockchain::Error::Storage)?;
 			let mut op = backend.begin_operation()?;
 			backend.begin_state_operation(&mut op, BlockId::Hash(Default::default()))?;
 			let state_root = op.reset_storage(genesis_storage)?;
@@ -880,7 +881,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 					&state,
 					changes_trie_state.as_ref(),
 					*parent_hash,
-				)?;
+				).map_err(sp_blockchain::Error::Storage)?;

 				if import_block.header.state_root()
 					!= &gen_storage_changes.transaction_storage_root
@@ -37,7 +37,8 @@
 //! needed must be provided in the given directory.
 //!
 use std::{
-	fs, collections::{HashMap, hash_map::DefaultHasher}, path::Path,
+	fs, collections::{HashMap, hash_map::DefaultHasher},
+	path::{Path, PathBuf},
 	hash::Hasher as _,
 };
 use sp_core::traits::FetchRuntimeCode;
@@ -82,6 +83,29 @@ impl FetchRuntimeCode for WasmBlob {
 	}
 }

+#[derive(Debug, thiserror::Error)]
+#[allow(missing_docs)]
+pub enum WasmOverrideError {
+	#[error("Failed to get runtime version: {0}")]
+	VersionInvalid(String),
+
+	#[error("WASM override IO error")]
+	Io(PathBuf, #[source] std::io::Error),
+
+	#[error("Overwriting WASM requires a directory where local \
+	WASM is stored. {} is not a directory", .0.display())]
+	NotADirectory(PathBuf),
+
+	#[error("Duplicate WASM Runtimes found: \n{}\n", .0.join("\n") )]
+	DuplicateRuntime(Vec<String>),
+}
+
+impl From<WasmOverrideError> for sp_blockchain::Error {
+	fn from(err: WasmOverrideError) -> Self {
+		Self::Application(Box::new(err))
+	}
+}
+
 /// Scrapes WASM from a folder and returns WASM from that folder
 /// if the runtime spec version matches.
 #[derive(Clone, Debug)]
@@ -119,16 +143,13 @@ where
 	/// Scrapes a folder for WASM runtimes.
 	/// Returns a hashmap of the runtime version and wasm runtime code.
 	fn scrape_overrides(dir: &Path, executor: &E) -> Result<HashMap<u32, WasmBlob>> {
+
 		let handle_err = |e: std::io::Error | -> sp_blockchain::Error {
-			sp_blockchain::Error::Msg(format!("{}", e.to_string()))
+			WasmOverrideError::Io(dir.to_owned(), e).into()
 		};

 		if !dir.is_dir() {
-			return Err(sp_blockchain::Error::Msg(format!(
-				"Overwriting WASM requires a directory where \
-				 local WASM is stored. {:?} is not a directory",
-				 dir,
-			)));
+			return Err(WasmOverrideError::NotADirectory(dir.to_owned()).into());
 		}

 		let mut overrides = HashMap::new();
@@ -149,9 +170,7 @@ where
 		}

 		if !duplicates.is_empty() {
-			let duplicate_file_list = duplicates.join("\n");
-			let msg = format!("Duplicate WASM Runtimes found: \n{}\n", duplicate_file_list);
-			return Err(sp_blockchain::Error::Msg(msg));
+			return Err(WasmOverrideError::DuplicateRuntime(duplicates).into());
 		}

 		Ok(overrides)
@@ -164,7 +183,7 @@ where
 	) -> Result<RuntimeVersion> {
 		let mut ext = BasicExternalities::default();
 		executor.runtime_version(&mut ext, &code.runtime_code(heap_pages))
-			.map_err(|e| sp_blockchain::Error::VersionInvalid(format!("{:?}", e)).into())
+			.map_err(|e| WasmOverrideError::VersionInvalid(format!("{:?}", e)).into())
 	}
 }

@@ -236,14 +255,10 @@ mod tests {
 			let scraped = WasmOverride::scrape_overrides(dir, exec);

 			match scraped {
-				Err(e) => {
-					match e {
-						sp_blockchain::Error::Msg(msg) => {
-							let is_match = msg
-								.matches("Duplicate WASM Runtimes found")
-								.map(ToString::to_string)
-								.collect::<Vec<String>>();
-							assert!(is_match.len() >= 1)
+				Err(sp_blockchain::Error::Application(e)) => {
+					match e.downcast_ref::<WasmOverrideError>() {
+						Some(WasmOverrideError::DuplicateRuntime(duplicates)) => {
+							assert_eq!(duplicates.len(), 1);
 						},
 						_ => panic!("Test should end with Msg Error Variant")
 					}
@@ -27,25 +27,38 @@ use sp_blockchain;
 pub type Result<T> = std::result::Result<T, Error>;

 /// Service errors.
-#[derive(Debug, derive_more::Display, derive_more::From)]
+#[derive(Debug, thiserror::Error)]
+#[allow(missing_docs)]
+#[non_exhaustive]
 pub enum Error {
-	/// Client error.
-	Client(sp_blockchain::Error),
-	/// IO error.
-	Io(std::io::Error),
-	/// Consensus error.
-	Consensus(sp_consensus::Error),
-	/// Network error.
-	Network(sc_network::error::Error),
-	/// Keystore error.
-	Keystore(sc_keystore::Error),
-	/// Best chain selection strategy is missing.
-	#[display(fmt="Best chain selection strategy (SelectChain) is not provided.")]
+	#[error(transparent)]
+	Client(#[from] sp_blockchain::Error),
+	
+	#[error(transparent)]
+	Io(#[from] std::io::Error),
+	
+	#[error(transparent)]
+	Consensus(#[from] sp_consensus::Error),
+	
+	#[error(transparent)]
+	Network(#[from] sc_network::error::Error),
+
+	#[error(transparent)]
+	Keystore(#[from] sc_keystore::Error),
+
+	#[error("Best chain selection strategy (SelectChain) is not provided.")]
 	SelectChainRequired,
-	/// Tasks executor is missing.
-	#[display(fmt="Tasks executor hasn't been provided.")]
+
+	#[error("Tasks executor hasn't been provided.")]
 	TaskExecutorRequired,
-	/// Other error.
+
+	#[error("Prometheus metrics error")]
+	Prometheus(#[from] prometheus_endpoint::PrometheusError),
+
+	#[error("Application")]
+	Application(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
+
+	#[error("Other: {0}")]
 	Other(String),
 }

@@ -55,21 +68,8 @@ impl<'a> From<&'a str> for Error {
 	}
 }

-impl From<prometheus_endpoint::PrometheusError> for Error {
-	fn from(e: prometheus_endpoint::PrometheusError) -> Self {
-		Error::Other(format!("Prometheus error: {}", e))
-	}
-}
-
-impl std::error::Error for Error {
-	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-		match self {
-			Error::Client(ref err) => Some(err),
-			Error::Io(ref err) => Some(err),
-			Error::Consensus(ref err) => Some(err),
-			Error::Network(ref err) => Some(err),
-			Error::Keystore(ref err) => Some(err),
-			_ => None,
-		}
+impl<'a> From<String> for Error {
+	fn from(s: String) -> Self {
+		Error::Other(s)
 	}
 }