DRY error handling for methods (#515)

* DRY error handling for methods

* Fix clippy issues + unnecessary borrow

utils/src/server/helpers.rs

@@ -27,7 +27,11 @@
 use crate::server::rpc_module::MethodSink;
 use futures_channel::mpsc;
 use futures_util::stream::StreamExt;
-use jsonrpsee_types::v2::{ErrorCode, ErrorObject, Id, InvalidRequest, Response, RpcError, TwoPointZero};
+use jsonrpsee_types::error::{CallError, Error};
+use jsonrpsee_types::v2::{
+	error::{CALL_EXECUTION_FAILED_CODE, UNKNOWN_ERROR_CODE},
+	ErrorCode, ErrorObject, Id, InvalidRequest, Response, RpcError, TwoPointZero,
+};
 use serde::Serialize;
 
 /// Helper for sending JSON-RPC responses to the client
@@ -62,6 +66,22 @@ pub fn send_error(id: Id, tx: &MethodSink, error: ErrorObject) {
 	}
 }
 
+/// Helper for sending the general purpose `Error` as a JSON-RPC errors to the client
+pub fn send_call_error(id: Id, tx: &MethodSink, err: Error) {
+	let (code, message, data) = match err {
+		Error::Call(CallError::InvalidParams(e)) => (ErrorCode::InvalidParams, e.to_string(), None),
+		Error::Call(CallError::Failed(e)) => (ErrorCode::ServerError(CALL_EXECUTION_FAILED_CODE), e.to_string(), None),
+		Error::Call(CallError::Custom { code, message, data }) => (code.into(), message, data),
+		// This should normally not happen because the most common use case is to
+		// return `Error::Call` in `register_async_method`.
+		e => (ErrorCode::ServerError(UNKNOWN_ERROR_CODE), e.to_string(), None),
+	};
+
+	let err = ErrorObject { code, message: &message, data: data.as_deref() };
+
+	send_error(id, tx, err)
+}
+
 /// Figure out if this is a sufficiently complete request that we can extract an [`Id`] out of, or just plain
 /// unparseable garbage.
 pub fn prepare_error(data: &[u8]) -> (Id<'_>, ErrorCode) {

utils/src/server/resource_limiting.rs

@@ -58,7 +58,7 @@
 //! pub trait Rpc {
 //!     #[method(name = "my_expensive_method", resources("cpu" = 5, "mem" = 2))]
 //!     async fn my_expensive_method(&self) -> RpcResult<&'static str> {
-//! 	    // Do work
+//!         // Do work
 //!         Ok("hello")
 //!     }
 //! }
@@ -71,15 +71,15 @@
 //! #
 //! # fn main() -> RpcResult<()> {
 //! #
-//!	let mut module = RpcModule::new(());
+//! let mut module = RpcModule::new(());
 //!
 //! module
 //!     .register_async_method("my_expensive_method", |_, _| async move {
 //!         // Do work
 //!         Ok("hello")
 //!     })?
-//!	    .resource("cpu", 5)?
-//!	    .resource("mem", 2)?;
+//!     .resource("cpu", 5)?
+//!     .resource("mem", 2)?;
 //! # Ok(())
 //! # }
 //! ```

utils/src/server/rpc_module.rs

@@ -24,18 +24,17 @@
 // IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
-use crate::server::helpers::{send_error, send_response};
+use crate::server::helpers::{send_call_error, send_error, send_response};
 use crate::server::resource_limiting::{ResourceGuard, ResourceTable, ResourceVec, Resources};
 use beef::Cow;
 use futures_channel::{mpsc, oneshot};
 use futures_util::{future::BoxFuture, FutureExt, StreamExt};
 use jsonrpsee_types::{
-	error::{CallError, Error, SubscriptionClosedError},
+	error::{Error, SubscriptionClosedError},
 	traits::ToRpcParams,
 	v2::{
-		error::{CALL_EXECUTION_FAILED_CODE, UNKNOWN_ERROR_CODE},
-		ErrorCode, ErrorObject, Id, Params, Request, Response, SubscriptionId as RpcSubscriptionId,
-		SubscriptionPayload, SubscriptionResponse, TwoPointZero,
+		ErrorCode, Id, Params, Request, Response, SubscriptionId as RpcSubscriptionId, SubscriptionPayload,
+		SubscriptionResponse, TwoPointZero,
 	},
 	DeserializeOwned,
 };
@@ -427,32 +426,7 @@ impl<Context: Send + Sync + 'static> RpcModule<Context> {
 			MethodCallback::new_sync(Arc::new(move |id, params, tx, _| {
 				match callback(params, &*ctx) {
 					Ok(res) => send_response(id, tx, res),
-					Err(Error::Call(CallError::InvalidParams(e))) => {
-						let error = ErrorObject { code: ErrorCode::InvalidParams, message: &e.to_string(), data: None };
-						send_error(id, tx, error)
-					}
-					Err(Error::Call(CallError::Failed(e))) => {
-						let err = ErrorObject {
-							code: ErrorCode::ServerError(CALL_EXECUTION_FAILED_CODE),
-							message: &e.to_string(),
-							data: None,
-						};
-						send_error(id, tx, err)
-					}
-					Err(Error::Call(CallError::Custom { code, message, data })) => {
-						let err = ErrorObject { code: code.into(), message: &message, data: data.as_deref() };
-						send_error(id, tx, err)
-					}
-					// This should normally not happen because the most common use case is to
-					// return `Error::Call` in `register_method`.
-					Err(e) => {
-						let err = ErrorObject {
-							code: ErrorCode::ServerError(UNKNOWN_ERROR_CODE),
-							message: &e.to_string(),
-							data: None,
-						};
-						send_error(id, tx, err)
-					}
+					Err(err) => send_call_error(id, tx, err),
 				};
 			})),
 		)?;
