chore/error: remove from str conversion and add deprecation notificat…

…ions

client/service/src/client/wasm_override.rs

@@ -119,16 +119,16 @@ 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()))
+
+		let handle_err = {
+			let dir = dir.to_owned();
+			move |e: std::io::Error | -> sp_blockchain::Error {
+				sp_blockchain::Error::WasmOverrideIo(dir, e)
+			}
 		};
 
 		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(sp_blockchain::Error::WasmOverrideNotADirectory(dir.to_owned()));
 		}
 
 		let mut overrides = HashMap::new();
@@ -149,9 +149,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(sp_blockchain::Error::DuplicateWasmRuntime(duplicates));
 		}
 
 		Ok(overrides)

client/sync-state-rpc/src/lib.rs

@@ -79,17 +79,13 @@ impl<TBl, TCl> SyncStateRpcHandler<TBl, TCl>
 	fn build_sync_state(&self) -> Result<sc_chain_spec::LightSyncState<TBl>, sp_blockchain::Error> {
 		let finalized_hash = self.client.info().finalized_hash;
 		let finalized_header = self.client.header(BlockId::Hash(finalized_hash))?
-			.ok_or_else(|| sp_blockchain::Error::Msg(
-				format!("Failed to get the header for block {:?}", finalized_hash)
-			))?;
+			.ok_or_else(|| sp_blockchain::Error::MissingHashInHeader(finalized_hash.to_string()))?;
 
 		let finalized_block_weight = sc_consensus_babe::aux_schema::load_block_weight(
-			&*self.client,
-			finalized_hash,
-		)?
-			.ok_or_else(|| sp_blockchain::Error::Msg(
-				format!("Failed to load the block weight for block {:?}", finalized_hash)
-			))?;
+				&*self.client,
+				finalized_hash,
+			)?
+			.ok_or_else(|| sp_blockchain::Error::MissingBlockWeightInHeader(finalized_hash.to_string()))?;
 
 		Ok(sc_chain_spec::LightSyncState {
 			finalized_block_header: finalized_header,

primitives/blockchain/src/backend.rs

@@ -172,7 +172,7 @@ pub trait Backend<Block: BlockT>: HeaderBackend<Block> + HeaderMetadata<Block, E
 			if let Some(max_number) = maybe_max_number {
 				loop {
 					let current_header = self.header(BlockId::Hash(current_hash.clone()))?
-						.ok_or_else(|| Error::from(format!("failed to get header for hash {}", current_hash)))?;
+						.ok_or_else(|| Error::MissingHashInHeader(current_hash.to_string()))?;
 
 					if current_header.number() <= &max_number {
 						best_hash = current_header.hash();
@@ -191,7 +191,7 @@ pub trait Backend<Block: BlockT>: HeaderBackend<Block> + HeaderMetadata<Block, E
 				}
 
 				let current_header = self.header(BlockId::Hash(current_hash.clone()))?
-					.ok_or_else(|| Error::from(format!("failed to get header for hash {}", current_hash)))?;
+					.ok_or_else(|| Error::MissingHashInHeader(current_hash.to_string()))?;
 
 				// stop search in this chain once we go below the target's block number
 				if current_header.number() < target_header.number() {

primitives/blockchain/src/error.rs

@@ -35,112 +35,149 @@ pub enum ApplyExtrinsicFailed {
 	/// unappliable onto the current block.
 	#[error("Extrinsic is not valid: {0:?}")]
 	Validity(#[from] TransactionValidityError),
+
 	/// This is used for miscellaneous errors that can be represented by string and not handleable.
 	///
 	/// This will become obsolete with complete migration to v4 APIs.
+	#[deprecated(note = "Introduce more typed error variants as needed")]
 	#[error("Extrinsic failed: {0}")]
 	Msg(String),
 }
 
 /// Substrate Client error
 #[derive(Debug, thiserror::Error)]
+#[allow(missing_docs)]
 pub enum Error {
 	/// Consensus Error
 	#[error(transparent)]
 	Consensus(#[from] sp_consensus::Error),
+
 	/// Backend error.
 	#[error("Backend error: {0}")]
 	Backend(String),
+
 	/// Unknown block.
 	#[error("UnknownBlock: {0}")]
 	UnknownBlock(String),
+
 	/// The `apply_extrinsic` is not valid due to the given `TransactionValidityError`.
 	#[error(transparent)]
 	ApplyExtrinsicFailed(#[from] ApplyExtrinsicFailed),
+
 	/// Execution error.
 	#[error("Execution failed: {0:?}")]
 	Execution(Box<dyn sp_state_machine::Error>),
+
 	/// Blockchain error.
 	#[error("Blockchain")]
 	Blockchain(#[source] Box<Error>),
+
 	/// Invalid authorities set received from the runtime.
 	#[error("Current state of blockchain has invalid authorities set")]
 	InvalidAuthoritiesSet,
+
 	/// Could not get runtime version.
 	#[error("Failed to get runtime version: {0}")]
 	VersionInvalid(String),
+
 	/// Genesis config is invalid.
 	#[error("Genesis config provided is invalid")]
 	GenesisInvalid,
+
 	/// Error decoding header justification.
 	#[error("error decoding justification for header")]
 	JustificationDecode,
+
 	/// Justification for header is correctly encoded, but invalid.
 	#[error("bad justification for header: {0}")]
 	BadJustification(String),
+
 	/// Not available on light client.
 	#[error("This method is not currently available when running in light client mode")]
 	NotAvailableOnLightClient,
+
 	/// Invalid remote CHT-based proof.
 	#[error("Remote node has responded with invalid header proof")]
 	InvalidCHTProof,
+
 	/// Remote fetch has been cancelled.
 	#[error("Remote data fetch has been cancelled")]
 	RemoteFetchCancelled,
+
 	/// Remote fetch has been failed.
 	#[error("Remote data fetch has been failed")]
 	RemoteFetchFailed,
+
 	/// Error decoding call result.
 	#[error("Error decoding call result of {0}")]
 	CallResultDecode(&'static str, #[source] CodecError),
+
 	/// Error converting a parameter between runtime and node.
 	#[error("Error converting `{0}` between runtime and node")]
 	RuntimeParamConversion(String),
+
 	/// Changes tries are not supported.
 	#[error("Changes tries are not supported by the runtime")]
 	ChangesTriesNotSupported,
+
 	/// Error reading changes tries configuration.
 	#[error("Error reading changes tries configuration")]
 	ErrorReadingChangesTriesConfig,
+
 	/// Key changes query has failed.
 	#[error("Failed to check changes proof: {0}")]
 	ChangesTrieAccessFailed(String),
+
 	/// Last finalized block not parent of current.
 	#[error("Did not finalize blocks in sequential order.")]
 	NonSequentialFinalization(String),
+
 	/// Safety violation: new best block not descendent of last finalized.
 	#[error("Potential long-range attack: block not in finalized chain.")]
 	NotInFinalizedChain,
+
 	/// Hash that is required for building CHT is missing.
 	#[error("Failed to get hash of block for building CHT")]
 	MissingHashRequiredForCHT,
+
 	/// Invalid calculated state root on block import.
 	#[error("Calculated state root does not match.")]
 	InvalidStateRoot,
+
 	/// Incomplete block import pipeline.
 	#[error("Incomplete block import pipeline.")]
 	IncompletePipeline,
+
+	/// Transaction pool initizliation is not complete.
 	#[error("Transaction pool not ready for block production.")]
 	TransactionPoolNotReady,
+
+	/// Database yielded an error.
 	#[error("Database")]
 	DatabaseError(#[from] sp_database::error::DatabaseError),
+
+	#[error("Failed to get header for hash {0}")]
+	MissingHashInHeader(String),
+
+	#[error("Failed to load the block weight for block {0}")]
+	MissingBlockWeightInHeader(String),
+
+	#[error("WASM override IO error")]
+	WasmOverrideIo(PathBuf, #[source] std::io::Error),
+
+	#[error("Overwriting WASM requires a directory where local \
+	WASM is stored. {0} is not a directory", .0.display())]
+	WasmOverrideNotADirectory(PathBuf),
+
+	#[error("Duplicate WASM Runtimes found: \n{}\n", .0.join("\n") )]
+	DuplicateWasmRuntime(Vec<String>),
+
 	/// A convenience variant for String
+	#[deprecated(note = "Introduce more typed error variants as needed")]
 	#[error("{0}")]
 	Msg(String),
 }
 
-impl<'a> From<&'a str> for Error {
-	fn from(s: &'a str) -> Self {
-		Error::Msg(s.into())
-	}
-}
-
-impl From<String> for Error {
-	fn from(s: String) -> Self {
-		Error::Msg(s)
-	}
-}
-
 impl From<Box<dyn sp_state_machine::Error + Send + Sync>> for Error {
 	fn from(e: Box<dyn sp_state_machine::Error + Send + Sync>) -> Self {
 		Self::from_state(e)