Skip to content
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

enha: send original error response in ProxyGetRequest #1516

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions proc-macros/tests/ui/incorrect/rpc/rpc_empty_bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ error[E0277]: the trait bound `<Conf as Config>::Hash: Clone` is not satisfied
|
= note: required for `Result<<Conf as Config>::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)
|
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved

error[E0277]: the trait bound `<Conf as Config>::Hash: DeserializeOwned` is not satisfied
--> tests/ui/incorrect/rpc/rpc_empty_bounds.rs:9:1
Expand Down
19 changes: 14 additions & 5 deletions server/src/middleware/http/proxy_get_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{ErrorCode, ErrorObject, Id, RequestSer};
use std::collections::HashMap;
use std::future::Future;
use std::pin::Pin;
Expand Down Expand Up @@ -179,16 +179,25 @@ 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::<RpcPayload>(&bytes) {
#[derive(serde::Deserialize, serde::Serialize, Debug)]
struct ErrorResponse<'a> {
#[serde(borrow)]
error: &'a serde_json::value::RawValue,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
error: &'a serde_json::value::RawValue,
error: ErrorObject<'a>,

Copy link
Contributor Author

@dcfreire dcfreire Jan 10, 2025

Choose a reason for hiding this comment

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

I applied this by mistake, if it is ok here to directly deserialize to the error object, why not also directly deserialize the result in the success response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I think in this case we can just do serde_json::from_slice::<ErrorObject>(&bytes) and not have the ErrorResponse struct at all

Copy link
Contributor Author

@dcfreire dcfreire Jan 10, 2025

Choose a reason for hiding this comment

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

Also I think in this case we can just do serde_json::from_slice::<ErrorObject>(&bytes)

This is incorrect, my bad

}

let response = if let Ok(payload) = serde_json::from_slice::<SuccessResponse>(&bytes) {
http::response::ok_response(payload.result.to_string())
} else {
http::response::internal_error()
let error = serde_json::from_slice::<ErrorResponse>(&bytes)
.and_then(|payload| serde_json::from_str::<ErrorObject>(&payload.error.to_string()))
dcfreire marked this conversation as resolved.
Show resolved Hide resolved
.unwrap_or_else(|_| ErrorObject::from(ErrorCode::InternalError));
http::response::error_response(error)
};

Ok(response)
Expand Down
8 changes: 7 additions & 1 deletion server/src/transport/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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)
Expand Down