@@ -479,33 +453,7 @@ impl<Context: Send + Sync + 'static> RpcModule<Context> {
 				let future = async move {
 					match callback(params, ctx).await {
 						Ok(res) => send_response(id, &tx, res),
-						Err(Error::Call(CallError::InvalidParams(e))) => {
-							let error =
-								ErrorObject { code: ErrorCode::InvalidParams, message: &e.to_string(), data: None };
-							send_error(id, &tx, error)
-						}
-						Err(Error::Call(CallError::Failed(e))) => {
-							let err = ErrorObject {
-								code: ErrorCode::ServerError(CALL_EXECUTION_FAILED_CODE),
-								message: &e.to_string(),
-								data: None,
-							};
-							send_error(id, &tx, err)
-						}
-						Err(Error::Call(CallError::Custom { code, message, data })) => {
-							let err = ErrorObject { code: code.into(), message: &message, data: data.as_deref() };
-							send_error(id, &tx, err)
-						}
-						// This should normally not happen because the most common use case is to
-						// return `Error::Call` in `register_async_method`.
-						Err(e) => {
-							let err = ErrorObject {
-								code: ErrorCode::ServerError(UNKNOWN_ERROR_CODE),
-								message: &e.to_string(),
-								data: None,
-							};
-							send_error(id, &tx, err)
-						}
+						Err(err) => send_call_error(id, &tx, err),
 					};
 
 					// Release claimed resources

ws-client/src/transport.rs

@@ -267,7 +267,6 @@ impl<'a> WsTransportClientBuilder<'a> {
 										}
 										Mode::Plain => None,
 									};
-									break;
 								}
 								// Relative URI.
 								else {
@@ -292,8 +291,8 @@ impl<'a> WsTransportClientBuilder<'a> {
 										};
 									}
 									target.sockaddrs = sockaddrs;
-									break;
 								}
+								break;
 							}
 							Err(e) => {
 								err = Some(Err(WsHandshakeError::Url(e.to_string().into())));