From eff1cb96ba425ce8a6355335cf033fe86cc31104 Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Thu, 9 Jan 2025 18:26:20 +0000 Subject: [PATCH 01/11] enha: send original error response in ProxyGetRequest --- .../src/middleware/http/proxy_get_request.rs | 21 ++++++++++++++----- server/src/transport/http.rs | 9 ++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/server/src/middleware/http/proxy_get_request.rs b/server/src/middleware/http/proxy_get_request.rs index 49e32d657d..ad443cb6ad 100644 --- a/server/src/middleware/http/proxy_get_request.rs +++ b/server/src/middleware/http/proxy_get_request.rs @@ -36,7 +36,7 @@ use hyper::header::{ACCEPT, CONTENT_TYPE}; use hyper::http::HeaderValue; use hyper::{Method, Uri}; use jsonrpsee_core::BoxError; -use jsonrpsee_types::{Id, RequestSer}; +use jsonrpsee_types::{ErrorObject, Id, RequestSer}; use std::collections::HashMap; use std::future::Future; use std::pin::Pin; @@ -179,16 +179,27 @@ where bytes.extend(data); } - #[derive(serde::Deserialize)] - struct RpcPayload<'a> { + #[derive(serde::Deserialize, serde::Serialize, Debug)] + struct SuccessResponse<'a> { #[serde(borrow)] result: &'a serde_json::value::RawValue, } - let response = if let Ok(payload) = serde_json::from_slice::(&bytes) { + #[derive(serde::Deserialize, serde::Serialize, Debug)] + struct ErrorResponse<'a> { + #[serde(borrow)] + error: &'a serde_json::value::RawValue, + } + + let response = if let Ok(payload) = serde_json::from_slice::(&bytes) { http::response::ok_response(payload.result.to_string()) } else { - http::response::internal_error() + serde_json::from_slice::(&bytes) + .and_then(|payload| serde_json::from_str::(&payload.error.to_string())) + .map_or_else( + |_| http::response::internal_error(), + |error| http::response::error_response(error.to_owned()), + ) }; Ok(response) diff --git a/server/src/transport/http.rs b/server/src/transport/http.rs index d05c772cad..6cbeb15a3a 100644 --- a/server/src/transport/http.rs +++ b/server/src/transport/http.rs @@ -127,6 +127,15 @@ pub mod response { from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, error, JSON) } + /// Create a json response for general errors returned by the called method. + pub fn error_response(error: ErrorObjectOwned) -> HttpResponse { + let err = ResponsePayload::<()>::error(error); + let rp = Response::new(err, Id::Null); + let error = serde_json::to_string(&rp).expect("JSON serialization infallible; qed"); + + from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, error, JSON) + } + /// Create a text/plain response for not allowed hosts. pub fn host_not_allowed() -> HttpResponse { from_template(hyper::StatusCode::FORBIDDEN, "Provided Host header is not whitelisted.\n", TEXT) From 687467298267828cd513b89061a963a1daaf3943 Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Fri, 10 Jan 2025 15:11:10 +0000 Subject: [PATCH 02/11] reply to get requests with strings instead of json --- server/src/middleware/http/proxy_get_request.rs | 6 +++--- server/src/transport/http.rs | 14 +++++--------- types/src/error.rs | 7 ++++++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/server/src/middleware/http/proxy_get_request.rs b/server/src/middleware/http/proxy_get_request.rs index ad443cb6ad..061683d9e9 100644 --- a/server/src/middleware/http/proxy_get_request.rs +++ b/server/src/middleware/http/proxy_get_request.rs @@ -36,7 +36,7 @@ use hyper::header::{ACCEPT, CONTENT_TYPE}; use hyper::http::HeaderValue; use hyper::{Method, Uri}; use jsonrpsee_core::BoxError; -use jsonrpsee_types::{ErrorObject, Id, RequestSer}; +use jsonrpsee_types::{ErrorCode, ErrorObject, Id, RequestSer}; use std::collections::HashMap; use std::future::Future; use std::pin::Pin; @@ -197,8 +197,8 @@ where serde_json::from_slice::(&bytes) .and_then(|payload| serde_json::from_str::(&payload.error.to_string())) .map_or_else( - |_| http::response::internal_error(), - |error| http::response::error_response(error.to_owned()), + |_| http::response::error_response(ErrorCode::InternalError.to_string()), + |error| http::response::error_response(error.to_string()), ) }; diff --git a/server/src/transport/http.rs b/server/src/transport/http.rs index 6cbeb15a3a..28db321f79 100644 --- a/server/src/transport/http.rs +++ b/server/src/transport/http.rs @@ -127,15 +127,6 @@ pub mod response { from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, error, JSON) } - /// Create a json response for general errors returned by the called method. - pub fn error_response(error: ErrorObjectOwned) -> HttpResponse { - let err = ResponsePayload::<()>::error(error); - let rp = Response::new(err, Id::Null); - let error = serde_json::to_string(&rp).expect("JSON serialization infallible; qed"); - - from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, error, JSON) - } - /// Create a text/plain response for not allowed hosts. pub fn host_not_allowed() -> HttpResponse { from_template(hyper::StatusCode::FORBIDDEN, "Provided Host header is not whitelisted.\n", TEXT) @@ -183,6 +174,11 @@ pub mod response { from_template(hyper::StatusCode::OK, body, JSON) } + /// Create a general internal error response. + pub fn error_response(body: impl Into) -> HttpResponse { + from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, body, JSON) + } + /// Create a response for unsupported content type. pub fn unsupported_content_type() -> HttpResponse { from_template( diff --git a/types/src/error.rs b/types/src/error.rs index 4617e8e318..0388d578a6 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -39,7 +39,6 @@ pub type ErrorObjectOwned = ErrorObject<'static>; /// [Failed JSON-RPC response object](https://www.jsonrpc.org/specification#response_object). #[derive(Debug, Deserialize, Serialize, Clone, thiserror::Error)] #[serde(deny_unknown_fields)] -#[error("{self:?}")] pub struct ErrorObject<'a> { /// Code code: ErrorCode, @@ -96,6 +95,12 @@ impl<'a> ErrorObject<'a> { } } +impl fmt::Display for ErrorObject<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}: {}", self.code(), self.message()) + } +} + impl PartialEq for ErrorObject<'_> { fn eq(&self, other: &Self) -> bool { let this_raw = self.data.as_ref().map(|r| r.get()); From a6ee4350709e48fb071f0251a47c1500128189ab Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Fri, 10 Jan 2025 15:36:11 +0000 Subject: [PATCH 03/11] bless trybuild output --- proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr index 33beae6bcb..ef4202484a 100644 --- a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr +++ b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr @@ -17,6 +17,10 @@ error[E0277]: the trait bound `::Hash: Clone` is not satisfied | = note: required for `Result<::Hash, ErrorObject<'_>>` to implement `IntoResponse` = note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider removing this method call, as the receiver has type `&Self` and `&Self: Clone` trivially holds + | +9 | #[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())] + | error[E0277]: the trait bound `::Hash: DeserializeOwned` is not satisfied --> tests/ui/incorrect/rpc/rpc_empty_bounds.rs:9:1 From bb631072c242bf27072ccd8f8a22a7d83af7e5cc Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Fri, 10 Jan 2025 16:29:08 +0000 Subject: [PATCH 04/11] return the inner json object, not a formatted string --- server/src/middleware/http/proxy_get_request.rs | 8 +++----- server/src/transport/http.rs | 13 +++++++------ types/src/error.rs | 7 +------ 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/server/src/middleware/http/proxy_get_request.rs b/server/src/middleware/http/proxy_get_request.rs index 061683d9e9..26e1c3e265 100644 --- a/server/src/middleware/http/proxy_get_request.rs +++ b/server/src/middleware/http/proxy_get_request.rs @@ -194,12 +194,10 @@ where let response = if let Ok(payload) = serde_json::from_slice::(&bytes) { http::response::ok_response(payload.result.to_string()) } else { - serde_json::from_slice::(&bytes) + let error = serde_json::from_slice::(&bytes) .and_then(|payload| serde_json::from_str::(&payload.error.to_string())) - .map_or_else( - |_| http::response::error_response(ErrorCode::InternalError.to_string()), - |error| http::response::error_response(error.to_string()), - ) + .unwrap_or_else(|_| ErrorObject::from(ErrorCode::InternalError)); + http::response::error_response(error) }; Ok(response) diff --git a/server/src/transport/http.rs b/server/src/transport/http.rs index 28db321f79..1d0b7d5cd7 100644 --- a/server/src/transport/http.rs +++ b/server/src/transport/http.rs @@ -111,7 +111,7 @@ where /// HTTP response helpers. pub mod response { use jsonrpsee_types::error::{reject_too_big_request, ErrorCode}; - use jsonrpsee_types::{ErrorObjectOwned, Id, Response, ResponsePayload}; + use jsonrpsee_types::{ErrorObject, ErrorObjectOwned, Id, Response, ResponsePayload}; use crate::{HttpBody, HttpResponse}; @@ -127,6 +127,12 @@ pub mod response { from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, error, JSON) } + /// Create a json response for general errors returned by the called method. + pub fn error_response(error: ErrorObject) -> HttpResponse { + let error = serde_json::to_string(&error).expect("JSON serialization infallible; qed"); + from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, error, JSON) + } + /// Create a text/plain response for not allowed hosts. pub fn host_not_allowed() -> HttpResponse { from_template(hyper::StatusCode::FORBIDDEN, "Provided Host header is not whitelisted.\n", TEXT) @@ -174,11 +180,6 @@ pub mod response { from_template(hyper::StatusCode::OK, body, JSON) } - /// Create a general internal error response. - pub fn error_response(body: impl Into) -> HttpResponse { - from_template(hyper::StatusCode::INTERNAL_SERVER_ERROR, body, JSON) - } - /// Create a response for unsupported content type. pub fn unsupported_content_type() -> HttpResponse { from_template( diff --git a/types/src/error.rs b/types/src/error.rs index 0388d578a6..4617e8e318 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -39,6 +39,7 @@ pub type ErrorObjectOwned = ErrorObject<'static>; /// [Failed JSON-RPC response object](https://www.jsonrpc.org/specification#response_object). #[derive(Debug, Deserialize, Serialize, Clone, thiserror::Error)] #[serde(deny_unknown_fields)] +#[error("{self:?}")] pub struct ErrorObject<'a> { /// Code code: ErrorCode, @@ -95,12 +96,6 @@ impl<'a> ErrorObject<'a> { } } -impl fmt::Display for ErrorObject<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}: {}", self.code(), self.message()) - } -} - impl PartialEq for ErrorObject<'_> { fn eq(&self, other: &Self) -> bool { let this_raw = self.data.as_ref().map(|r| r.get()); From 0d1203bc59e23036075d7709a04994729ab5f9d9 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 10 Jan 2025 18:04:50 +0100 Subject: [PATCH 05/11] Update proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr --- proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr | 1 - 1 file changed, 1 deletion(-) diff --git a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr index bbb0fd0107..9fad2d19b2 100644 --- a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr +++ b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr @@ -17,7 +17,6 @@ error[E0277]: the trait bound `::Hash: Clone` is not satisfied | = note: required for `Result<::Hash, ErrorObject<'_>>` to implement `IntoResponse` = note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider removing this method call, as the receiver has type `&Self` and `&Self: Clone` trivially holds | 9 | #[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())] | From d28d63574416e0a15d66a22ad5f06789a30e4ce0 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 10 Jan 2025 18:04:56 +0100 Subject: [PATCH 06/11] Update proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr --- proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr | 1 - 1 file changed, 1 deletion(-) diff --git a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr index 9fad2d19b2..4da61ec016 100644 --- a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr +++ b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr @@ -17,7 +17,6 @@ error[E0277]: the trait bound `::Hash: Clone` is not satisfied | = note: required for `Result<::Hash, ErrorObject<'_>>` to implement `IntoResponse` = note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info) - | 9 | #[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())] | From 2593681a4853434223e80eeebe4ce08ee40c9492 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 10 Jan 2025 18:05:11 +0100 Subject: [PATCH 07/11] Update proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr --- proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr | 1 - 1 file changed, 1 deletion(-) diff --git a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr index 4da61ec016..5b8775d844 100644 --- a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr +++ b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr @@ -17,7 +17,6 @@ error[E0277]: the trait bound `::Hash: Clone` is not satisfied | = note: required for `Result<::Hash, ErrorObject<'_>>` to implement `IntoResponse` = note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info) -9 | #[rpc(server, client, namespace = "foo", client_bounds(), server_bounds())] | error[E0277]: the trait bound `::Hash: DeserializeOwned` is not satisfied From 9ec0250c5fc92892fa6990a143af17021bd4ae6b Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 10 Jan 2025 18:05:24 +0100 Subject: [PATCH 08/11] Update proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr --- proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr | 1 - 1 file changed, 1 deletion(-) diff --git a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr index 5b8775d844..64d05937b1 100644 --- a/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr +++ b/proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr @@ -17,7 +17,6 @@ error[E0277]: the trait bound `::Hash: Clone` is not satisfied | = note: required for `Result<::Hash, ErrorObject<'_>>` to implement `IntoResponse` = note: this error originates in the attribute macro `rpc` (in Nightly builds, run with -Z macro-backtrace for more info) - | error[E0277]: the trait bound `::Hash: DeserializeOwned` is not satisfied --> tests/ui/incorrect/rpc/rpc_empty_bounds.rs:9:1 From bf70a0245e3c6fe25a26f5717309afadd32e8856 Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Fri, 10 Jan 2025 15:01:07 -0300 Subject: [PATCH 09/11] Update server/src/middleware/http/proxy_get_request.rs Co-authored-by: Niklas Adolfsson --- server/src/middleware/http/proxy_get_request.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/middleware/http/proxy_get_request.rs b/server/src/middleware/http/proxy_get_request.rs index 26e1c3e265..1d83b89301 100644 --- a/server/src/middleware/http/proxy_get_request.rs +++ b/server/src/middleware/http/proxy_get_request.rs @@ -188,7 +188,7 @@ where #[derive(serde::Deserialize, serde::Serialize, Debug)] struct ErrorResponse<'a> { #[serde(borrow)] - error: &'a serde_json::value::RawValue, + error: ErrorObject<'a>, } let response = if let Ok(payload) = serde_json::from_slice::(&bytes) { From 86c5dcade22b1b57115203189a4bd5ffee65dfad Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Fri, 10 Jan 2025 20:19:47 +0000 Subject: [PATCH 10/11] simplify deserialization --- server/src/middleware/http/proxy_get_request.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/middleware/http/proxy_get_request.rs b/server/src/middleware/http/proxy_get_request.rs index 1d83b89301..1538a9fb88 100644 --- a/server/src/middleware/http/proxy_get_request.rs +++ b/server/src/middleware/http/proxy_get_request.rs @@ -195,7 +195,7 @@ where http::response::ok_response(payload.result.to_string()) } else { let error = serde_json::from_slice::(&bytes) - .and_then(|payload| serde_json::from_str::(&payload.error.to_string())) + .map(|payload| payload.error) .unwrap_or_else(|_| ErrorObject::from(ErrorCode::InternalError)); http::response::error_response(error) }; From 926424a537d6789899de43e3f6ac209fc3bd916a Mon Sep 17 00:00:00 2001 From: Daniel Freire Date: Fri, 10 Jan 2025 20:22:38 +0000 Subject: [PATCH 11/11] remove unused derives --- server/src/middleware/http/proxy_get_request.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/middleware/http/proxy_get_request.rs b/server/src/middleware/http/proxy_get_request.rs index 1538a9fb88..0c48000501 100644 --- a/server/src/middleware/http/proxy_get_request.rs +++ b/server/src/middleware/http/proxy_get_request.rs @@ -179,13 +179,13 @@ where bytes.extend(data); } - #[derive(serde::Deserialize, serde::Serialize, Debug)] + #[derive(serde::Deserialize)] struct SuccessResponse<'a> { #[serde(borrow)] result: &'a serde_json::value::RawValue, } - #[derive(serde::Deserialize, serde::Serialize, Debug)] + #[derive(serde::Deserialize)] struct ErrorResponse<'a> { #[serde(borrow)] error: ErrorObject<'a>,