From 0b231926e4d3813b954a0e293d9cae685798a42a Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Thu, 19 Jan 2023 13:59:36 +0530 Subject: [PATCH 01/11] Implement support for custom errors --- core/src/server/rpc_module.rs | 14 ++-- examples/examples/tokio_console.rs | 3 +- proc-macros/Cargo.toml | 1 + proc-macros/src/render_client.rs | 33 +++++++++- proc-macros/tests/ui/correct/errors.rs | 88 ++++++++++++++++++++++++++ server/src/tests/helpers.rs | 10 +-- server/src/tests/http.rs | 4 +- tests/tests/helpers.rs | 2 +- tests/tests/resource_limiting.rs | 6 +- tests/tests/rpc_module.rs | 2 +- 10 files changed, 143 insertions(+), 20 deletions(-) create mode 100644 proc-macros/tests/ui/correct/errors.rs diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 8a3a4d5ae6..b08cfe797a 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -569,14 +569,15 @@ impl RpcModule { } /// Register a new asynchronous RPC method, which computes the response with the given callback. - pub fn register_async_method( + pub fn register_async_method( &mut self, method_name: &'static str, callback: Fun, ) -> Result where R: Serialize + Send + Sync + 'static, - Fut: Future> + Send, + E: for<'a> Into, + Fut: Future> + Send, Fun: (Fn(Params<'static>, Arc) -> Fut) + Clone + Send + Sync + 'static, { let ctx = self.ctx.clone(); @@ -589,7 +590,7 @@ impl RpcModule { let future = async move { let result = match callback(params, ctx).await { Ok(res) => MethodResponse::response(id, res, max_response_size), - Err(err) => MethodResponse::error(id, err), + Err(err) => MethodResponse::error(id, err.into()), }; // Release claimed resources @@ -606,7 +607,7 @@ impl RpcModule { /// Register a new **blocking** synchronous RPC method, which computes the response with the given callback. /// Unlike the regular [`register_method`](RpcModule::register_method), this method can block its thread and perform expensive computations. - pub fn register_blocking_method( + pub fn register_blocking_method( &mut self, method_name: &'static str, callback: F, @@ -614,7 +615,8 @@ impl RpcModule { where Context: Send + Sync + 'static, R: Serialize, - F: Fn(Params, Arc) -> Result + Clone + Send + Sync + 'static, + E: for<'a> Into, + F: Fn(Params, Arc) -> Result + Clone + Send + Sync + 'static, { let ctx = self.ctx.clone(); let callback = self.methods.verify_and_insert( @@ -626,7 +628,7 @@ impl RpcModule { tokio::task::spawn_blocking(move || { let result = match callback(params, ctx) { Ok(result) => MethodResponse::response(id, result, max_response_size), - Err(err) => MethodResponse::error(id, err), + Err(err) => MethodResponse::error(id, err.into()), }; // Release claimed resources diff --git a/examples/examples/tokio_console.rs b/examples/examples/tokio_console.rs index 03310c1d5f..7d826fb059 100644 --- a/examples/examples/tokio_console.rs +++ b/examples/examples/tokio_console.rs @@ -36,6 +36,7 @@ use std::net::SocketAddr; +use jsonrpsee::core::Error; use jsonrpsee::server::ServerBuilder; use jsonrpsee::RpcModule; @@ -55,7 +56,7 @@ async fn run_server() -> anyhow::Result { module.register_method("memory_call", |_, _| Ok("A".repeat(1024 * 1024)))?; module.register_async_method("sleep", |_, _| async { tokio::time::sleep(std::time::Duration::from_millis(100)).await; - Ok("lo") + Result::<_, Error>::Ok("lo") })?; let addr = server.local_addr()?; diff --git a/proc-macros/Cargo.toml b/proc-macros/Cargo.toml index 091b0805d1..79b5fc4c86 100644 --- a/proc-macros/Cargo.toml +++ b/proc-macros/Cargo.toml @@ -25,3 +25,4 @@ trybuild = "1.0" tokio = { version = "1.16", features = ["rt", "macros"] } futures-channel = { version = "0.3.14", default-features = false } futures-util = { version = "0.3.14", default-features = false } +serde_json = "1" diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index 652f3486ff..4d8bf39d34 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -28,7 +28,7 @@ use crate::helpers::generate_where_clause; use crate::rpc_macro::{RpcDescription, RpcMethod, RpcSubscription}; use proc_macro2::TokenStream as TokenStream2; use quote::quote; -use syn::{FnArg, Pat, PatIdent, PatType, TypeParam}; +use syn::{AngleBracketedGenericArguments, FnArg, GenericArgument, Pat, PatIdent, PatType, PathArguments, TypeParam}; impl RpcDescription { pub(super) fn render_client(&self) -> Result { @@ -68,6 +68,36 @@ impl RpcDescription { Ok(trait_impl) } + fn patch_result_error(&self, ty: &syn::Type) -> syn::Type { + let mut path = match ty { + syn::Type::Path(path) => path.clone(), + _ => panic!("Client only supports bare or Result values: {:?}", ty), + }; + + let Some(first_segment) = path.path.segments.first_mut() else { + return syn::Type::Path(path); + }; + + if first_segment.ident != "Result" { + return syn::Type::Path(path); + } + + let args = match first_segment.arguments { + PathArguments::AngleBracketed(AngleBracketedGenericArguments { ref mut args, .. }) => args, + _ => unreachable!("Unexpected Result structure"), + }; + + let error = args.last_mut().unwrap(); + let error_type = match error { + GenericArgument::Type(error_type) => error_type, + _ => unreachable!("Unexpected Result structure"), + }; + + *error_type = syn::Type::Verbatim(self.jrps_client_item(quote! { core::Error })); + + syn::Type::Path(path) + } + fn render_method(&self, method: &RpcMethod) -> Result { // `jsonrpsee::Error` let jrps_error = self.jrps_client_item(quote! { core::Error }); @@ -83,6 +113,7 @@ impl RpcDescription { // `returns` represent the return type of the *rust method* (`Result< <..>, jsonrpsee::core::Error`). let (called_method, returns) = if let Some(returns) = &method.returns { let called_method = quote::format_ident!("request"); + let returns = self.patch_result_error(returns); let returns = quote! { #returns }; (called_method, returns) diff --git a/proc-macros/tests/ui/correct/errors.rs b/proc-macros/tests/ui/correct/errors.rs new file mode 100644 index 0000000000..da5e57af8a --- /dev/null +++ b/proc-macros/tests/ui/correct/errors.rs @@ -0,0 +1,88 @@ +//! Example of using custom errors. + +use std::net::SocketAddr; + +use jsonrpsee::core::async_trait; +use jsonrpsee::proc_macros::rpc; +use jsonrpsee::server::ServerBuilder; +use jsonrpsee::ws_client::*; + +pub enum CustomError { + One, + Two { custom_data: u32 }, +} + +impl From for jsonrpsee::core::Error { + fn from(err: CustomError) -> Self { + let code = match &err { + CustomError::One => 101, + CustomError::Two { .. } => 102, + }; + let data = match &err { + CustomError::One => None, + CustomError::Two { custom_data } => Some(serde_json::json!({ "customData": custom_data })), + }; + + let data = data.map(|val| serde_json::value::to_raw_value(&val).unwrap()); + + let error_object = jsonrpsee::types::ErrorObjectOwned::owned(code, "custom_error", data); + + Self::Call(jsonrpsee::types::error::CallError::Custom(error_object)) + } +} + +#[rpc(client, server, namespace = "foo")] +pub trait Rpc { + #[method(name = "method1")] + async fn method1(&self) -> Result; + + #[method(name = "method2")] + async fn method2(&self) -> Result; +} + +pub struct RpcServerImpl; + +#[async_trait] +impl RpcServer for RpcServerImpl { + async fn method1(&self) -> Result { + Err(CustomError::One) + } + + async fn method2(&self) -> Result { + Err(CustomError::Two { custom_data: 123 }) + } +} + +pub async fn server() -> SocketAddr { + let server = ServerBuilder::default().build("127.0.0.1:0").await.unwrap(); + let addr = server.local_addr().unwrap(); + let server_handle = server.start(RpcServerImpl.into_rpc()).unwrap(); + + tokio::spawn(server_handle.stopped()); + + addr +} + +#[tokio::main] +async fn main() { + let server_addr = server().await; + let server_url = format!("ws://{}", server_addr); + let client = WsClientBuilder::default().build(&server_url).await.unwrap(); + + let get_error_object = |err| match err { + jsonrpsee::core::Error::Call(jsonrpsee::types::error::CallError::Custom(object)) => object, + _ => panic!("wrong error kind: {:?}", err), + }; + + let error = client.method1().await.unwrap_err(); + let error_object = get_error_object(error); + assert_eq!(error_object.code(), 101); + assert_eq!(error_object.message(), "custom_error"); + assert!(error_object.data().is_none()); + + let error = client.method2().await.unwrap_err(); + let error_object = get_error_object(error); + assert_eq!(error_object.code(), 102); + assert_eq!(error_object.message(), "custom_error"); + assert_eq!(error_object.data().unwrap().get(), r#"{"customData":123}"#); +} diff --git a/server/src/tests/helpers.rs b/server/src/tests/helpers.rs index 1330a0b819..6259d6697b 100644 --- a/server/src/tests/helpers.rs +++ b/server/src/tests/helpers.rs @@ -59,7 +59,7 @@ pub(crate) async fn server_with_handles() -> (SocketAddr, ServerHandle) { tracing::debug!("server respond to hello"); // Call some async function inside. futures_util::future::ready(()).await; - Ok("hello") + Result::<_, Error>::Ok("hello") } }) .unwrap(); @@ -67,7 +67,7 @@ pub(crate) async fn server_with_handles() -> (SocketAddr, ServerHandle) { .register_async_method("add_async", |params, _| async move { let params: Vec = params.parse()?; let sum: u64 = params.into_iter().sum(); - Ok(sum) + Result::<_, Error>::Ok(sum) }) .unwrap(); module @@ -111,7 +111,7 @@ pub(crate) async fn server_with_handles() -> (SocketAddr, ServerHandle) { module .register_async_method("should_ok_async", |_p, ctx| async move { ctx.ok().map_err(CallError::Failed)?; - Ok("ok") + Result::<_, Error>::Ok("ok") }) .unwrap(); @@ -146,7 +146,7 @@ pub(crate) async fn server_with_context() -> SocketAddr { .register_async_method("should_ok_async", |_p, ctx| async move { ctx.ok().map_err(CallError::Failed)?; // Call some async function inside. - Ok(futures_util::future::ready("ok!").await) + Result::<_, Error>::Ok(futures_util::future::ready("ok!").await) }) .unwrap(); @@ -154,7 +154,7 @@ pub(crate) async fn server_with_context() -> SocketAddr { .register_async_method("err_async", |_p, ctx| async move { ctx.ok().map_err(CallError::Failed)?; // Async work that returns an error - futures_util::future::err::<(), _>(anyhow!("nah").into()).await + futures_util::future::err::<(), Error>(anyhow!("nah").into()).await }) .unwrap(); diff --git a/server/src/tests/http.rs b/server/src/tests/http.rs index 1a3bfdbd53..de33fa5d8a 100644 --- a/server/src/tests/http.rs +++ b/server/src/tests/http.rs @@ -46,7 +46,7 @@ async fn server() -> (SocketAddr, ServerHandle) { let mut module = RpcModule::new(ctx); let addr = server.local_addr().unwrap(); module.register_method("say_hello", |_, _| Ok("lo")).unwrap(); - module.register_async_method("say_hello_async", |_, _| async move { Ok("lo") }).unwrap(); + module.register_async_method("say_hello_async", |_, _| async move { Result::<_, Error>::Ok("lo") }).unwrap(); module .register_method("add", |params, _| { let params: Vec = params.parse()?; @@ -78,7 +78,7 @@ async fn server() -> (SocketAddr, ServerHandle) { module .register_async_method("should_ok_async", |_p, ctx| async move { ctx.ok().map_err(CallError::Failed)?; - Ok("ok") + Result::<_, Error>::Ok("ok") }) .unwrap(); diff --git a/tests/tests/helpers.rs b/tests/tests/helpers.rs index f81176b529..2009167a1f 100644 --- a/tests/tests/helpers.rs +++ b/tests/tests/helpers.rs @@ -196,7 +196,7 @@ pub async fn server() -> SocketAddr { module .register_async_method("slow_hello", |_, _| async { tokio::time::sleep(std::time::Duration::from_secs(1)).await; - Ok("hello") + Result::<_, Error>::Ok("hello") }) .unwrap(); diff --git a/tests/tests/resource_limiting.rs b/tests/tests/resource_limiting.rs index 27650490f5..14bfd11a2b 100644 --- a/tests/tests/resource_limiting.rs +++ b/tests/tests/resource_limiting.rs @@ -46,20 +46,20 @@ fn module_manual() -> Result, Error> { module.register_async_method("say_hello", |_, _| async move { sleep(Duration::from_millis(50)).await; - Ok("hello") + Result::<_, Error>::Ok("hello") })?; module .register_async_method("expensive_call", |_, _| async move { sleep(Duration::from_millis(50)).await; - Ok("hello expensive call") + Result::<_, Error>::Ok("hello expensive call") })? .resource("CPU", 3)?; module .register_async_method("memory_hog", |_, _| async move { sleep(Duration::from_millis(50)).await; - Ok("hello memory hog") + Result::<_, Error>::Ok("hello memory hog") })? .resource("CPU", 0)? .resource("MEM", 8)?; diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index 9382de1248..28b6b38653 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -126,7 +126,7 @@ async fn calling_method_without_server() { module .register_async_method("roo", |params, ctx| { let ns: Vec = params.parse().expect("valid params please"); - async move { Ok(ctx.roo(ns)) } + async move { Result::<_, Error>::Ok(ctx.roo(ns)) } }) .unwrap(); let res: u64 = module.call("roo", [12, 13]).await.unwrap(); From 8a6091dfa388b70dd8f4d9a8a9f558dc08540c58 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Fri, 20 Jan 2023 22:41:58 +0530 Subject: [PATCH 02/11] Remove unneded for<'a> from E bound --- core/src/server/rpc_module.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index b08cfe797a..8a0fff850d 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -576,7 +576,7 @@ impl RpcModule { ) -> Result where R: Serialize + Send + Sync + 'static, - E: for<'a> Into, + E: Into, Fut: Future> + Send, Fun: (Fn(Params<'static>, Arc) -> Fut) + Clone + Send + Sync + 'static, { @@ -615,7 +615,7 @@ impl RpcModule { where Context: Send + Sync + 'static, R: Serialize, - E: for<'a> Into, + E: Into, F: Fn(Params, Arc) -> Result + Clone + Send + Sync + 'static, { let ctx = self.ctx.clone(); From fd642eb44074b7878de8adc150c48fea98e695d5 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Fri, 20 Jan 2023 22:41:07 +0530 Subject: [PATCH 03/11] Fix doctest --- core/src/server/resource_limiting.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/server/resource_limiting.rs b/core/src/server/resource_limiting.rs index 685712caab..e50398b82c 100644 --- a/core/src/server/resource_limiting.rs +++ b/core/src/server/resource_limiting.rs @@ -75,7 +75,7 @@ //! module //! .register_async_method("my_expensive_method", |_, _| async move { //! // Do work -//! Ok("hello") +//! Result::<_, jsonrpsee::core::Error>::Ok("hello") //! })? //! .resource("cpu", 5)? //! .resource("mem", 2)?; From 1b368fd0cd27d8ef251a8f1c8f4a4e50888cf2d6 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Fri, 20 Jan 2023 23:10:45 +0530 Subject: [PATCH 04/11] Handle the case where there are not exactly two arguments --- proc-macros/src/render_client.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index 4d8bf39d34..2b58347127 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -87,6 +87,11 @@ impl RpcDescription { _ => unreachable!("Unexpected Result structure"), }; + if args.len() != 2 { + // Unexpected number of type arguments, just leave it as-is. + return syn::Type::Path(path); + } + let error = args.last_mut().unwrap(); let error_type = match error { GenericArgument::Type(error_type) => error_type, From ea833bc28e00dd5f0df62f689f533b9e8c72a06d Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Fri, 20 Jan 2023 23:48:51 +0530 Subject: [PATCH 05/11] Support for other Result paths --- proc-macros/src/helpers.rs | 22 ++++++++++++++++++++++ proc-macros/src/render_client.rs | 23 ++++++++++++++--------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/proc-macros/src/helpers.rs b/proc-macros/src/helpers.rs index 5e5492c1bb..b7e80cebbe 100644 --- a/proc-macros/src/helpers.rs +++ b/proc-macros/src/helpers.rs @@ -192,6 +192,28 @@ pub(crate) fn extract_doc_comments(attrs: &[syn::Attribute]) -> TokenStream2 { quote! ( #(#docs)* ) } +/// Extract the mutable path segment from the given path for the given valid paths. +pub(crate) fn path_segment_mut<'path, 'valid>( + path: &'path mut syn::Path, + valid_paths: impl IntoIterator + 'valid, +) -> Option<&'path mut syn::PathSegment> { + for valids_path in valid_paths { + if valids_path.len() != path.segments.len() { + continue; + } + let segments_match = path + .segments + .iter_mut() + .zip(valids_path) + .all(|(actual_segment, valid_path_segment)| actual_segment.ident == valid_path_segment); + if !segments_match { + continue; + } + return path.segments.last_mut(); + } + None +} + #[cfg(test)] mod tests { use super::is_option; diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index 2b58347127..c94ea8d03c 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -24,7 +24,7 @@ // IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. use crate::attributes::ParamKind; -use crate::helpers::generate_where_clause; +use crate::helpers::{generate_where_clause, path_segment_mut}; use crate::rpc_macro::{RpcDescription, RpcMethod, RpcSubscription}; use proc_macro2::TokenStream as TokenStream2; use quote::quote; @@ -68,28 +68,33 @@ impl RpcDescription { Ok(trait_impl) } + /// Alter the error varaint (`Err`) type of the [`Result`], or return the type as-is if it is not a [`Result`]. + /// Intended for rewriting the retuirn type of RPC methods for clients, where the return type is defined + /// as a [`Result`] with a custom error. Clients can not actually benefit from custom errors right now, so + /// we just rewrtie the type to be the good old [`core::Error`]. fn patch_result_error(&self, ty: &syn::Type) -> syn::Type { - let mut path = match ty { + let mut type_path = match ty { syn::Type::Path(path) => path.clone(), _ => panic!("Client only supports bare or Result values: {:?}", ty), }; - let Some(first_segment) = path.path.segments.first_mut() else { - return syn::Type::Path(path); + let valids_paths = [&["Result"][..], &["std", "result", "Result"][..], &["core", "result", "Result"][..]]; + let Some(result_segment) = path_segment_mut(&mut type_path.path, valids_paths) else { + return syn::Type::Path(type_path); }; - if first_segment.ident != "Result" { - return syn::Type::Path(path); + if result_segment.ident != "Result" { + return syn::Type::Path(type_path); } - let args = match first_segment.arguments { + let args = match result_segment.arguments { PathArguments::AngleBracketed(AngleBracketedGenericArguments { ref mut args, .. }) => args, _ => unreachable!("Unexpected Result structure"), }; if args.len() != 2 { // Unexpected number of type arguments, just leave it as-is. - return syn::Type::Path(path); + return syn::Type::Path(type_path); } let error = args.last_mut().unwrap(); @@ -100,7 +105,7 @@ impl RpcDescription { *error_type = syn::Type::Verbatim(self.jrps_client_item(quote! { core::Error })); - syn::Type::Path(path) + syn::Type::Path(type_path) } fn render_method(&self, method: &RpcMethod) -> Result { From 648438b79d5011b11383dd5720cf89ccbc6b15d3 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 21 Jan 2023 00:15:38 +0530 Subject: [PATCH 06/11] Rewrite with a more explicit rewriting logic --- proc-macros/src/helpers.rs | 22 ---------- proc-macros/src/render_client.rs | 71 ++++++++++++++++---------------- 2 files changed, 36 insertions(+), 57 deletions(-) diff --git a/proc-macros/src/helpers.rs b/proc-macros/src/helpers.rs index b7e80cebbe..5e5492c1bb 100644 --- a/proc-macros/src/helpers.rs +++ b/proc-macros/src/helpers.rs @@ -192,28 +192,6 @@ pub(crate) fn extract_doc_comments(attrs: &[syn::Attribute]) -> TokenStream2 { quote! ( #(#docs)* ) } -/// Extract the mutable path segment from the given path for the given valid paths. -pub(crate) fn path_segment_mut<'path, 'valid>( - path: &'path mut syn::Path, - valid_paths: impl IntoIterator + 'valid, -) -> Option<&'path mut syn::PathSegment> { - for valids_path in valid_paths { - if valids_path.len() != path.segments.len() { - continue; - } - let segments_match = path - .segments - .iter_mut() - .zip(valids_path) - .all(|(actual_segment, valid_path_segment)| actual_segment.ident == valid_path_segment); - if !segments_match { - continue; - } - return path.segments.last_mut(); - } - None -} - #[cfg(test)] mod tests { use super::is_option; diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index c94ea8d03c..8b020a7a01 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -24,11 +24,12 @@ // IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. use crate::attributes::ParamKind; -use crate::helpers::{generate_where_clause, path_segment_mut}; +use crate::helpers::generate_where_clause; use crate::rpc_macro::{RpcDescription, RpcMethod, RpcSubscription}; use proc_macro2::TokenStream as TokenStream2; -use quote::quote; -use syn::{AngleBracketedGenericArguments, FnArg, GenericArgument, Pat, PatIdent, PatType, PathArguments, TypeParam}; +use quote::{quote, quote_spanned}; +use syn::spanned::Spanned; +use syn::{AngleBracketedGenericArguments, FnArg, Pat, PatIdent, PatType, PathArguments, TypeParam}; impl RpcDescription { pub(super) fn render_client(&self) -> Result { @@ -68,44 +69,44 @@ impl RpcDescription { Ok(trait_impl) } - /// Alter the error varaint (`Err`) type of the [`Result`], or return the type as-is if it is not a [`Result`]. - /// Intended for rewriting the retuirn type of RPC methods for clients, where the return type is defined - /// as a [`Result`] with a custom error. Clients can not actually benefit from custom errors right now, so - /// we just rewrtie the type to be the good old [`core::Error`]. - fn patch_result_error(&self, ty: &syn::Type) -> syn::Type { - let mut type_path = match ty { - syn::Type::Path(path) => path.clone(), - _ => panic!("Client only supports bare or Result values: {:?}", ty), + /// Verify and rewrite the return type (for methods). + fn return_result_type(&self, ty: &syn::Type) -> TokenStream2 { + // We expect a valid type path. + let syn::Type::Path(type_path) = ty else { + return quote_spanned!(ty.span() => compile_error!("Expecting something like 'Result' here. (1)")); }; - let valids_paths = [&["Result"][..], &["std", "result", "Result"][..], &["core", "result", "Result"][..]]; - let Some(result_segment) = path_segment_mut(&mut type_path.path, valids_paths) else { - return syn::Type::Path(type_path); + // The path (eg std::result::Result) should have a final segment like 'Result'. + let Some(type_name) = type_path.path.segments.last() else { + return quote_spanned!(ty.span() => compile_error!("Expecting this path to end in something like 'Result'")); }; - if result_segment.ident != "Result" { - return syn::Type::Path(type_path); - } - - let args = match result_segment.arguments { - PathArguments::AngleBracketed(AngleBracketedGenericArguments { ref mut args, .. }) => args, - _ => unreachable!("Unexpected Result structure"), - }; - - if args.len() != 2 { - // Unexpected number of type arguments, just leave it as-is. - return syn::Type::Path(type_path); - } - - let error = args.last_mut().unwrap(); - let error_type = match error { - GenericArgument::Type(error_type) => error_type, - _ => unreachable!("Unexpected Result structure"), + // Get the generic args eg the in Result. + let PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) = &type_name.arguments else { + return quote_spanned!(ty.span() => compile_error!("Expecting something like 'Result' here, but got no generic args (eg no '').")); }; - *error_type = syn::Type::Verbatim(self.jrps_client_item(quote! { core::Error })); + if type_name.ident == "Result" { + // Result should have 2 generic args. + if args.len() != 2 { + return quote_spanned!(args.span() => compile_error!("Expecting two generic args here.")); + } - syn::Type::Path(type_path) + // The first generic arg is our custom return type. + // Return a new Result with that custom type and jsonrpsee::core::Error: + let custom_return_type = args.first().unwrap(); + let error_type = self.jrps_client_item(quote! { core::Error }); + quote!(std::result::Result<#custom_return_type, #error_type>) + } else if type_name.ident == "RpcResult" { + // RpcResult (an alias we export) should have 1 generic arg. + if args.len() != 1 { + return quote_spanned!(args.span() => compile_error!("Expecting two generic args here.")); + } + quote!(#ty) + } else { + // Any other type name isn't allowed. + quote_spanned!(type_name.span() => compile_error!("The response type should be Result or RpcResult")) + } } fn render_method(&self, method: &RpcMethod) -> Result { @@ -123,7 +124,7 @@ impl RpcDescription { // `returns` represent the return type of the *rust method* (`Result< <..>, jsonrpsee::core::Error`). let (called_method, returns) = if let Some(returns) = &method.returns { let called_method = quote::format_ident!("request"); - let returns = self.patch_result_error(returns); + let returns = self.return_result_type(returns); let returns = quote! { #returns }; (called_method, returns) From fa3d48ca89845f7022743f0d6484c83458688b80 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 21 Jan 2023 00:29:24 +0530 Subject: [PATCH 07/11] Back to rewriting the error argument --- proc-macros/src/render_client.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index 8b020a7a01..130e863e60 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -70,19 +70,19 @@ impl RpcDescription { } /// Verify and rewrite the return type (for methods). - fn return_result_type(&self, ty: &syn::Type) -> TokenStream2 { + fn return_result_type(&self, mut ty: syn::Type) -> TokenStream2 { // We expect a valid type path. - let syn::Type::Path(type_path) = ty else { + let syn::Type::Path(ref mut type_path) = ty else { return quote_spanned!(ty.span() => compile_error!("Expecting something like 'Result' here. (1)")); }; // The path (eg std::result::Result) should have a final segment like 'Result'. - let Some(type_name) = type_path.path.segments.last() else { + let Some(type_name) = type_path.path.segments.last_mut() else { return quote_spanned!(ty.span() => compile_error!("Expecting this path to end in something like 'Result'")); }; // Get the generic args eg the in Result. - let PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) = &type_name.arguments else { + let PathArguments::AngleBracketed(AngleBracketedGenericArguments { args, .. }) = &mut type_name.arguments else { return quote_spanned!(ty.span() => compile_error!("Expecting something like 'Result' here, but got no generic args (eg no '').")); }; @@ -92,11 +92,11 @@ impl RpcDescription { return quote_spanned!(args.span() => compile_error!("Expecting two generic args here.")); } - // The first generic arg is our custom return type. - // Return a new Result with that custom type and jsonrpsee::core::Error: - let custom_return_type = args.first().unwrap(); - let error_type = self.jrps_client_item(quote! { core::Error }); - quote!(std::result::Result<#custom_return_type, #error_type>) + // Force the last argument to be `jsonrpsee::core::Error`: + let error_arg = args.last_mut().unwrap(); + *error_arg = syn::GenericArgument::Type(syn::Type::Verbatim(self.jrps_client_item(quote! { core::Error }))); + + quote!(#ty) } else if type_name.ident == "RpcResult" { // RpcResult (an alias we export) should have 1 generic arg. if args.len() != 1 { @@ -124,7 +124,7 @@ impl RpcDescription { // `returns` represent the return type of the *rust method* (`Result< <..>, jsonrpsee::core::Error`). let (called_method, returns) = if let Some(returns) = &method.returns { let called_method = quote::format_ident!("request"); - let returns = self.return_result_type(returns); + let returns = self.return_result_type(returns.clone()); let returns = quote! { #returns }; (called_method, returns) From 0b334a5e56f264f2c888789f7168e194318b24ff Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 21 Jan 2023 00:43:15 +0530 Subject: [PATCH 08/11] Add UI error for non-result --- .../ui/incorrect/method/method_non_result_return_type.rs | 9 +++++++++ .../method/method_non_result_return_type.stderr | 5 +++++ 2 files changed, 14 insertions(+) create mode 100644 proc-macros/tests/ui/incorrect/method/method_non_result_return_type.rs create mode 100644 proc-macros/tests/ui/incorrect/method/method_non_result_return_type.stderr diff --git a/proc-macros/tests/ui/incorrect/method/method_non_result_return_type.rs b/proc-macros/tests/ui/incorrect/method/method_non_result_return_type.rs new file mode 100644 index 0000000000..7ce91455bb --- /dev/null +++ b/proc-macros/tests/ui/incorrect/method/method_non_result_return_type.rs @@ -0,0 +1,9 @@ +use jsonrpsee::proc_macros::rpc; + +#[rpc(client)] +pub trait NonResultReturnType { + #[method(name = "a")] + async fn a(&self) -> u16; +} + +fn main() {} diff --git a/proc-macros/tests/ui/incorrect/method/method_non_result_return_type.stderr b/proc-macros/tests/ui/incorrect/method/method_non_result_return_type.stderr new file mode 100644 index 0000000000..c8d42bffe8 --- /dev/null +++ b/proc-macros/tests/ui/incorrect/method/method_non_result_return_type.stderr @@ -0,0 +1,5 @@ +error: Expecting something like 'Result' here, but got no generic args (eg no ''). + --> tests/ui/incorrect/method/method_non_result_return_type.rs:6:23 + | +6 | async fn a(&self) -> u16; + | ^^^ From 03cb9127d7a68e15e4d407b61cce0a60f2fc0631 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Tue, 24 Jan 2023 19:25:44 +0530 Subject: [PATCH 09/11] Apply suggestions from code review Co-authored-by: Niklas Adolfsson --- proc-macros/src/render_client.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index 130e863e60..27f7b448f5 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -89,7 +89,7 @@ impl RpcDescription { if type_name.ident == "Result" { // Result should have 2 generic args. if args.len() != 2 { - return quote_spanned!(args.span() => compile_error!("Expecting two generic args here.")); + return quote_spanned!(args.span() => compile_error!("Result must be have two arguments)); } // Force the last argument to be `jsonrpsee::core::Error`: @@ -100,12 +100,12 @@ impl RpcDescription { } else if type_name.ident == "RpcResult" { // RpcResult (an alias we export) should have 1 generic arg. if args.len() != 1 { - return quote_spanned!(args.span() => compile_error!("Expecting two generic args here.")); + return quote_spanned!(args.span() => compile_error!("RpcResult must have one argument")); } quote!(#ty) } else { // Any other type name isn't allowed. - quote_spanned!(type_name.span() => compile_error!("The response type should be Result or RpcResult")) + quote_spanned!(type_name.span() => compile_error!("The return type must be Result or RpcResult")) } } From 6faf51c97bf9e6e0ba3c9b46c2c56591e7270d0c Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Tue, 24 Jan 2023 20:41:55 +0530 Subject: [PATCH 10/11] Fix a typo --- proc-macros/src/render_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/render_client.rs b/proc-macros/src/render_client.rs index 27f7b448f5..08826767da 100644 --- a/proc-macros/src/render_client.rs +++ b/proc-macros/src/render_client.rs @@ -89,7 +89,7 @@ impl RpcDescription { if type_name.ident == "Result" { // Result should have 2 generic args. if args.len() != 2 { - return quote_spanned!(args.span() => compile_error!("Result must be have two arguments)); + return quote_spanned!(args.span() => compile_error!("Result must be have two arguments")); } // Force the last argument to be `jsonrpsee::core::Error`: From b7f24c0884589ce2113fc60bb53360f8f6a18cc3 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Tue, 24 Jan 2023 20:49:02 +0530 Subject: [PATCH 11/11] Fix errors in the rest of the targets --- benches/helpers.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/benches/helpers.rs b/benches/helpers.rs index 53ccef87f3..7935650d77 100644 --- a/benches/helpers.rs +++ b/benches/helpers.rs @@ -163,11 +163,17 @@ fn gen_rpc_module() -> jsonrpsee::RpcModule<()> { let mut module = jsonrpsee::RpcModule::new(()); module.register_method(SYNC_FAST_CALL, |_, _| Ok("lo")).unwrap(); - module.register_async_method(ASYNC_FAST_CALL, |_, _| async { Ok("lo") }).unwrap(); + module + .register_async_method(ASYNC_FAST_CALL, |_, _| async { Result::<_, jsonrpsee::core::Error>::Ok("lo") }) + .unwrap(); module.register_method(SYNC_MEM_CALL, |_, _| Ok("A".repeat(MIB))).unwrap(); - module.register_async_method(ASYNC_MEM_CALL, |_, _| async move { Ok("A".repeat(MIB)) }).unwrap(); + module + .register_async_method(ASYNC_MEM_CALL, |_, _| async move { + Result::<_, jsonrpsee::core::Error>::Ok("A".repeat(MIB)) + }) + .unwrap(); module .register_method(SYNC_SLOW_CALL, |_, _| { @@ -179,7 +185,7 @@ fn gen_rpc_module() -> jsonrpsee::RpcModule<()> { module .register_async_method(ASYNC_SLOW_CALL, |_, _| async move { tokio::time::sleep(SLOW_CALL).await; - Ok("slow call async") + Result::<_, jsonrpsee::core::Error>::Ok("slow call async") }) .unwrap();