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.

Sign up for GitHub

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

Jump to bottom

chore: remove direct tracing calls from proc-macros #1405

Merged
merged 5 commits into from
Jul 31, 2024
Merged

chore: remove direct tracing calls from proc-macros #1405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
27a672e
chore: remove direct tracing calls from proc-macros
DaniPopes Jun 11, 2024
2f216b8
chore: extract panic functions
DaniPopes Jun 11, 2024
80f3681
Merge branch 'master' into tracing-procmacros
DaniPopes Jul 5, 2024
eef7a7d
docs: add a comment explaining the use of `#[cold]`
DaniPopes Jul 5, 2024
29e1909
Merge branch 'master' into tracing-procmacros
niklasad1 Jul 31, 2024
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
5 changes: 5 additions & 0 deletions core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ pub type RpcResult<T> = std::result::Result<T, jsonrpsee_types::ErrorObjectOwned
/// Empty server `RpcParams` type to use while registering modules.
pub type EmptyServerParams = Vec<()>;

#[doc(hidden)]
mod proc_macros_support;

/// Re-exports for proc-macro library to not require any additional
/// dependencies to be explicitly added on the client side.
#[doc(hidden)]
Expand All @@ -80,6 +83,8 @@ pub mod __reexports {
cfg_client_or_server! {
pub use tokio;
}

pub use super::proc_macros_support::*;
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
}

pub use serde::{de::DeserializeOwned, Serialize};
Expand Down
26 changes: 26 additions & 0 deletions core/src/proc_macros_support.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use jsonrpsee_types::ErrorObjectOwned;

niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
// We're marking functions on the error paths as #[cold] to both reduce chance of inlining and to
// make the generated assembly slightly better.

#[cold]
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
pub fn log_fail_parse(arg_pat: &str, ty: &str, err: &ErrorObjectOwned, optional: bool) {
let optional = if optional { "optional " } else { "" };
tracing::debug!("Error parsing {optional}\"{arg_pat}\" as \"{ty}\": {err}");
}

#[cold]
pub fn log_fail_parse_as_object(err: &ErrorObjectOwned) {
tracing::debug!("Failed to parse JSON-RPC params as object: {err}");
}

#[cold]
pub fn panic_fail_serialize(param: &str, err: serde_json::Error) -> ! {
panic!("Parameter `{param}` cannot be serialized: {err}");
}

