Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 8 additions & 18 deletions http-client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use crate::traits::Client;
use crate::transport::HttpTransportClient;
use crate::v2::request::{JsonRpcCallSer, JsonRpcNotificationSer};
use crate::v2::{
error::JsonRpcErrorAlloc,
error::JsonRpcError,
params::{Id, JsonRpcParams},
response::JsonRpcResponse,
};
use crate::{Error, JsonRawValue, TEN_MB_SIZE_BYTES};
use crate::{Error, TEN_MB_SIZE_BYTES};
use async_trait::async_trait;
use fnv::FnvHashMap;
use serde::de::DeserializeOwned;
Expand Down Expand Up @@ -76,12 +76,12 @@ impl Client for HttpClient {
let response: JsonRpcResponse<_> = match serde_json::from_slice(&body) {
Ok(response) => response,
Err(_) => {
let err: JsonRpcErrorAlloc = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err));
let err: JsonRpcError = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err.to_string()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I changed the error type to a String in Error::Request to get rid of the alloc type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I noticed that. I don't think there much else we can do right?

}
};

let response_id = parse_request_id(response.id)?;
let response_id = response.id.as_number().copied().ok_or(Error::InvalidRequestId)?;

if response_id == id {
Ok(response.result)
Expand Down Expand Up @@ -115,15 +115,15 @@ impl Client for HttpClient {
let rps: Vec<JsonRpcResponse<_>> = match serde_json::from_slice(&body) {
Ok(response) => response,
Err(_) => {
let err: JsonRpcErrorAlloc = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err));
let err: JsonRpcError = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err.to_string()));
}
};

// NOTE: `R::default` is placeholder and will be replaced in loop below.
let mut responses = vec![R::default(); ordered_requests.len()];
for rp in rps {
let response_id = parse_request_id(rp.id)?;
let response_id = rp.id.as_number().copied().ok_or(Error::InvalidRequestId)?;
let pos = match request_set.get(&response_id) {
Some(pos) => *pos,
None => return Err(Error::InvalidRequestId),
Expand All @@ -133,13 +133,3 @@ impl Client for HttpClient {
Ok(responses)
}
}

fn parse_request_id(raw: Option<&JsonRawValue>) -> Result<u64, Error> {
match raw {
None => Err(Error::InvalidRequestId),
Some(id) => {
let id = serde_json::from_str(id.get()).map_err(Error::ParseError)?;
Ok(id)
}
}
}
13 changes: 8 additions & 5 deletions http-client/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::v2::{
error::{JsonRpcErrorCode, JsonRpcErrorObjectAlloc},
error::{JsonRpcError, JsonRpcErrorCode, JsonRpcErrorObject},
params::JsonRpcParams,
};
use crate::{traits::Client, Error, HttpClientBuilder, JsonValue};
Expand Down Expand Up @@ -107,9 +107,12 @@ async fn run_request_with_response(response: String) -> Result<JsonValue, Error>
client.request("say_hello", JsonRpcParams::NoParams).await
}

