-
Notifications
You must be signed in to change notification settings - Fork 205
[types]: ID type instead of serde_json::RawValue #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
653cf78
00e7636
d03e092
3affd41
eb07a01
e5e9312
3c7d20f
ace5cb2
9eddc0c
78fec4f
e95b6c3
659ce7d
d5c9da9
f81eb77
8097e33
26d3031
a2264bb
84c00cd
7f901b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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}; | ||
|
|
@@ -162,17 +163,17 @@ 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, | ||
| err, | ||
| req.id | ||
| ); | ||
| send_error(req.id, &tx, JsonRpcErrorCode::ServerError(-1).into()); | ||
| send_error(req.id.clone(), &tx, JsonRpcErrorCode::ServerError(-1).into()); | ||
| } | ||
| } else { | ||
| send_error(req.id, &tx, JsonRpcErrorCode::MethodNotFound.into()); | ||
| send_error(req.id.clone(), &tx, JsonRpcErrorCode::MethodNotFound.into()); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -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())); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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())); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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] | ||
|
|
@@ -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 we ack the notification with an empty response. | ||
|
||
| assert_eq!(response.body, ""); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
|
|
@@ -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, ""); | ||
| } | ||
There was a problem hiding this comment.
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::Requestto get rid of the alloc type.There was a problem hiding this comment.
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?