#[cfg(debug_assertions)]
#[cold]
pub fn panic_fail_register() -> ! {
panic!("RPC macro method names should never conflict. This is a bug, please report it.");
}
10 changes: 6 additions & 4 deletions proc-macros/src/render_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ impl RpcDescription {
let jsonrpsee = self.jsonrpsee_client_path.as_ref().unwrap();
let p = Ident::new(ILLEGAL_PARAM_NAME, proc_macro2::Span::call_site());

let reexports = self.jrps_client_item(quote! { core::__reexports });

if params.is_empty() {
return quote!({
#jsonrpsee::core::params::ArrayParams::new()
Expand Down Expand Up @@ -249,8 +251,8 @@ impl RpcDescription {
quote!({
let mut #p = #jsonrpsee::core::params::ObjectParams::new();
#(
if let Err(err) = #p.insert( #params_insert ) {
panic!("Parameter `{}` cannot be serialized: {:?}", stringify!( #params_insert ), err);
if let Err(err) = #p.insert(#params_insert) {
#reexports::panic_fail_serialize(stringify!(#params_insert), err);
}
)*
#p
Expand All @@ -263,8 +265,8 @@ impl RpcDescription {
quote!({
let mut #p = #jsonrpsee::core::params::ArrayParams::new();
#(
if let Err(err) = #p.insert( #params ) {
panic!("Parameter `{}` cannot be serialized: {:?}", stringify!( #params ), err);
if let Err(err) = #p.insert(#params) {
#reexports::panic_fail_serialize(stringify!(#params), err);
}
)*
#p
Expand Down
196 changes: 78 additions & 118 deletions proc-macros/src/render_server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,22 @@ impl RpcDescription {
})
}

/// Helper that will ignore results of `register_*` method calls, and panic if there have been
/// any errors in debug builds.
///
/// The debug assert is a safeguard should the contract that guarantees the method names to
/// never conflict in the macro be broken in the future.
fn handle_register_result(&self, tokens: TokenStream2) -> TokenStream2 {
let reexports = self.jrps_server_item(quote! { core::__reexports });
quote! {{
let _res = #tokens;
#[cfg(debug_assertions)]
if _res.is_err() {
#reexports::panic_fail_register();
}
DaniPopes marked this conversation as resolved.
Show resolved Hide resolved
}}
}

fn render_into_rpc(&self) -> Result<TokenStream2, syn::Error> {
let rpc_module = self.jrps_server_item(quote! { RpcModule });

Expand All @@ -121,18 +137,6 @@ impl RpcDescription {
}
};

/// Helper that will ignore results of `register_*` method calls, and panic
/// if there have been any errors in debug builds.
///
/// The debug assert is a safeguard should the contract that guarantees the method
/// names to never conflict in the macro be broken in the future.
fn handle_register_result(tokens: TokenStream2) -> TokenStream2 {
quote! {{
let res = #tokens;
debug_assert!(res.is_ok(), "RPC macro method names should never conflict, this is a bug, please report it.");
}}
}

let methods = self
.methods
.iter()
Expand All @@ -153,14 +157,14 @@ impl RpcDescription {

if method.signature.sig.asyncness.is_some() {
if method.with_extensions {
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_async_method(#rpc_method_name, |params, context, ext| async move {
#parsing
#into_response::into_response(context.as_ref().#rust_method_name(&ext, #params_seq).await)
})
})
} else {
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_async_method(#rpc_method_name, |params, context, _| async move {
#parsing
#into_response::into_response(context.as_ref().#rust_method_name(#params_seq).await)
Expand All @@ -172,14 +176,14 @@ impl RpcDescription {
if method.blocking { quote!(register_blocking_method) } else { quote!(register_method) };

if method.with_extensions {
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.#register_kind(#rpc_method_name, |params, context, ext| {
#parsing
#into_response::into_response(context.#rust_method_name(&ext, #params_seq))
})
})
} else {
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.#register_kind(#rpc_method_name, |params, context, _| {
#parsing
#into_response::into_response(context.#rust_method_name(#params_seq))
Expand Down Expand Up @@ -223,37 +227,37 @@ impl RpcDescription {

if sub.signature.sig.asyncness.is_some() {
if sub.with_extensions {
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_subscription(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut pending, context, ext| async move {
#parsing
#into_sub_response::into_response(context.as_ref().#rust_method_name(pending, &ext, #params_seq).await)
})
})
} else {
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_subscription(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut pending, context, _| async move {
#parsing
#into_sub_response::into_response(context.as_ref().#rust_method_name(pending, #params_seq).await)
})
})
}
} else if sub.with_extensions {
handle_register_result(quote! {
rpc.register_subscription_raw(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut pending, context, ext| {
#parsing
let _ = context.as_ref().#rust_method_name(pending, &ext, #params_seq);
#sub_err::None
})
self.handle_register_result(quote! {
rpc.register_subscription_raw(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut pending, context, ext| {
#parsing
let _ = context.as_ref().#rust_method_name(pending, &ext, #params_seq);
#sub_err::None
})
})
} else {
handle_register_result(quote! {
rpc.register_subscription_raw(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut pending, context, _| {
#parsing
let _ = context.as_ref().#rust_method_name(pending, #params_seq);
#sub_err::None
})
self.handle_register_result(quote! {
rpc.register_subscription_raw(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut pending, context, _| {
#parsing
let _ = context.as_ref().#rust_method_name(pending, #params_seq);
#sub_err::None
})
}
})
}
})
.collect::<Vec<_>>();

Expand All @@ -270,7 +274,7 @@ impl RpcDescription {
.iter()
.map(|alias| {
check_name(alias, rust_method_name.span());
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_alias(#alias, #rpc_name)
})
})
Expand All @@ -293,7 +297,7 @@ impl RpcDescription {
.iter()
.map(|alias| {
check_name(alias, rust_method_name.span());
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_alias(#alias, #sub_name)
})
})
Expand All @@ -303,7 +307,7 @@ impl RpcDescription {
.iter()
.map(|alias| {
check_name(alias, rust_method_name.span());
handle_register_result(quote! {
self.handle_register_result(quote! {
rpc.register_alias(#alias, #unsub_name)
})
})
Expand Down Expand Up @@ -350,59 +354,36 @@ impl RpcDescription {

let params_fields_seq = params.iter().map(RpcFnArg::arg_pat);
let params_fields = quote! { #(#params_fields_seq),* };
let tracing = self.jrps_server_item(quote! { tracing });
let sub_err = self.jrps_server_item(quote! { SubscriptionCloseResponse });
let response_payload = self.jrps_server_item(quote! { ResponsePayload });
let tokio = self.jrps_server_item(quote! { core::__reexports::tokio });

let reexports = self.jrps_server_item(quote! { core::__reexports });

let error_ret = if let Some(pending) = &sub {
let tokio = quote! { #reexports::tokio };
let sub_err = self.jrps_server_item(quote! { SubscriptionCloseResponse });
quote! {
#tokio::spawn(#pending.reject(e));
return #sub_err::None;
}
} else {
let response_payload = self.jrps_server_item(quote! { ResponsePayload });
quote! {
return #response_payload::error(e);
}
};

// Code to decode sequence of parameters from a JSON array.
let decode_array = {
let decode_fields = params.iter().map(|RpcFnArg { arg_pat, ty, .. }| match (is_option(ty), sub.as_ref()) {
(true, Some(pending)) => {
quote! {
let #arg_pat: #ty = match seq.optional_next() {
Ok(v) => v,
Err(e) => {
#tracing::debug!(concat!("Error parsing optional \"", stringify!(#arg_pat), "\" as \"", stringify!(#ty), "\": {:?}"), e);
#tokio::spawn(#pending.reject(e));
return #sub_err::None;
}
};
}
}
(true, None) => {
quote! {
let #arg_pat: #ty = match seq.optional_next() {
Ok(v) => v,
Err(e) => {
#tracing::debug!(concat!("Error parsing optional \"", stringify!(#arg_pat), "\" as \"", stringify!(#ty), "\": {:?}"), e);
return #response_payload::error(e);
}
};
}
}
(false, Some(pending)) => {
quote! {
let #arg_pat: #ty = match seq.next() {
Ok(v) => v,
Err(e) => {
#tracing::debug!(concat!("Error parsing optional \"", stringify!(#arg_pat), "\" as \"", stringify!(#ty), "\": {:?}"), e);
#tokio::spawn(#pending.reject(e));
return #sub_err::None;
}
};
}
}
(false, None) => {
quote! {
let #arg_pat: #ty = match seq.next() {
Ok(v) => v,
Err(e) => {
#tracing::debug!(concat!("Error parsing \"", stringify!(#arg_pat), "\" as \"", stringify!(#ty), "\": {:?}"), e);
return #response_payload::error(e);
}
};
}
let decode_fields = params.iter().map(|RpcFnArg { arg_pat, ty, .. }| {
let is_option = is_option(ty);
let next_method = if is_option { quote!(optional_next) } else { quote!(next) };
quote! {
let #arg_pat: #ty = match seq.#next_method() {
Ok(v) => v,
Err(e) => {
#reexports::log_fail_parse(stringify!(#arg_pat), stringify!(#ty), &e, #is_option);
#error_ret
}
};
}
});

Expand Down Expand Up @@ -447,43 +428,22 @@ impl RpcDescription {
let destruct = params.iter().map(RpcFnArg::arg_pat).map(|a| quote!(parsed.#a));
let types = params.iter().map(RpcFnArg::ty);

if let Some(pending) = sub {
quote! {
#[derive(#serde::Deserialize)]
#[serde(crate = #serde_crate)]
struct ParamsObject<#(#generics,)*> {
#(#fields)*
}

let parsed: ParamsObject<#(#types,)*> = match params.parse() {
Ok(p) => p,
Err(e) => {
#tracing::debug!("Failed to parse JSON-RPC params as object: {}", e);
#tokio::spawn(#pending.reject(e));
return #sub_err::None;
}
};

(#(#destruct),*)
quote! {
#[derive(#serde::Deserialize)]
#[serde(crate = #serde_crate)]
struct ParamsObject<#(#generics,)*> {
#(#fields)*
}
} else {
quote! {
#[derive(#serde::Deserialize)]
#[serde(crate = #serde_crate)]
struct ParamsObject<#(#generics,)*> {
#(#fields)*
}

let parsed: ParamsObject<#(#types,)*> = match params.parse() {
Ok(p) => p,
Err(e) => {
#tracing::debug!("Failed to parse JSON-RPC params as object: {}", e);
return #response_payload::error(e);
}
};
let parsed: ParamsObject<#(#types,)*> = match params.parse() {
Ok(p) => p,
Err(e) => {
#reexports::log_fail_parse_as_object(&e);
#error_ret
}
};

(#(#destruct),*)
}
(#(#destruct),*)
}
};

Expand Down
Loading