diff --git a/http-client/src/client.rs b/http-client/src/client.rs index 7d981e6dac..c1bc3164d1 100644 --- a/http-client/src/client.rs +++ b/http-client/src/client.rs @@ -27,8 +27,8 @@ impl HttpClientBuilder { /// Build the HTTP client with target to connect to. pub fn build(self, target: impl AsRef) -> Result { - let transport = HttpTransportClient::new(target, self.max_request_body_size) - .map_err(|e| Error::TransportError(Box::new(e)))?; + let transport = + HttpTransportClient::new(target, self.max_request_body_size).map_err(|e| Error::Transport(Box::new(e)))?; Ok(HttpClient { transport, request_id: AtomicU64::new(0) }) } } @@ -55,7 +55,7 @@ impl Client for HttpClient { self.transport .send(serde_json::to_string(¬if).map_err(Error::ParseError)?) .await - .map_err(|e| Error::TransportError(Box::new(e))) + .map_err(|e| Error::Transport(Box::new(e))) } /// Perform a request towards the server. @@ -71,7 +71,7 @@ impl Client for HttpClient { .transport .send_and_read_body(serde_json::to_string(&request).map_err(Error::ParseError)?) .await - .map_err(|e| Error::TransportError(Box::new(e)))?; + .map_err(|e| Error::Transport(Box::new(e)))?; let response: JsonRpcResponse<_> = match serde_json::from_slice(&body) { Ok(response) => response, @@ -110,7 +110,7 @@ impl Client for HttpClient { .transport .send_and_read_body(serde_json::to_string(&batch_request).map_err(Error::ParseError)?) .await - .map_err(|e| Error::TransportError(Box::new(e)))?; + .map_err(|e| Error::Transport(Box::new(e)))?; let rps: Vec> = match serde_json::from_slice(&body) { Ok(response) => response, diff --git a/http-server/src/module.rs b/http-server/src/module.rs index f4d42f7efb..93ff7de277 100644 --- a/http-server/src/module.rs +++ b/http-server/src/module.rs @@ -1,5 +1,10 @@ -use jsonrpsee_types::{traits::RpcMethod, v2::params::RpcParams, Error}; -use jsonrpsee_utils::server::{send_response, Methods}; +use jsonrpsee_types::v2::error::{JsonRpcErrorCode, JsonRpcErrorObject}; +use jsonrpsee_types::{ + error::{Error, InvalidParams, ServerCallError}, + traits::RpcMethod, + v2::params::RpcParams, +}; +use jsonrpsee_utils::server::{send_error, send_response, Methods}; use serde::Serialize; use std::sync::Arc; @@ -31,16 +36,17 @@ impl RpcModule { pub fn register_method(&mut self, method_name: &'static str, callback: F) -> Result<(), Error> where R: Serialize, - F: RpcMethod, + F: RpcMethod, { self.verify_method_name(method_name)?; self.methods.insert( method_name, Box::new(move |id, params, tx, _| { - let result = callback(params)?; - - send_response(id, tx, result); + match callback(params) { + Ok(res) => send_response(id, tx, res), + Err(InvalidParams) => send_error(id, tx, JsonRpcErrorCode::InvalidParams.into()), + }; Ok(()) }), @@ -82,7 +88,7 @@ impl RpcContextModule { where Context: Send + Sync + 'static, R: Serialize, - F: Fn(RpcParams, &Context) -> Result + Send + Sync + 'static, + F: Fn(RpcParams, &Context) -> Result + Send + Sync + 'static, { self.module.verify_method_name(method_name)?; @@ -91,10 +97,16 @@ impl RpcContextModule { self.module.methods.insert( method_name, Box::new(move |id, params, tx, _| { - let result = callback(params, &*ctx)?; - - send_response(id, tx, result); - + match callback(params, &*ctx) { + Ok(res) => send_response(id, tx, res), + Err(ServerCallError::InvalidParams(_)) => { + send_error(id, tx, JsonRpcErrorCode::InvalidParams.into()) + } + Err(ServerCallError::ContextFailed(err)) => { + let err = JsonRpcErrorObject { code: 1.into(), message: &err.to_string(), data: None }; + send_error(id, tx, err) + } + }; Ok(()) }), ); diff --git a/http-server/src/server.rs b/http-server/src/server.rs index 544e130427..326ddb0ca5 100644 --- a/http-server/src/server.rs +++ b/http-server/src/server.rs @@ -36,7 +36,7 @@ use hyper::{ service::{make_service_fn, service_fn}, Error as HyperError, }; -use jsonrpsee_types::error::{Error, GenericTransportError}; +use jsonrpsee_types::error::{Error, GenericTransportError, InvalidParams}; use jsonrpsee_types::v2::request::{JsonRpcInvalidRequest, JsonRpcRequest}; use jsonrpsee_types::v2::{error::JsonRpcErrorCode, params::RpcParams}; use jsonrpsee_utils::{hyper_helpers::read_response_to_body, server::send_error}; @@ -124,7 +124,7 @@ impl Server { pub fn register_method(&mut self, method_name: &'static str, callback: F) -> Result<(), Error> where R: Serialize, - F: Fn(RpcParams) -> Result + Send + Sync + 'static, + F: Fn(RpcParams) -> Result + Send + Sync + 'static, { self.root.register_method(method_name, callback) } diff --git a/http-server/src/tests.rs b/http-server/src/tests.rs index 95c80382cc..c46b89703e 100644 --- a/http-server/src/tests.rs +++ b/http-server/src/tests.rs @@ -42,14 +42,23 @@ async fn single_method_call_with_params() { let addr = server().await; let uri = to_http_uri(addr); - std::thread::sleep(std::time::Duration::from_secs(2)); - let req = r#"{"jsonrpc":"2.0","method":"add", "params":[1, 2],"id":1}"#; let response = http_request(req.into(), uri).await.unwrap(); assert_eq!(response.status, StatusCode::OK); assert_eq!(response.body, ok_response(JsonValue::Number(3.into()), Id::Num(1))); } +#[tokio::test] +async fn single_method_call_with_faulty_params_returns_err() { + let addr = server().await; + let uri = to_http_uri(addr); + + let req = r#"{"jsonrpc":"2.0","method":"add", "params":["Invalid"],"id":1}"#; + let response = http_request(req.into(), uri).await.unwrap(); + assert_eq!(response.status, StatusCode::OK); + assert_eq!(response.body, invalid_params(Id::Num(1))); +} + #[tokio::test] async fn should_return_method_not_found() { let addr = server().await; diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index c7743550c9..e809ef8318 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -203,14 +203,14 @@ async fn wss_works() { #[tokio::test] async fn ws_with_non_ascii_url_doesnt_hang_or_panic() { let err = WsClientBuilder::default().build("wss://♥♥♥♥♥♥∀∂").await; - assert!(matches!(err, Err(Error::TransportError(_)))); + assert!(matches!(err, Err(Error::Transport(_)))); } #[tokio::test] async fn http_with_non_ascii_url_doesnt_hang_or_panic() { let client = HttpClientBuilder::default().build("http://♥♥♥♥♥♥∀∂").unwrap(); let err: Result<(), Error> = client.request("system_chain", JsonRpcParams::NoParams).await; - assert!(matches!(err, Err(Error::TransportError(_)))); + assert!(matches!(err, Err(Error::Transport(_)))); } #[tokio::test] diff --git a/types/src/error.rs b/types/src/error.rs index 1483c7e7b1..cb1e355d87 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -16,12 +16,37 @@ impl fmt::Display for Mismatch { } } +/// Invalid params. +#[derive(Debug)] +pub struct InvalidParams; + +/// Error that may occur when a server fails to execute provided closure/callback. +#[derive(Debug, thiserror::Error)] +pub enum ServerCallError { + #[error("Invalid params in the RPC call")] + /// InvalidParams, + InvalidParams(InvalidParams), + #[error("Provided context failed: {0}")] + /// Provided context/metadata failed. + ContextFailed(#[source] Box), +} + +impl From for ServerCallError { + fn from(params: InvalidParams) -> Self { + Self::InvalidParams(params) + } +} + /// Error type. #[derive(Debug, thiserror::Error)] pub enum Error { + /// Error that may occur when a server fails to execute provided closure/callback. + /// Note, will most likely imply that the JSON-RPC request was valid. + #[error("Server call failed: {0}")] + ServerCall(ServerCallError), /// Networking error or error on the low-level protocol layer. #[error("Networking or low-level protocol error: {0}")] - TransportError(#[source] Box), + Transport(#[source] Box), /// JSON-RPC request error. #[error("JSON-RPC request error: {0:?}")] Request(#[source] JsonRpcErrorAlloc), @@ -34,7 +59,7 @@ pub enum Error { /// The background task has been terminated. #[error("The background task been terminated because: {0}; restart required")] RestartNeeded(String), - /// Failed to parse the data that the server sent back to us. + /// Failed to parse the data. #[error("Parse error: {0}")] ParseError(#[source] serde_json::Error), /// Invalid subscription ID. diff --git a/types/src/traits.rs b/types/src/traits.rs index b6a657a923..96bafbdc3a 100644 --- a/types/src/traits.rs +++ b/types/src/traits.rs @@ -47,6 +47,6 @@ pub trait SubscriptionClient: Client { } /// JSON-RPC server interface for managing method calls. -pub trait RpcMethod: Fn(RpcParams) -> Result + Send + Sync + 'static {} +pub trait RpcMethod: Fn(RpcParams) -> Result + Send + Sync + 'static {} -impl RpcMethod for T where T: Fn(RpcParams) -> Result + Send + Sync + 'static {} +impl RpcMethod for T where T: Fn(RpcParams) -> Result + Send + Sync + 'static {} diff --git a/types/src/v2/mod.rs b/types/src/v2/mod.rs index a3f977ab7e..d063057e4d 100644 --- a/types/src/v2/mod.rs +++ b/types/src/v2/mod.rs @@ -1,4 +1,4 @@ -use crate::Error; +use crate::error::InvalidParams; use serde::de::DeserializeOwned; use serde_json::value::RawValue; @@ -12,11 +12,11 @@ pub mod request; pub mod response; /// Parse request ID from RawValue. -pub fn parse_request_id(raw: Option<&RawValue>) -> Result { +pub fn parse_request_id(raw: Option<&RawValue>) -> Result { match raw { - None => Err(Error::InvalidRequestId), + None => Err(InvalidParams), Some(v) => { - let val = serde_json::from_str(v.get()).map_err(Error::ParseError)?; + let val = serde_json::from_str(v.get()).map_err(|_| InvalidParams)?; Ok(val) } } diff --git a/types/src/v2/params.rs b/types/src/v2/params.rs index 6145312ce7..cfe6b9e48d 100644 --- a/types/src/v2/params.rs +++ b/types/src/v2/params.rs @@ -1,4 +1,4 @@ -use crate::error::Error; +use crate::error::InvalidParams; use alloc::collections::BTreeMap; use serde::de::{self, Deserializer, Unexpected, Visitor}; use serde::ser::Serializer; @@ -78,18 +78,18 @@ impl<'a> RpcParams<'a> { } /// Attempt to parse all parameters as array or map into type T - pub fn parse(self) -> Result + pub fn parse(self) -> Result where T: Deserialize<'a>, { match self.0 { - None => Err(Error::InvalidParams), - Some(params) => serde_json::from_str(params).map_err(|_| Error::InvalidParams), + None => Err(InvalidParams), + Some(params) => serde_json::from_str(params).map_err(|_| InvalidParams), } } /// Attempt to parse only the first parameter from an array into type T - pub fn one(self) -> Result + pub fn one(self) -> Result where T: Deserialize<'a>, { diff --git a/ws-client/src/client.rs b/ws-client/src/client.rs index c783fb3ed6..08ef718da6 100644 --- a/ws-client/src/client.rs +++ b/ws-client/src/client.rs @@ -253,7 +253,7 @@ impl<'a> WsClientBuilder<'a> { let (to_back, from_front) = mpsc::channel(self.max_concurrent_requests); let (err_tx, err_rx) = oneshot::channel(); - let (sockaddrs, host, mode) = parse_url(url).map_err(|e| Error::TransportError(Box::new(e)))?; + let (sockaddrs, host, mode) = parse_url(url).map_err(|e| Error::Transport(Box::new(e)))?; let builder = WsTransportClientBuilder { sockaddrs, @@ -265,7 +265,7 @@ impl<'a> WsClientBuilder<'a> { max_request_body_size: self.max_request_body_size, }; - let (sender, receiver) = builder.build().await.map_err(|e| Error::TransportError(Box::new(e)))?; + let (sender, receiver) = builder.build().await.map_err(|e| Error::Transport(Box::new(e)))?; async_std::task::spawn(async move { background_task(sender, receiver, from_front, err_tx, max_capacity_per_subscription).await; @@ -518,7 +518,7 @@ async fn background_task( .expect("ID unused checked above; qed"), Err(e) => { log::warn!("[backend]: client request failed: {:?}", e); - let _ = request.send_back.map(|s| s.send(Err(Error::TransportError(Box::new(e))))); + let _ = request.send_back.map(|s| s.send(Err(Error::Transport(Box::new(e))))); } } } @@ -535,7 +535,7 @@ async fn background_task( .expect("Request ID unused checked above; qed"), Err(e) => { log::warn!("[backend]: client subscription failed: {:?}", e); - let _ = sub.send_back.send(Err(Error::TransportError(Box::new(e)))); + let _ = sub.send_back.send(Err(Error::Transport(Box::new(e)))); } }, // User dropped a subscription. @@ -600,7 +600,7 @@ async fn background_task( } Either::Right((Some(Err(e)), _)) => { log::error!("Error: {:?} terminating client", e); - let _ = front_error.send(Error::TransportError(Box::new(e))); + let _ = front_error.send(Error::Transport(Box::new(e))); return; } Either::Right((None, _)) => { diff --git a/ws-client/src/helpers.rs b/ws-client/src/helpers.rs index 540b630095..cc1dcdaefa 100644 --- a/ws-client/src/helpers.rs +++ b/ws-client/src/helpers.rs @@ -17,7 +17,7 @@ pub fn process_batch_response(manager: &mut RequestManager, rps: Vec = Vec::with_capacity(rps.len()); for rp in rps { - let id = parse_request_id(rp.id)?; + let id = parse_request_id(rp.id).map_err(|_| Error::InvalidRequestId)?; digest.push(id); rps_unordered.push((id, rp.result)); } @@ -82,7 +82,7 @@ pub fn process_single_response( response: JsonRpcResponse, max_capacity_per_subscription: usize, ) -> Result, Error> { - let response_id = parse_request_id(response.id)?; + let response_id = parse_request_id(response.id).map_err(|_| Error::InvalidRequestId)?; match manager.request_status(&response_id) { RequestStatus::PendingMethodCall => { let send_back_oneshot = match manager.complete_pending_call(response_id) { diff --git a/ws-server/Cargo.toml b/ws-server/Cargo.toml index fee1b28c13..760bb641df 100644 --- a/ws-server/Cargo.toml +++ b/ws-server/Cargo.toml @@ -11,6 +11,7 @@ documentation = "https://docs.rs/jsonrpsee-ws-server" [dependencies] anyhow = "1.0.34" +env_logger = "0.8" futures-channel = "0.3" futures-util = { version = "0.3", default-features = false, features = ["io"] } jsonrpsee-types = { path = "../types", version = "0.2.0-alpha.6" } diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 5be73959a2..03021c8841 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -37,7 +37,7 @@ use tokio::net::{TcpListener, ToSocketAddrs}; use tokio_stream::{wrappers::TcpListenerStream, StreamExt}; use tokio_util::compat::TokioAsyncReadCompatExt; -use jsonrpsee_types::error::Error; +use jsonrpsee_types::error::{Error, InvalidParams}; use jsonrpsee_types::v2::error::JsonRpcErrorCode; use jsonrpsee_types::v2::params::{JsonRpcNotificationParams, RpcParams, TwoPointZero}; use jsonrpsee_types::v2::request::{JsonRpcInvalidRequest, JsonRpcNotification, JsonRpcRequest}; @@ -105,7 +105,7 @@ impl Server { pub fn register_method(&mut self, method_name: &'static str, callback: F) -> Result<(), Error> where R: Serialize, - F: Fn(RpcParams) -> Result + Send + Sync + 'static, + F: Fn(RpcParams) -> Result + Send + Sync + 'static, { self.root.register_method(method_name, callback) } diff --git a/ws-server/src/server/module.rs b/ws-server/src/server/module.rs index 142898165a..8d146b0c0a 100644 --- a/ws-server/src/server/module.rs +++ b/ws-server/src/server/module.rs @@ -1,7 +1,10 @@ use crate::server::{RpcParams, SubscriptionId, SubscriptionSink}; -use jsonrpsee_types::error::Error; -use jsonrpsee_types::traits::RpcMethod; -use jsonrpsee_utils::server::{send_response, Methods}; +use jsonrpsee_types::{error::InvalidParams, traits::RpcMethod}; +use jsonrpsee_types::{ + error::{Error, ServerCallError}, + v2::error::{JsonRpcErrorCode, JsonRpcErrorObject}, +}; +use jsonrpsee_utils::server::{send_error, send_response, Methods}; use parking_lot::Mutex; use rustc_hash::FxHashMap; use serde::Serialize; @@ -35,16 +38,17 @@ impl RpcModule { pub fn register_method(&mut self, method_name: &'static str, callback: F) -> Result<(), Error> where R: Serialize, - F: RpcMethod, + F: RpcMethod, { self.verify_method_name(method_name)?; self.methods.insert( method_name, Box::new(move |id, params, tx, _| { - let result = callback(params)?; - - send_response(id, tx, result); + match callback(params) { + Ok(res) => send_response(id, tx, res), + Err(InvalidParams) => send_error(id, tx, JsonRpcErrorCode::InvalidParams.into()), + }; Ok(()) }), @@ -95,7 +99,7 @@ impl RpcModule { self.methods.insert( unsubscribe_method_name, Box::new(move |id, params, tx, conn| { - let sub_id = params.one()?; + let sub_id = params.one().map_err(|e| anyhow::anyhow!("{:?}", e))?; subscribers.lock().remove(&(conn, sub_id)); @@ -142,7 +146,7 @@ impl RpcContextModule { where Context: Send + Sync + 'static, R: Serialize, - F: Fn(RpcParams, &Context) -> Result + Send + Sync + 'static, + F: Fn(RpcParams, &Context) -> Result + Send + Sync + 'static, { self.module.verify_method_name(method_name)?; @@ -151,14 +155,20 @@ impl RpcContextModule { self.module.methods.insert( method_name, Box::new(move |id, params, tx, _| { - let result = callback(params, &*ctx)?; - - send_response(id, tx, result); + match callback(params, &*ctx) { + Ok(res) => send_response(id, tx, res), + Err(ServerCallError::InvalidParams(_)) => { + send_error(id, tx, JsonRpcErrorCode::InvalidParams.into()) + } + Err(ServerCallError::ContextFailed(err)) => { + let err = JsonRpcErrorObject { code: 1.into(), message: &err.to_string(), data: None }; + send_error(id, tx, err) + } + }; Ok(()) }), ); - Ok(()) } diff --git a/ws-server/src/tests.rs b/ws-server/src/tests.rs index 9c7ffe8107..b6c607250a 100644 --- a/ws-server/src/tests.rs +++ b/ws-server/src/tests.rs @@ -58,6 +58,19 @@ async fn single_method_call_with_params_works() { assert_eq!(response, ok_response(JsonValue::Number(3.into()), Id::Num(1))); } +#[tokio::test] +async fn single_method_call_with_faulty_params_returns_err() { + let _ = env_logger::try_init(); + let (server_started_tx, server_started_rx) = oneshot::channel::(); + tokio::spawn(server(server_started_tx)); + let server_addr = server_started_rx.await.unwrap(); + let mut client = WebSocketTestClient::new(server_addr).await.unwrap(); + + let req = r#"{"jsonrpc":"2.0","method":"add", "params":["Invalid"],"id":1}"#; + let response = client.send_request_text(req).await.unwrap(); + assert_eq!(response, invalid_params(Id::Num(1))); +} + #[tokio::test] async fn single_method_send_binary() { let (server_started_tx, server_started_rx) = oneshot::channel::();