Remove thiserror and use more specific Error names (#419)

* small result clean up

* lets not overcomplicate

* remove thiserror

* remove unused errors

Author: haerdib

Cargo.lock

@@ -20,14 +20,12 @@ members = [
 [dependencies]
 # crates.io no_std
 codec = { package = "parity-scale-codec", version = "3.2.1", default-features = false, features = ['derive'] }
+derive_more = { version = "0.99.5" }
 hex = { version = "0.4.3", default-features = false, features = ["alloc"] }
 log = { version = "0.4.14", default-features = false }
 serde = { version = "1.0.136", default-features = false, features = ["derive"] }
 serde_json = { version = "1.0.79", default-features = false }
 
-#FIXME: convert back to thiserror once possible #317
-thiserror = { version = "1.0", package = "thiserror-core", default-features = false }
-
 # crates.io std only
 url = { version = "2.0.0", optional = true }
 
@@ -69,7 +67,6 @@ std = [
     "log/std",
     "serde/std",
     "serde_json/std",
-    "thiserror/std",
     # crates.io std only
     "url",
     # substrate no_std

Cargo.toml

@@ -243,7 +243,7 @@ where
 	fn get_genesis_hash(client: &Client) -> Result<Runtime::Hash> {
 		let genesis: Option<Runtime::Hash> =
 			client.request("chain_getBlockHash", rpc_params![Some(0)])?;
-		genesis.ok_or(Error::Genesis)
+		genesis.ok_or(Error::FetchGenesisHash)
 	}
 
 	/// Get runtime version from node via websocket query.

src/api/api_client.rs

@@ -24,70 +24,21 @@ use alloc::{boxed::Box, string::String};
 
 pub type Result<T> = core::result::Result<T, Error>;
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug, derive_more::From)]
 pub enum Error {
-	#[error("Fetching genesis hash failed. Are you connected to the correct endpoint?")]
-	Genesis,
-	#[error("Fetching runtime version failed. Are you connected to the correct endpoint?")]
-	RuntimeVersion,
-	#[error("Fetching Metadata failed. Are you connected to the correct endpoint?")]
-	MetadataFetch,
-	#[error("Operation needs a signer to be set in the api")]
+	FetchGenesisHash,
 	NoSigner,
-	#[error("RpcClient error: {0:?}")]
-	RpcClient(#[from] RpcClientError),
-	#[error("Metadata Error: {0:?}")]
+	RpcClient(RpcClientError),
 	Metadata(MetadataError),
-	#[error("InvalidMetadata: {0:?}")]
 	InvalidMetadata(InvalidMetadataError),
-	#[error("Events Error: {0:?}")]
 	NodeApi(ac_node_api::error::Error),
-	#[error("Error decoding storage value: {0}")]
 	StorageValueDecode(codec::Error),
-	#[error("UnsupportedXtStatus Error: Can only wait for finalized, in block, broadcast and ready. Waited for: {0:?}")]
 	UnsupportedXtStatus(XtStatus),
-	#[error("Error converting NumberOrHex to Balance")]
 	TryFromIntError,
-	#[error("The node runtime could not dispatch an extrinsic")]
 	Dispatch(DispatchError),
-	#[error("Extrinsic Error: {0}")]
 	Extrinsic(String),
-	#[error("Stream ended unexpectedly")]
 	NoStream,
-	#[error("Expected a block hash")]
 	NoBlockHash,
-	#[error("Did not find any block")]
 	NoBlock,
-	#[error(transparent)]
-	Other(#[from] Box<dyn core::error::Error + Send + Sync + 'static>),
-}
-
-impl From<codec::Error> for Error {
-	fn from(error: codec::Error) -> Self {
-		Error::StorageValueDecode(error)
-	}
-}
-
-impl From<InvalidMetadataError> for Error {
-	fn from(error: InvalidMetadataError) -> Self {
-		Error::InvalidMetadata(error)
-	}
-}
-
-impl From<MetadataError> for Error {
-	fn from(error: MetadataError) -> Self {
-		Error::Metadata(error)
-	}
-}
-
-impl From<ac_node_api::error::Error> for Error {
-	fn from(error: ac_node_api::error::Error) -> Self {
-		Error::NodeApi(error)
-	}
-}
-
-impl From<ac_node_api::error::DispatchError> for Error {
-	fn from(error: ac_node_api::error::DispatchError) -> Self {
-		Error::Dispatch(error)
-	}
+	Other(Box<dyn core::error::Error + Send + Sync + 'static>),
 }

src/api/error.rs

@@ -19,29 +19,21 @@ use alloc::{boxed::Box, string::String};
 
 pub type Result<T> = core::result::Result<T, Error>;
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Debug)]
 pub enum Error {
-	#[error("Serde json error: {0}")]
-	Serde(serde_json::error::Error),
-	#[error("mpsc send Error: {0}")]
-	Send(String),
-	#[error("Could not convert to valid Url: {0}")]
-	Url(String),
-	#[error("ChannelReceiveError, sender is disconnected: {0}")]
-	ChannelDisconnected(String),
-	#[error("Failure during thread creation: {0}")]
+	SerdeJson(serde_json::error::Error),
+	MpscSend(String),
+	InvalidUrl(String),
+	RecvError(String),
 	Io(String),
-	#[error("Exceeded maximum amount of connections")]
-	ConnectionAttemptsExceeded,
-	#[error("Websocket Connection was closed unexpectedly")]
+	MaxConnectionAttemptsExceeded,
 	ConnectionClosed,
-	#[error(transparent)]
-	Client(#[from] Box<dyn core::error::Error + Send + Sync + 'static>),
+	Client(Box<dyn core::error::Error + Send + Sync + 'static>),
 }
 
 impl From<serde_json::error::Error> for Error {
 	fn from(error: serde_json::error::Error) -> Self {
-		Self::Serde(error)
+		Self::SerdeJson(error)
 	}
 }
 
@@ -68,13 +60,13 @@ mod std_only {
 
 	impl From<SendError<String>> for Error {
 		fn from(error: SendError<String>) -> Self {
-			Self::Send(error.0)
+			Self::MpscSend(error.0)
 		}
 	}
 
 	impl From<RecvError> for Error {
 		fn from(error: RecvError) -> Self {
-			Self::ChannelDisconnected(format!("{:?}", error))
+			Self::RecvError(format!("{:?}", error))
 		}
 	}
 
@@ -86,7 +78,7 @@ mod std_only {
 
 	impl From<url::ParseError> for Error {
 		fn from(error: url::ParseError) -> Self {
-			Self::Io(format!("{:?}", error))
+			Self::InvalidUrl(format!("{:?}", error))
 		}
 	}
 }

src/rpc/error.rs

@@ -141,7 +141,7 @@ fn subscribe_to_server(
 pub fn do_reconnect(error: &RpcClientError) -> bool {
 	matches!(
 		error,
-		RpcClientError::Serde(_) | RpcClientError::ConnectionClosed | RpcClientError::Client(_)
+		RpcClientError::SerdeJson(_) | RpcClientError::ConnectionClosed | RpcClientError::Client(_)
 	)
 }
 
@@ -173,7 +173,7 @@ fn attempt_connection_until(url: &Url, max_attempts: u8) -> Result<(MySocket, Re
 		current_attempt += 1;
 	}
 
-	Err(RpcClientError::ConnectionAttemptsExceeded)
+	Err(RpcClientError::MaxConnectionAttemptsExceeded)
 }
 
 fn read_until_text_message(socket: &mut MySocket) -> Result<String> {

src/rpc/tungstenite_client/client.rs

@@ -85,7 +85,6 @@ impl WsRpcClient {
 	where
 		MessageHandler: HandleMessage + Clone + Send + 'static,
 		MessageHandler::ThreadMessage: Send + Sync + Debug,
-		MessageHandler::Error: Into<ws::Error>,
 		MessageHandler::Context: From<MessageContext<MessageHandler::ThreadMessage>>,
 	{
 		let mut socket = ws::Builder::new().build(move |out| RpcClient {
@@ -116,7 +115,6 @@ impl WsRpcClient {
 	where
 		MessageHandler: HandleMessage + Clone + Send + 'static,
 		MessageHandler::ThreadMessage: Send + Sync + Debug,
-		MessageHandler::Error: Into<ws::Error>,
 		MessageHandler::Context: From<MessageContext<MessageHandler::ThreadMessage>>,
 	{
 		let (result_in, result_out) = channel();

src/rpc/ws_client/client.rs

@@ -20,26 +20,19 @@ pub use ac_node_api::{events::EventDetails, StaticEvent};
 pub use client::WsRpcClient;
 use log::*;
 use std::{fmt::Debug, sync::mpsc::Sender as ThreadOut};
-use ws::{CloseCode, Handler, Handshake, Message, Sender};
+use ws::{CloseCode, Handler, Handshake, Message, Result as WsResult, Sender};
 
 pub mod client;
 pub mod subscription;
 
-type RpcResult<T> = Result<T, RpcClientError>;
-
-pub type RpcMessage = RpcResult<String>;
+pub type RpcMessage = crate::rpc::Result<String>;
 
 #[allow(clippy::result_large_err)]
 pub(crate) trait HandleMessage {
 	type ThreadMessage;
-	type Error;
 	type Context;
-	type Result;
 
-	fn handle_message(
-		&self,
-		context: &mut Self::Context,
-	) -> core::result::Result<Self::Result, Self::Error>;
+	fn handle_message(&self, context: &mut Self::Context) -> WsResult<()>;
 }
 
 // Clippy says request is never used, even though it is..
@@ -62,27 +55,23 @@ pub(crate) struct RpcClient<MessageHandler, ThreadMessage> {
 impl<MessageHandler: HandleMessage> Handler
 	for RpcClient<MessageHandler, MessageHandler::ThreadMessage>
 where
-	MessageHandler::Error: Into<ws::Error>,
 	MessageHandler::Context: From<MessageContext<MessageHandler::ThreadMessage>>,
 {
-	fn on_open(&mut self, _: Handshake) -> Result<(), ws::Error> {
+	fn on_open(&mut self, _: Handshake) -> WsResult<()> {
 		info!("sending request: {}", self.request);
 		self.out.send(self.request.clone())?;
 		Ok(())
 	}
 
-	fn on_message(&mut self, msg: Message) -> Result<(), ws::Error> {
+	fn on_message(&mut self, msg: Message) -> WsResult<()> {
 		let mut context: MessageHandler::Context = MessageContext {
 			out: self.out.clone(),
 			request: self.request.clone(),
 			result: self.result.clone(),
 			msg,
 		}
 		.into();
-		self.message_handler
-			.handle_message(&mut context)
-			.map_err(|e| e.into())
-			.map(|_| ())
+		self.message_handler.handle_message(&mut context)
 	}
 }
 
@@ -91,11 +80,9 @@ pub(crate) struct RequestHandler;
 
 impl HandleMessage for RequestHandler {
 	type ThreadMessage = RpcMessage;
-	type Error = ws::Error;
 	type Context = MessageContext<Self::ThreadMessage>;
-	type Result = ();
 
-	fn handle_message(&self, context: &mut Self::Context) -> Result<Self::Result, Self::Error> {
+	fn handle_message(&self, context: &mut Self::Context) -> WsResult<()> {
 		let result = &context.result;
 		let out = &context.out;
 		let msg = &context.msg;
@@ -106,11 +93,9 @@ impl HandleMessage for RequestHandler {
 		info!("Got get_request_msg {}", msg);
 		let result_str = serde_json::from_str(msg.as_text()?)
 			.map(|v: serde_json::Value| v["result"].to_string())
-			.map_err(RpcClientError::Serde);
+			.map_err(RpcClientError::SerdeJson);
 
-		result
-			.send(result_str)
-			.map_err(|e| Box::new(RpcClientError::Send(format!("{:?}", e))).into())
+		result.send(result_str).map_err(|e| Box::new(e).into())
 	}
 }
 
@@ -119,26 +104,24 @@ pub(crate) struct SubscriptionHandler {}
 
 impl HandleMessage for SubscriptionHandler {
 	type ThreadMessage = String;
-	type Error = ws::Error;
 	type Context = MessageContext<Self::ThreadMessage>;
-	type Result = ();
 
-	fn handle_message(&self, context: &mut Self::Context) -> Result<Self::Result, Self::Error> {
+	fn handle_message(&self, context: &mut Self::Context) -> WsResult<()> {
 		let result = &context.result;
 		let out = &context.out;
 		let msg = &context.msg;
 
 		info!("got on_subscription_msg {}", msg);
 		let value: serde_json::Value =
-			serde_json::from_str(msg.as_text()?).map_err(|e| Box::new(RpcClientError::Serde(e)))?;
+			serde_json::from_str(msg.as_text()?).map_err(|e| Box::new(e))?;
 
 		match value["id"].as_str() {
 			Some(_idstr) => {
 				warn!("Expected subscription, but received an id response instead: {:?}", value);
 			},
 			None => {
-				let answer = serde_json::to_string(&value["params"]["result"])
-					.map_err(|e| Box::new(RpcClientError::Serde(e)))?;
+				let answer =
+					serde_json::to_string(&value["params"]["result"]).map_err(|e| Box::new(e))?;
 
 				if let Err(e) = result.send(answer) {
 					// This may happen if the receiver has unsubscribed.