Skip to content

Commit

Permalink
ret err if context/params fails
Browse files Browse the repository at this point in the history
  • Loading branch information
niklasad1 committed Apr 29, 2021
1 parent d18fca3 commit 0c88db6
Show file tree
Hide file tree
Showing 15 changed files with 127 additions and 57 deletions.
10 changes: 5 additions & 5 deletions http-client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ impl HttpClientBuilder {

/// Build the HTTP client with target to connect to.
pub fn build(self, target: impl AsRef<str>) -> Result<HttpClient, Error> {
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) })
}
}
Expand All @@ -55,7 +55,7 @@ impl Client for HttpClient {
self.transport
.send(serde_json::to_string(&notif).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.
Expand All @@ -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,
Expand Down Expand Up @@ -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<JsonRpcResponse<_>> = match serde_json::from_slice(&body) {
Ok(response) => response,
Expand Down
34 changes: 23 additions & 11 deletions http-server/src/module.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -31,16 +36,17 @@ impl RpcModule {
pub fn register_method<F, R>(&mut self, method_name: &'static str, callback: F) -> Result<(), Error>
where
R: Serialize,
F: RpcMethod<R>,
F: RpcMethod<R, InvalidParams>,
{
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(())
}),
Expand Down Expand Up @@ -82,7 +88,7 @@ impl<Context> RpcContextModule<Context> {
where
Context: Send + Sync + 'static,
R: Serialize,
F: Fn(RpcParams, &Context) -> Result<R, Error> + Send + Sync + 'static,
F: Fn(RpcParams, &Context) -> Result<R, ServerCallError> + Send + Sync + 'static,
{
self.module.verify_method_name(method_name)?;

Expand All @@ -91,10 +97,16 @@ impl<Context> RpcContextModule<Context> {
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(())
}),
);
Expand Down
4 changes: 2 additions & 2 deletions http-server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -124,7 +124,7 @@ impl Server {
pub fn register_method<F, R>(&mut self, method_name: &'static str, callback: F) -> Result<(), Error>
where
R: Serialize,
F: Fn(RpcParams) -> Result<R, Error> + Send + Sync + 'static,
F: Fn(RpcParams) -> Result<R, InvalidParams> + Send + Sync + 'static,
{
self.root.register_method(method_name, callback)
}
Expand Down
13 changes: 11 additions & 2 deletions http-server/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions tests/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
29 changes: 27 additions & 2 deletions types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,37 @@ impl<T: fmt::Display> fmt::Display for Mismatch<T> {
}
}

/// 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<dyn std::error::Error + Send + Sync>),
}

impl From<InvalidParams> 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<dyn std::error::Error + Send + Sync>),
Transport(#[source] Box<dyn std::error::Error + Send + Sync>),
/// JSON-RPC request error.
#[error("JSON-RPC request error: {0:?}")]
Request(#[source] JsonRpcErrorAlloc),
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions types/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ pub trait SubscriptionClient: Client {
}

/// JSON-RPC server interface for managing method calls.
pub trait RpcMethod<R>: Fn(RpcParams) -> Result<R, Error> + Send + Sync + 'static {}
pub trait RpcMethod<R, E>: Fn(RpcParams) -> Result<R, E> + Send + Sync + 'static {}

impl<R, T> RpcMethod<R> for T where T: Fn(RpcParams) -> Result<R, Error> + Send + Sync + 'static {}
impl<R, T, E> RpcMethod<R, E> for T where T: Fn(RpcParams) -> Result<R, E> + Send + Sync + 'static {}
8 changes: 4 additions & 4 deletions types/src/v2/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::Error;
use crate::error::InvalidParams;
use serde::de::DeserializeOwned;
use serde_json::value::RawValue;

Expand All @@ -12,11 +12,11 @@ pub mod request;
pub mod response;

/// Parse request ID from RawValue.
pub fn parse_request_id<T: DeserializeOwned>(raw: Option<&RawValue>) -> Result<T, crate::Error> {
pub fn parse_request_id<T: DeserializeOwned>(raw: Option<&RawValue>) -> Result<T, InvalidParams> {
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)
}
}
Expand Down
10 changes: 5 additions & 5 deletions types/src/v2/params.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -78,18 +78,18 @@ impl<'a> RpcParams<'a> {
}

/// Attempt to parse all parameters as array or map into type T
pub fn parse<T>(self) -> Result<T, Error>
pub fn parse<T>(self) -> Result<T, InvalidParams>
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<T>(self) -> Result<T, Error>
pub fn one<T>(self) -> Result<T, InvalidParams>
where
T: Deserialize<'a>,
{
Expand Down
10 changes: 5 additions & 5 deletions ws-client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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)))));
}
}
}
Expand All @@ -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.
Expand Down Expand Up @@ -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, _)) => {
Expand Down
4 changes: 2 additions & 2 deletions ws-client/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub fn process_batch_response(manager: &mut RequestManager, rps: Vec<JsonRpcResp
let mut rps_unordered: 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));
}
Expand Down Expand Up @@ -82,7 +82,7 @@ pub fn process_single_response(
response: JsonRpcResponse<JsonValue>,
max_capacity_per_subscription: usize,
) -> Result<Option<RequestMessage>, 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) {
Expand Down
1 change: 1 addition & 0 deletions ws-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
4 changes: 2 additions & 2 deletions ws-server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -105,7 +105,7 @@ impl Server {
pub fn register_method<F, R>(&mut self, method_name: &'static str, callback: F) -> Result<(), Error>
where
R: Serialize,
F: Fn(RpcParams) -> Result<R, Error> + Send + Sync + 'static,
F: Fn(RpcParams) -> Result<R, InvalidParams> + Send + Sync + 'static,
{
self.root.register_method(method_name, callback)
}
Expand Down
Loading

0 comments on commit 0c88db6

Please sign in to comment.