fn assert_jsonrpc_error_response(error: Error, code: JsonRpcErrorObjectAlloc) {
match &error {
Error::Request(e) => assert_eq!(e.error, code),
e => panic!("Expected error: \"{}\", got: {:?}", error, e),
fn assert_jsonrpc_error_response(err: Error, exp: JsonRpcErrorObject) {
match &err {
Error::Request(e) => {
let this: JsonRpcError = serde_json::from_str(&e).unwrap();
assert_eq!(this.error, exp);
}
e => panic!("Expected error: \"{}\", got: {:?}", err, e),
};
}
15 changes: 10 additions & 5 deletions http-server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ use hyper::{
Error as HyperError,
};
use jsonrpsee_types::error::{CallError, Error, GenericTransportError};
use jsonrpsee_types::v2::request::{JsonRpcInvalidRequest, JsonRpcRequest};
use jsonrpsee_types::v2::{error::JsonRpcErrorCode, params::RpcParams};
use jsonrpsee_types::v2::error::JsonRpcErrorCode;
use jsonrpsee_types::v2::params::{Id, RpcParams};
use jsonrpsee_types::v2::request::{JsonRpcInvalidRequest, JsonRpcNotification, JsonRpcRequest};
use jsonrpsee_utils::hyper_helpers::read_response_to_body;
use jsonrpsee_utils::server::helpers::{collect_batch_response, send_error};
use jsonrpsee_utils::server::rpc_module::{MethodSink, RpcModule};
Expand Down Expand Up @@ -162,7 +163,7 @@ impl Server {
if let Some(method) = methods.get(&*req.method) {
let params = RpcParams::new(req.params.map(|params| params.get()));
// NOTE(niklasad1): connection ID is unused thus hardcoded to `0`.
if let Err(err) = (method)(req.id, params, &tx, 0) {
if let Err(err) = (method)(req.id.clone(), params, &tx, 0) {
log::error!(
"execution of method call '{}' failed: {:?}, request id={:?}",
req.method,
Expand Down Expand Up @@ -211,23 +212,27 @@ impl Server {
// Our [issue](https://github.com/paritytech/jsonrpsee/issues/296).
if let Ok(req) = serde_json::from_slice::<JsonRpcRequest>(&body) {
execute(&tx, req);
} else if let Ok(_req) = serde_json::from_slice::<JsonRpcNotification>(&body) {
return Ok::<_, HyperError>(response::ok_response("".into()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send empty response to notification,

currently we don't care if the method is registered or not (that API is not exposed)

} else if let Ok(batch) = serde_json::from_slice::<Vec<JsonRpcRequest>>(&body) {
if !batch.is_empty() {
single = false;
for req in batch {
execute(&tx, req);
}
} else {
send_error(None, &tx, JsonRpcErrorCode::InvalidRequest.into());
send_error(Id::Null, &tx, JsonRpcErrorCode::InvalidRequest.into());
}
} else if let Ok(_batch) = serde_json::from_slice::<Vec<JsonRpcNotification>>(&body) {
return Ok::<_, HyperError>(response::ok_response("".into()));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send empty response to batch notification,

currently we don't care if the method is registered or not (that API is not exposed)

} else {
log::error!(
"[service_fn], Cannot parse request body={:?}",
String::from_utf8_lossy(&body[..cmp::min(body.len(), 1024)])
);
let (id, code) = match serde_json::from_slice::<JsonRpcInvalidRequest>(&body) {
Ok(req) => (req.id, JsonRpcErrorCode::InvalidRequest),
Err(_) => (None, JsonRpcErrorCode::ParseError),
Err(_) => (Id::Null, JsonRpcErrorCode::ParseError),
};
send_error(id, &tx, code.into());
}
Expand Down
22 changes: 15 additions & 7 deletions http-server/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async fn invalid_single_method_call() {
let req = r#"{"jsonrpc":"2.0","method":1, "params": "bar"}"#;
let response = http_request(req.into(), uri.clone()).await.unwrap();
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, invalid_request(Id::Null));
assert_eq!(response.body, parse_error(Id::Null));
}

#[tokio::test]
Expand Down Expand Up @@ -169,14 +169,11 @@ async fn batched_notifications() {
let addr = server().await;
let uri = to_http_uri(addr);

let req = r#"[
{"jsonrpc": "2.0", "method": "notif", "params": [1,2,4]},
{"jsonrpc": "2.0", "method": "notif", "params": [7]}
]"#;
let req = r#"[{"jsonrpc": "2.0", "method": "notif", "params": [1,2,4]},{"jsonrpc": "2.0", "method": "notif", "params": [7]}]"#;
let response = http_request(req.into(), uri).await.unwrap();
assert_eq!(response.status, StatusCode::OK);
// Note: this is *not* according to spec. Response should be the empty string, `""`.
assert_eq!(response.body, r#"[{"jsonrpc":"2.0","result":"","id":null},{"jsonrpc":"2.0","result":"","id":null}]"#);
// Note: on HTTP acknowledge the notification with an empty response.
assert_eq!(response.body, "");
}

#[tokio::test]
Expand Down Expand Up @@ -248,3 +245,14 @@ async fn invalid_request_object() {
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, invalid_request(Id::Num(1)));
}

#[tokio::test]
async fn notif_works() {
let addr = server().await;
let uri = to_http_uri(addr);

let req = r#"{"jsonrpc":"2.0","method":"bar"}"#;
let response = http_request(req.into(), uri).await.unwrap();
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, "");
}
2 changes: 1 addition & 1 deletion types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ documentation = "https://docs.rs/jsonrpsee-types"

[dependencies]
async-trait = "0.1"
beef = "0.5"
beef = { version = "0.5", features = ["impl_serde"] }
futures-channel = { version = "0.3", features = ["sink"] }
futures-util = { version = "0.3", default-features = false, features = ["std", "sink", "channel"] }
log = { version = "0.4", default-features = false }
Expand Down
14 changes: 1 addition & 13 deletions types/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::v2::error::JsonRpcErrorAlloc;
use std::fmt;

/// Convenience type for displaying errors.
#[derive(Clone, Debug, PartialEq)]
pub struct Mismatch<T> {
Expand All @@ -16,10 +14,6 @@ impl<T: fmt::Display> fmt::Display for Mismatch<T> {
}
}

/// Invalid params.
#[derive(Debug)]
pub struct InvalidParams;

/// Error that occurs when a call failed.
#[derive(Debug, thiserror::Error)]
pub enum CallError {
Expand All @@ -31,12 +25,6 @@ pub enum CallError {
Failed(#[source] Box<dyn std::error::Error + Send + Sync>),
}

impl From<InvalidParams> for CallError {
fn from(_params: InvalidParams) -> Self {
Self::InvalidParams
}
}

/// Error type.
#[derive(Debug, thiserror::Error)]
pub enum Error {
Expand All @@ -48,7 +36,7 @@ pub enum Error {
Transport(#[source] Box<dyn std::error::Error + Send + Sync>),
/// JSON-RPC request error.
#[error("JSON-RPC request error: {0:?}")]
Request(#[source] JsonRpcErrorAlloc),
Request(String),
/// Frontend/backend channel error.
#[error("Frontend/backend channel error: {0}")]
Internal(#[source] futures_channel::mpsc::SendError),
Expand Down
93 changes: 36 additions & 57 deletions types/src/v2/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,57 +2,30 @@ use crate::v2::params::{Id, TwoPointZero};
use serde::de::Deserializer;
use serde::ser::Serializer;
use serde::{Deserialize, Serialize};
use serde_json::value::{RawValue, Value as JsonValue};
use serde_json::value::RawValue;
use std::fmt;
use thiserror::Error;

/// [Failed JSON-RPC response object](https://www.jsonrpc.org/specification#response_object).
#[derive(Serialize, Debug)]
#[derive(Serialize, Deserialize, Debug, PartialEq)]
pub struct JsonRpcError<'a> {
/// JSON-RPC version.
pub jsonrpc: TwoPointZero,
/// Error.
#[serde(borrow)]
pub error: JsonRpcErrorObject<'a>,
/// Request ID
pub id: Option<&'a RawValue>,
}
/// [Failed JSON-RPC response object with allocations](https://www.jsonrpc.org/specification#response_object).
#[derive(Error, Debug, Deserialize, PartialEq)]
pub struct JsonRpcErrorAlloc {
/// JSON-RPC version.
pub jsonrpc: TwoPointZero,
/// JSON-RPC error object.
pub error: JsonRpcErrorObjectAlloc,
/// Request ID.
pub id: Id,
pub id: Id<'a>,
}

impl fmt::Display for JsonRpcErrorAlloc {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{:?}: {:?}: {:?}", self.jsonrpc, self.error, self.id)
impl<'a> fmt::Display for JsonRpcError<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", serde_json::to_string(&self).expect("infallible; qed"))
}
}

/// JSON-RPC error object.
#[derive(Debug, PartialEq, Clone, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct JsonRpcErrorObjectAlloc {
/// Code
pub code: JsonRpcErrorCode,
/// Message
pub message: String,
/// Optional data
pub data: Option<JsonValue>,
}

impl From<JsonRpcErrorCode> for JsonRpcErrorObjectAlloc {
fn from(code: JsonRpcErrorCode) -> Self {
Self { code, message: code.message().to_owned(), data: None }
}
}

/// JSON-RPC error object with no extra allocations.
#[derive(Debug, Serialize)]
#[derive(Debug, Deserialize, Serialize, Clone)]
#[serde(deny_unknown_fields)]
pub struct JsonRpcErrorObject<'a> {
/// Code
Expand All @@ -61,6 +34,7 @@ pub struct JsonRpcErrorObject<'a> {
pub message: &'a str,
/// Optional data
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(borrow)]
pub data: Option<&'a RawValue>,
}

Expand All @@ -70,6 +44,14 @@ impl<'a> From<JsonRpcErrorCode> for JsonRpcErrorObject<'a> {
}
}

impl<'a> PartialEq for JsonRpcErrorObject<'a> {
fn eq(&self, other: &Self) -> bool {
let this_raw = self.data.map(|r| r.get());
let other_raw = self.data.map(|r| r.get());
self.code == other.code && self.message == other.message && this_raw == other_raw
}
}

/// Parse error code.
pub const PARSE_ERROR_CODE: i32 = -32700;
/// Internal error code.
Expand Down Expand Up @@ -180,47 +162,44 @@ impl serde::Serialize for JsonRpcErrorCode {

#[cfg(test)]
mod tests {
use super::{
Id, JsonRpcError, JsonRpcErrorAlloc, JsonRpcErrorCode, JsonRpcErrorObject, JsonRpcErrorObjectAlloc,
TwoPointZero,
};
use super::{Id, JsonRpcError, JsonRpcErrorCode, JsonRpcErrorObject, TwoPointZero};

#[test]
fn deserialize_works() {
let ser = r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}"#;
let err: JsonRpcErrorAlloc = serde_json::from_str(ser).unwrap();
assert_eq!(err.jsonrpc, TwoPointZero);
assert_eq!(
err.error,
JsonRpcErrorObjectAlloc { code: JsonRpcErrorCode::ParseError, message: "Parse error".into(), data: None }
);
assert_eq!(err.id, Id::Null);
let exp = JsonRpcError {
jsonrpc: TwoPointZero,
error: JsonRpcErrorObject { code: JsonRpcErrorCode::ParseError, message: "Parse error".into(), data: None },
id: Id::Null,
};
let err: JsonRpcError = serde_json::from_str(ser).unwrap();
assert_eq!(exp, err);
}

#[test]
fn deserialize_with_optional_data() {
let ser = r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error", "data":"vegan"},"id":null}"#;
let err: JsonRpcErrorAlloc = serde_json::from_str(ser).unwrap();
assert_eq!(err.jsonrpc, TwoPointZero);
assert_eq!(
err.error,
JsonRpcErrorObjectAlloc {
let data = serde_json::value::to_raw_value(&"vegan").unwrap();
let exp = JsonRpcError {
jsonrpc: TwoPointZero,
error: JsonRpcErrorObject {
code: JsonRpcErrorCode::ParseError,
message: "Parse error".into(),
data: Some("vegan".into())
}
);
assert_eq!(err.id, Id::Null);
data: Some(&*data),
},
id: Id::Null,
};
let err: JsonRpcError = serde_json::from_str(ser).unwrap();
assert_eq!(exp, err);
}

#[test]
fn serialize_works() {
let exp = r#"{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error"},"id":1337}"#;
let raw_id = serde_json::value::to_raw_value(&1337).unwrap();
let err = JsonRpcError {
jsonrpc: TwoPointZero,
error: JsonRpcErrorObject { code: JsonRpcErrorCode::InternalError, message: "Internal error", data: None },
id: Some(&*raw_id),
id: Id::Number(1337),
};
let ser = serde_json::to_string(&err).unwrap();
assert_eq!(exp, ser);
Expand Down
Loading