From 107a4e96dc7af64fd6ad0a83de859f8664f8e90a Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Jun 2022 16:08:32 +0300 Subject: [PATCH 01/71] subscription: Allow errors in subscription callbacks Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index b387615b59..8777c0288c 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -384,6 +384,7 @@ impl Methods { /// let mut module = RpcModule::new(()); /// module.register_subscription("hi", "hi", "goodbye", |_, pending, _| { /// pending.accept().unwrap().send(&"one answer").unwrap(); + /// Ok(()) /// }).unwrap(); /// let (resp, mut stream) = module.raw_json_request(r#"{"jsonrpc":"2.0","method":"hi","id":0}"#).await.unwrap(); /// let resp = serde_json::from_str::>(&resp).unwrap(); @@ -445,6 +446,7 @@ impl Methods { /// let mut module = RpcModule::new(()); /// module.register_subscription("hi", "hi", "goodbye", |_, pending, _| { /// pending.accept().unwrap().send(&"one answer").unwrap(); + /// Ok(()) /// }).unwrap(); /// /// let mut sub = module.subscribe("hi", EmptyParams::new()).await.unwrap(); @@ -674,6 +676,8 @@ impl RpcModule { /// let sum = x + (*ctx); /// let _ = sink.send(&sum); /// }); + /// + /// Ok(()) /// }); /// ``` pub fn register_subscription( @@ -685,7 +689,7 @@ impl RpcModule { ) -> Result where Context: Send + Sync + 'static, - F: Fn(Params, PendingSubscription, Arc) + Send + Sync + 'static, + F: Fn(Params, PendingSubscription, Arc) -> Result<(), ErrorObjectOwned> + Send + Sync + 'static, { if subscribe_method_name == unsubscribe_method_name { return Err(Error::SubscriptionNameConflict(subscribe_method_name.into())); @@ -750,7 +754,7 @@ impl RpcModule { claimed, })); - callback(params, sink, ctx.clone()); + let _ = callback(params, sink, ctx.clone()); true })), @@ -915,6 +919,7 @@ impl SubscriptionSink { /// } /// } /// }); + /// Ok(()) /// }); /// ``` pub async fn pipe_from_try_stream(&mut self, mut stream: S) -> SubscriptionClosed @@ -987,6 +992,7 @@ impl SubscriptionSink { /// let mut sink = pending.accept().unwrap(); /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); /// tokio::spawn(async move { sink.pipe_from_stream(stream).await; }); + /// Ok(()) /// }); /// ``` pub async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed From e05f5a8f96dfb5df547af12b815c9cec507115e5 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 17 Jun 2022 17:24:46 +0300 Subject: [PATCH 02/71] subscription: Remove the need to own the error Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 8777c0288c..6298b07abb 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -689,7 +689,7 @@ impl RpcModule { ) -> Result where Context: Send + Sync + 'static, - F: Fn(Params, PendingSubscription, Arc) -> Result<(), ErrorObjectOwned> + Send + Sync + 'static, + F: Fn(Params, PendingSubscription, Arc) -> Result<(), ErrorObject> + Send + Sync + 'static, { if subscribe_method_name == unsubscribe_method_name { return Err(Error::SubscriptionNameConflict(subscribe_method_name.into())); From 866595f1d0da5695e945736342b170c7e307d971 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 15:25:54 +0300 Subject: [PATCH 03/71] error: Build `ErrorObject` from `CallError` for improved ergonomics Signed-off-by: Alexandru Vasile --- types/src/error.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/types/src/error.rs b/types/src/error.rs index 4db09994a9..2db9d53cc1 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -156,6 +156,16 @@ impl<'a> From for ErrorObject<'a> { } } +impl<'a> From for ErrorObject<'a> { + fn from(error: CallError) -> Self { + match error { + CallError::InvalidParams(e) => ErrorObject::owned(INVALID_PARAMS_CODE, e.to_string(), None::<()>), + CallError::Failed(e) => ErrorObject::owned(CALL_EXECUTION_FAILED_CODE, e.to_string(), None::<()>), + CallError::Custom(err) => err, + } + } +} + /// Parse error code. pub const PARSE_ERROR_CODE: i32 = -32700; /// Oversized request error code. From 52a9c412caaf5eb1f43e7740faf9b55c4af193e7 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 15:36:17 +0300 Subject: [PATCH 04/71] Update examples for the new subscription API Signed-off-by: Alexandru Vasile --- examples/examples/proc_macro.rs | 4 +++- examples/examples/ws_pubsub_broadcast.rs | 3 ++- examples/examples/ws_pubsub_with_params.rs | 17 +++++++++++------ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index 610789e53c..9e8c53d21d 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -28,6 +28,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, client::Subscription, Error}; use jsonrpsee::proc_macros::rpc; +use jsonrpsee::types::ErrorObjectOwned; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder, WsServerHandle}; @@ -45,7 +46,7 @@ where /// Subscription that takes a `StorageKey` as input and produces a `Vec`. #[subscription(name = "subscribeStorage" => "override", item = Vec)] - fn subscribe_storage(&self, keys: Option>); + fn subscribe_storage(&self, keys: Option>) -> Result<(), ErrorObjectOwned>; } pub struct RpcServerImpl; @@ -64,6 +65,7 @@ impl RpcServer for RpcServerImpl { if let Some(mut sink) = pending.accept() { let _ = sink.send(&vec![[0; 32]]); } + Ok(()) } } diff --git a/examples/examples/ws_pubsub_broadcast.rs b/examples/examples/ws_pubsub_broadcast.rs index 6c144fc430..f96b925a2b 100644 --- a/examples/examples/ws_pubsub_broadcast.rs +++ b/examples/examples/ws_pubsub_broadcast.rs @@ -75,7 +75,7 @@ async fn run_server() -> anyhow::Result { let rx = BroadcastStream::new(tx.clone().subscribe()); let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; tokio::spawn(async move { @@ -89,6 +89,7 @@ async fn run_server() -> anyhow::Result { } }; }); + Ok(()) })?; let addr = server.local_addr()?; server.start(module)?; diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index 432e9a144c..65dddf4b2c 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -67,9 +67,10 @@ async fn run_server() -> anyhow::Result { let mut module = RpcModule::new(()); module .register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, pending, _| { - let (idx, mut sink) = match (params.one(), pending.accept()) { - (Ok(idx), Some(sink)) => (idx, sink), - _ => return, + let idx = params.one()?; + let mut sink = match pending.accept() { + Some(sink) => sink, + _ => return Ok(()), }; let item = LETTERS.chars().nth(idx); @@ -88,13 +89,15 @@ async fn run_server() -> anyhow::Result { SubscriptionClosed::RemotePeerAborted => (), }; }); + Ok(()) }) .unwrap(); module .register_subscription("sub_params_two", "params_two", "unsub_params_two", |params, pending, _| { - let (one, two, mut sink) = match (params.parse::<(usize, usize)>(), pending.accept()) { - (Ok((one, two)), Some(sink)) => (one, two, sink), - _ => return, + let (one, two) = params.parse::<(usize, usize)>()?; + let mut sink = match pending.accept() { + Some(sink) => sink, + _ => return Ok(()), }; let item = &LETTERS[one..two]; @@ -114,6 +117,8 @@ async fn run_server() -> anyhow::Result { SubscriptionClosed::RemotePeerAborted => (), }; }); + + Ok(()) }) .unwrap(); From 6d25ec378e1ad76737deaf260ba51016699386d6 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 15:43:47 +0300 Subject: [PATCH 05/71] Add alias for subscription result Signed-off-by: Alexandru Vasile --- types/src/error.rs | 3 +++ types/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/types/src/error.rs b/types/src/error.rs index 2db9d53cc1..97aab49868 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -80,6 +80,9 @@ impl<'a> fmt::Display for ErrorResponse<'a> { } } +/// The return type of the subscription's method. +pub type ResultSubscription = Result<(), ErrorObjectOwned>; + /// Owned variant of [`ErrorObject`]. pub type ErrorObjectOwned = ErrorObject<'static>; diff --git a/types/src/lib.rs b/types/src/lib.rs index db81587161..ae330c17d3 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -43,7 +43,7 @@ pub mod response; /// JSON-RPC response error object related types. pub mod error; -pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse}; +pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ResultSubscription}; pub use params::{Id, Params, ParamsSequence, ParamsSer, SubscriptionId, TwoPointZero}; pub use request::{InvalidRequest, Notification, NotificationSer, Request, RequestSer}; pub use response::{Response, SubscriptionPayload, SubscriptionResponse}; From d5e0fc8133d0838afd927eaeee5c1c8e4364be77 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 19:37:15 +0300 Subject: [PATCH 06/71] macros: Render server subscription method with `ResultSubscription` Signed-off-by: Alexandru Vasile --- proc-macros/src/render_server.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 2f7807f66c..2868a720fd 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -32,7 +32,7 @@ use crate::helpers::{generate_where_clause, is_option}; use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{quote, quote_spanned}; use syn::punctuated::Punctuated; -use syn::Token; +use syn::{parse_quote, ReturnType, Token}; impl RpcDescription { pub(super) fn render_server(&self) -> Result { @@ -75,6 +75,12 @@ impl RpcDescription { // Add `SubscriptionSink` as the second input parameter to the signature. let subscription_sink: syn::FnArg = syn::parse_quote!(subscription_sink: #subscription_sink_ty); let mut sub_sig = sub.signature.clone(); + + // For ergonomic reasons, the server's subscription method should return `ResultSubscription`. + let return_ty = self.jrps_server_item(quote! { types::ResultSubscription }); + let output: ReturnType = parse_quote! { -> #return_ty }; + sub_sig.sig.output = output; + sub_sig.sig.inputs.insert(1, subscription_sink); quote! { #docs @@ -325,7 +331,7 @@ impl RpcDescription { #tracing::error!(concat!("Error parsing optional \"", stringify!(#name), "\" as \"", stringify!(#ty), "\": {:?}"), e); let _e: #err = e.into(); #pending.reject(_e); - return; + return Ok(()); } }; } @@ -349,7 +355,7 @@ impl RpcDescription { #tracing::error!(concat!("Error parsing \"", stringify!(#name), "\" as \"", stringify!(#ty), "\": {:?}"), e); let _e: #err = e.into(); #pending.reject(_e); - return; + return Ok(()); } }; } @@ -400,7 +406,7 @@ impl RpcDescription { #tracing::error!("Failed to parse JSON-RPC params as object: {}", e); let _e: #err = e.into(); #pending.reject(_e); - return; + return Ok(()); } }; From fb6268d0f60b47ac60efa9bc0ffe08d0faa6e3a8 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 19:37:47 +0300 Subject: [PATCH 07/71] Port `proc_macro` example to new API Signed-off-by: Alexandru Vasile --- examples/examples/proc_macro.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index 9e8c53d21d..6cca7ccdf3 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -28,7 +28,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, client::Subscription, Error}; use jsonrpsee::proc_macros::rpc; -use jsonrpsee::types::ErrorObjectOwned; +use jsonrpsee::types::ResultSubscription; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder, WsServerHandle}; @@ -46,7 +46,7 @@ where /// Subscription that takes a `StorageKey` as input and produces a `Vec`. #[subscription(name = "subscribeStorage" => "override", item = Vec)] - fn subscribe_storage(&self, keys: Option>) -> Result<(), ErrorObjectOwned>; + fn subscribe_storage(&self, keys: Option>); } pub struct RpcServerImpl; @@ -61,7 +61,12 @@ impl RpcServer for RpcServerImpl { Ok(vec![storage_key]) } - fn subscribe_storage(&self, pending: PendingSubscription, _keys: Option>) { + // Note that the server's subscription method must return `ResultSubscription`. + fn subscribe_storage( + &self, + pending: PendingSubscription, + _keys: Option>, + ) -> ResultSubscription { if let Some(mut sink) = pending.accept() { let _ = sink.send(&vec![[0; 32]]); } From 924c0ee37d1d7cbb56c4ab91170c44bd05fdabce Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 19:44:21 +0300 Subject: [PATCH 08/71] Rename `ResultSubscription` to `ReturnTypeSubscription` to avoid confusion Signed-off-by: Alexandru Vasile --- examples/examples/proc_macro.rs | 6 +++--- proc-macros/src/render_server.rs | 4 ++-- types/src/error.rs | 2 +- types/src/lib.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index 6cca7ccdf3..ff1e2e7f7f 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -28,7 +28,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, client::Subscription, Error}; use jsonrpsee::proc_macros::rpc; -use jsonrpsee::types::ResultSubscription; +use jsonrpsee::types::ReturnTypeSubscription; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder, WsServerHandle}; @@ -61,12 +61,12 @@ impl RpcServer for RpcServerImpl { Ok(vec![storage_key]) } - // Note that the server's subscription method must return `ResultSubscription`. + // Note that the server's subscription method must return `ReturnTypeSubscription`. fn subscribe_storage( &self, pending: PendingSubscription, _keys: Option>, - ) -> ResultSubscription { + ) -> ReturnTypeSubscription { if let Some(mut sink) = pending.accept() { let _ = sink.send(&vec![[0; 32]]); } diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 2868a720fd..009d46577e 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -76,8 +76,8 @@ impl RpcDescription { let subscription_sink: syn::FnArg = syn::parse_quote!(subscription_sink: #subscription_sink_ty); let mut sub_sig = sub.signature.clone(); - // For ergonomic reasons, the server's subscription method should return `ResultSubscription`. - let return_ty = self.jrps_server_item(quote! { types::ResultSubscription }); + // For ergonomic reasons, the server's subscription method should return `ReturnTypeSubscription`. + let return_ty = self.jrps_server_item(quote! { types::ReturnTypeSubscription }); let output: ReturnType = parse_quote! { -> #return_ty }; sub_sig.sig.output = output; diff --git a/types/src/error.rs b/types/src/error.rs index 97aab49868..fe8bc6ba1a 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -81,7 +81,7 @@ impl<'a> fmt::Display for ErrorResponse<'a> { } /// The return type of the subscription's method. -pub type ResultSubscription = Result<(), ErrorObjectOwned>; +pub type ReturnTypeSubscription = Result<(), ErrorObjectOwned>; /// Owned variant of [`ErrorObject`]. pub type ErrorObjectOwned = ErrorObject<'static>; diff --git a/types/src/lib.rs b/types/src/lib.rs index ae330c17d3..bb5dab1100 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -43,7 +43,7 @@ pub mod response; /// JSON-RPC response error object related types. pub mod error; -pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ResultSubscription}; +pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ReturnTypeSubscription}; pub use params::{Id, Params, ParamsSequence, ParamsSer, SubscriptionId, TwoPointZero}; pub use request::{InvalidRequest, Notification, NotificationSer, Request, RequestSer}; pub use response::{Response, SubscriptionPayload, SubscriptionResponse}; From 60058eb7e269bb3371935f7831fb8eeb16e38edf Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 20:04:56 +0300 Subject: [PATCH 09/71] Port all tests to new subscription API Signed-off-by: Alexandru Vasile --- proc-macros/tests/ui/correct/basic.rs | 14 +++++++++----- proc-macros/tests/ui/correct/only_server.rs | 6 ++++-- tests/tests/helpers.rs | 10 +++++++++- tests/tests/integration_tests.rs | 7 +++++-- tests/tests/proc_macros.rs | 16 ++++++++++------ tests/tests/resource_limiting.rs | 19 +++++++++++++------ tests/tests/rpc_module.rs | 16 ++++++++++------ ws-server/src/tests.rs | 14 +++++++++----- 8 files changed, 69 insertions(+), 33 deletions(-) diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index db84889c2a..c58413cadc 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -3,6 +3,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, client::ClientT, RpcResult}; +use jsonrpsee::types::ReturnTypeSubscription; use jsonrpsee::proc_macros::rpc; use jsonrpsee::rpc_params; use jsonrpsee::ws_client::*; @@ -63,28 +64,31 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) { + fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); + Ok(()) } - fn sub_with_params(&self, pending: PendingSubscription, val: u32) { + fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let _ = sink.send(&val); let _ = sink.send(&val); + Ok(()) } - fn sub_with_override_notif_method(&self, pending: PendingSubscription) { + fn sub_with_override_notif_method(&self, pending: PendingSubscription) -> ReturnTypeSubscription { if let Some(mut sink) = pending.accept() { let _ = sink.send(&1); } + Ok(()) } } diff --git a/proc-macros/tests/ui/correct/only_server.rs b/proc-macros/tests/ui/correct/only_server.rs index 371d1ca1b6..8bdccdcf82 100644 --- a/proc-macros/tests/ui/correct/only_server.rs +++ b/proc-macros/tests/ui/correct/only_server.rs @@ -1,6 +1,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, RpcResult}; +use jsonrpsee::types::ReturnTypeSubscription; use jsonrpsee::proc_macros::rpc; use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder}; @@ -28,14 +29,15 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) { + fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); + Ok(()) } } diff --git a/tests/tests/helpers.rs b/tests/tests/helpers.rs index 72df50fd9e..bb5b918af7 100644 --- a/tests/tests/helpers.rs +++ b/tests/tests/helpers.rs @@ -52,6 +52,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle } std::thread::sleep(Duration::from_millis(50)); }); + Ok(()) }) .unwrap(); @@ -64,6 +65,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle } std::thread::sleep(Duration::from_millis(100)); }); + Ok(()) }) .unwrap(); @@ -71,7 +73,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle .register_subscription("subscribe_add_one", "subscribe_add_one", "unsubscribe_add_one", |params, pending, _| { let mut count = match params.one::() { Ok(count) => count, - _ => return, + _ => return Ok(()), }; let mut sink = pending.accept().unwrap(); @@ -83,6 +85,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle } std::thread::sleep(Duration::from_millis(100)); }); + Ok(()) }) .unwrap(); @@ -98,6 +101,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle ); sink.close(err); }); + Ok(()) }) .unwrap(); @@ -116,6 +120,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle _ => unreachable!(), } }); + Ok(()) }) .unwrap(); @@ -141,6 +146,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle _ => unreachable!(), } }); + Ok(()) }) .unwrap(); @@ -164,6 +170,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle _ => unreachable!(), } }); + Ok(()) }, ) .unwrap(); @@ -212,6 +219,7 @@ pub async fn websocket_server_with_sleeping_subscription(tx: futures::channel::m let send_back = std::sync::Arc::make_mut(&mut tx); send_back.send(()).await.unwrap(); }); + Ok(()) }) .unwrap(); server.start(module).unwrap(); diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index 6cd0a57116..255c5bd5f7 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -364,6 +364,7 @@ async fn ws_server_should_stop_subscription_after_client_drop() { let send_back = Arc::make_mut(&mut tx); send_back.feed(close_err).await.unwrap(); }); + Ok(()) }) .unwrap(); @@ -506,7 +507,7 @@ async fn ws_server_limit_subs_per_conn_works() { .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; tokio::spawn(async move { @@ -520,6 +521,7 @@ async fn ws_server_limit_subs_per_conn_works() { _ => unreachable!(), }; }); + Ok(()) }) .unwrap(); server.start(module).unwrap(); @@ -563,7 +565,7 @@ async fn ws_server_unsub_methods_should_ignore_sub_limit() { .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; tokio::spawn(async move { @@ -577,6 +579,7 @@ async fn ws_server_unsub_methods_should_ignore_sub_limit() { _ => unreachable!(), }; }); + Ok(()) }) .unwrap(); server.start(module).unwrap(); diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index 5ee1e67407..6c7b469456 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -43,6 +43,7 @@ use serde_json::json; mod rpc_impl { use jsonrpsee::core::{async_trait, RpcResult}; use jsonrpsee::proc_macros::rpc; + use jsonrpsee::types::ReturnTypeSubscription; use jsonrpsee::PendingSubscription; #[rpc(client, server, namespace = "foo")] @@ -166,22 +167,24 @@ mod rpc_impl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) { + fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); + Ok(()) } - fn sub_with_params(&self, pending: PendingSubscription, val: u32) { + fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let _ = sink.send(&val); let _ = sink.send(&val); + Ok(()) } } @@ -194,12 +197,13 @@ mod rpc_impl { #[async_trait] impl OnlyGenericSubscriptionServer for RpcServerImpl { - fn sub(&self, pending: PendingSubscription, _: String) { + fn sub(&self, pending: PendingSubscription, _: String) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let _ = sink.send(&"hello"); + Ok(()) } } } diff --git a/tests/tests/resource_limiting.rs b/tests/tests/resource_limiting.rs index 56c2f97632..5c6ad1e303 100644 --- a/tests/tests/resource_limiting.rs +++ b/tests/tests/resource_limiting.rs @@ -33,6 +33,7 @@ use jsonrpsee::http_client::HttpClientBuilder; use jsonrpsee::http_server::{HttpServerBuilder, HttpServerHandle}; use jsonrpsee::proc_macros::rpc; use jsonrpsee::types::error::CallError; +use jsonrpsee::types::ReturnTypeSubscription; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{WsServerBuilder, WsServerHandle}; use jsonrpsee::{PendingSubscription, RpcModule}; @@ -67,8 +68,9 @@ fn module_manual() -> Result, Error> { .register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, pending, _| { let mut _sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; + Ok(()) })? .resource("SUB", 3)?; @@ -78,7 +80,7 @@ fn module_manual() -> Result, Error> { .register_subscription("subscribe_hello_limit", "s_hello", "unsubscribe_hello_limit", move |_, pending, _| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; tokio::spawn(async move { @@ -87,6 +89,8 @@ fn module_manual() -> Result, Error> { sleep(Duration::from_secs(1)).await; } }); + + Ok(()) })? .resource("SUB", 3)?; @@ -122,17 +126,18 @@ fn module_macro() -> RpcModule<()> { } impl RpcServer for () { - fn sub_hello(&self, pending: PendingSubscription) { + fn sub_hello(&self, pending: PendingSubscription) -> ReturnTypeSubscription { let mut _sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; + Ok(()) } - fn sub_hello_limit(&self, pending: PendingSubscription) { + fn sub_hello_limit(&self, pending: PendingSubscription) -> ReturnTypeSubscription { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; tokio::spawn(async move { @@ -141,6 +146,8 @@ fn module_macro() -> RpcModule<()> { sleep(Duration::from_secs(1)).await; } }); + + Ok(()) } } diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index e96c74254a..66ae6d58f3 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -70,7 +70,7 @@ fn flatten_rpc_modules() { fn rpc_context_modules_can_register_subscriptions() { let cx = (); let mut cxmodule = RpcModule::new(cx); - cxmodule.register_subscription("hi", "hi", "goodbye", |_, _, _| {}).unwrap(); + cxmodule.register_subscription("hi", "hi", "goodbye", |_, _, _| Ok(())).unwrap(); assert!(cxmodule.method("hi").is_some()); assert!(cxmodule.method("goodbye").is_some()); @@ -206,7 +206,7 @@ async fn subscribing_without_server() { .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let mut stream_data = vec!['0', '1', '2']; @@ -219,6 +219,7 @@ async fn subscribing_without_server() { let close = ErrorObject::borrowed(0, &"closed successfully", None); sink.close(close.into_owned()); }); + Ok(()) }) .unwrap(); @@ -244,7 +245,7 @@ async fn close_test_subscribing_without_server() { .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; std::thread::spawn(move || { @@ -259,6 +260,7 @@ async fn close_test_subscribing_without_server() { sink.close(SubscriptionClosed::RemotePeerAborted); } }); + Ok(()) }) .unwrap(); @@ -297,15 +299,16 @@ async fn subscribing_without_server_bad_params() { Err(e) => { let err: Error = e.into(); let _ = pending.reject(err); - return; + return Ok(()); } }; let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; sink.send(&p).unwrap(); + Ok(()) }) .unwrap(); @@ -323,7 +326,7 @@ async fn subscribe_unsubscribe_without_server() { .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let interval = interval(Duration::from_millis(200)); @@ -332,6 +335,7 @@ async fn subscribe_unsubscribe_without_server() { tokio::spawn(async move { sink.pipe_from_stream(stream).await; }); + Ok(()) }) .unwrap(); diff --git a/ws-server/src/tests.rs b/ws-server/src/tests.rs index cef1126586..ebe9849c87 100644 --- a/ws-server/src/tests.rs +++ b/ws-server/src/tests.rs @@ -123,12 +123,13 @@ async fn server_with_handles() -> (SocketAddr, ServerHandle) { .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { let sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; std::thread::spawn(move || loop { let _ = &sink; std::thread::sleep(std::time::Duration::from_secs(30)); }); + Ok(()) }) .unwrap(); @@ -507,10 +508,12 @@ async fn register_methods_works() { assert!(module.register_method("say_hello", |_, _| Ok("lo")).is_ok()); assert!(module.register_method("say_hello", |_, _| Ok("lo")).is_err()); assert!(module - .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, _, _| {}) + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, _, _| { Ok(()) }) .is_ok()); assert!(module - .register_subscription("subscribe_hello_again", "subscribe_hello_again", "unsubscribe_hello", |_, _, _| {}) + .register_subscription("subscribe_hello_again", "subscribe_hello_again", "unsubscribe_hello", |_, _, _| { + Ok(()) + }) .is_err()); assert!( module.register_method("subscribe_hello_again", |_, _| Ok("lo")).is_ok(), @@ -522,7 +525,7 @@ async fn register_methods_works() { async fn register_same_subscribe_unsubscribe_is_err() { let mut module = RpcModule::new(()); assert!(matches!( - module.register_subscription("subscribe_hello", "subscribe_hello", "subscribe_hello", |_, _, _| {}), + module.register_subscription("subscribe_hello", "subscribe_hello", "subscribe_hello", |_, _, _| { Ok(()) }), Err(Error::SubscriptionNameConflict(_)) )); } @@ -681,12 +684,13 @@ async fn custom_subscription_id_works() { .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { let sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; std::thread::spawn(move || loop { let _ = &sink; std::thread::sleep(std::time::Duration::from_secs(30)); }); + Ok(()) }) .unwrap(); server.start(module).unwrap(); From 5ee4ee16604fe3297abc9d5af6d99befbf552945 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 20:22:25 +0300 Subject: [PATCH 10/71] Update documentation Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 4 ++-- proc-macros/src/lib.rs | 22 +++++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 6298b07abb..e1320be755 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -661,14 +661,14 @@ impl RpcModule { /// Err(e) => { /// let err: Error = e.into(); /// pending.reject(err); - /// return; + /// return Ok(()); /// } /// }; /// /// let mut sink = match pending.accept() { /// Some(sink) => sink, /// _ => { - /// return; + /// return Ok(()); /// } /// }; /// diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index fed781d73a..70c5d2fa1c 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -49,13 +49,14 @@ pub(crate) mod visitor; /// type that implements `Client` or `SubscriptionClient` (depending on whether trait has /// subscriptions methods or not), namely `HttpClient` and `WsClient`. /// -/// For servers, it will generate a trait mostly equivalent to the input, with two main -/// differences: +/// For servers, it will generate a trait mostly equivalent to the input, with the following differences: /// /// - The trait will have one additional (already implemented) method, `into_rpc`, which turns any object that /// implements the server trait into an `RpcModule`. -/// - For subscription methods, there will be one additional argument inserted right after `&self`: `subscription_sink: -/// PendingSubscription`. It should be used accept or reject a pending subscription. +/// - For subscription methods: +/// - There will be one additional argument inserted right after `&self`: `subscription_sink: PendingSubscription`. +/// It should be used accept or reject a pending subscription. +/// - The return type of the subscription method is `ReturnTypeSubscription` for improved ergonomics. /// /// Since this macro can generate up to two traits, both server and client traits will have /// a new name. For the `Foo` trait, server trait will be named `FooServer`, and client, @@ -98,8 +99,8 @@ pub(crate) mod visitor; /// async fn async_method(&self, param_a: u8, param_b: String) -> u16; /// fn sync_method(&self) -> String; /// -/// // Note that `subscription_sink` was added automatically. -/// fn sub(&self, subscription_sink: PendingSubscription); +/// // Note that `subscription_sink` and `ReturnTypeSubscription` were added automatically. +/// fn sub(&self, subscription_sink: PendingSubscription) -> ReturnTypeSubscription; /// /// fn into_rpc(self) -> Result { /// // Actual implementation stripped, but inside we will create @@ -215,6 +216,7 @@ pub(crate) mod visitor; /// // RPC is put into a separate module to clearly show names of generated entities. /// mod rpc_impl { /// use jsonrpsee::{proc_macros::rpc, core::async_trait, core::RpcResult, ws_server::PendingSubscription}; +/// use jsonrpsee::types::ReturnTypeSubscription; /// /// // Generate both server and client implementations, prepend all the methods with `foo_` prefix. /// #[rpc(client, server, namespace = "foo")] @@ -286,21 +288,23 @@ pub(crate) mod visitor; /// /// // The stream API can be used to pipe items from the underlying stream /// // as subscription responses. -/// fn sub_override_notif_method(&self, pending: PendingSubscription) { -/// let mut sink = pending.accept().unwrap(); +/// fn sub_override_notif_method(&self, pending: PendingSubscription) -> ReturnTypeSubscription { +/// let mut sink = pending.accept().unwrap(); /// tokio::spawn(async move { /// let stream = futures_util::stream::iter(["one", "two", "three"]); /// sink.pipe_from_stream(stream).await; /// }); +/// Ok(()) /// } /// /// // We could've spawned a `tokio` future that yields values while our program works, /// // but for simplicity of the example we will only send two values and then close /// // the subscription. -/// fn sub(&self, pending: PendingSubscription) { +/// fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { /// let mut sink = pending.accept().unwrap(); /// let _ = sink.send(&"Response_A"); /// let _ = sink.send(&"Response_B"); +/// Ok(()) /// } /// } /// } From 50525f5d3fc5c61d30525fe3f5fc6abd6c304002 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 20:27:42 +0300 Subject: [PATCH 11/71] Port benches Signed-off-by: Alexandru Vasile --- benches/helpers.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/benches/helpers.rs b/benches/helpers.rs index 1cf4321004..b37d285b09 100644 --- a/benches/helpers.rs +++ b/benches/helpers.rs @@ -144,10 +144,11 @@ pub async fn ws_server(handle: tokio::runtime::Handle) -> (String, jsonrpsee::ws .register_subscription(SUB_METHOD_NAME, SUB_METHOD_NAME, UNSUB_METHOD_NAME, |_params, pending, _ctx| { let mut sink = match pending.accept() { Some(sink) => sink, - _ => return, + _ => return Ok(()), }; let x = "Hello"; tokio::spawn(async move { sink.send(&x) }); + Ok(()) }) .unwrap(); From eca7ce65b60619716321075b8a13d9e03de072fb Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 20 Jun 2022 20:34:20 +0300 Subject: [PATCH 12/71] Replace tabs with spaces & add documentation Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 10 +++++----- types/src/error.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index e1320be755..36dec0dd65 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -384,7 +384,7 @@ impl Methods { /// let mut module = RpcModule::new(()); /// module.register_subscription("hi", "hi", "goodbye", |_, pending, _| { /// pending.accept().unwrap().send(&"one answer").unwrap(); - /// Ok(()) + /// Ok(()) /// }).unwrap(); /// let (resp, mut stream) = module.raw_json_request(r#"{"jsonrpc":"2.0","method":"hi","id":0}"#).await.unwrap(); /// let resp = serde_json::from_str::>(&resp).unwrap(); @@ -446,7 +446,7 @@ impl Methods { /// let mut module = RpcModule::new(()); /// module.register_subscription("hi", "hi", "goodbye", |_, pending, _| { /// pending.accept().unwrap().send(&"one answer").unwrap(); - /// Ok(()) + /// Ok(()) /// }).unwrap(); /// /// let mut sub = module.subscribe("hi", EmptyParams::new()).await.unwrap(); @@ -677,7 +677,7 @@ impl RpcModule { /// let _ = sink.send(&sum); /// }); /// - /// Ok(()) + /// Ok(()) /// }); /// ``` pub fn register_subscription( @@ -919,7 +919,7 @@ impl SubscriptionSink { /// } /// } /// }); - /// Ok(()) + /// Ok(()) /// }); /// ``` pub async fn pipe_from_try_stream(&mut self, mut stream: S) -> SubscriptionClosed @@ -992,7 +992,7 @@ impl SubscriptionSink { /// let mut sink = pending.accept().unwrap(); /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); /// tokio::spawn(async move { sink.pipe_from_stream(stream).await; }); - /// Ok(()) + /// Ok(()) /// }); /// ``` pub async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed diff --git a/types/src/error.rs b/types/src/error.rs index fe8bc6ba1a..25ff0badaf 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -80,7 +80,7 @@ impl<'a> fmt::Display for ErrorResponse<'a> { } } -/// The return type of the subscription's method. +/// The return type of the subscription's method for the rpc server implementation. pub type ReturnTypeSubscription = Result<(), ErrorObjectOwned>; /// Owned variant of [`ErrorObject`]. From ab3ab520435b65afd41f3d35899370d4c9675cc6 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 13:48:12 +0300 Subject: [PATCH 13/71] Add dummy error for subscriptions Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 6 +++--- types/src/error.rs | 11 ++++++++++- types/src/lib.rs | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 36dec0dd65..38814ef87f 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -42,8 +42,8 @@ use futures_util::{future::BoxFuture, FutureExt, Stream, StreamExt, TryStream, T use jsonrpsee_types::error::{CallError, ErrorCode, ErrorObject, ErrorObjectOwned, SUBSCRIPTION_CLOSED_WITH_ERROR}; use jsonrpsee_types::response::{SubscriptionError, SubscriptionPayloadError}; use jsonrpsee_types::{ - ErrorResponse, Id, Params, Request, Response, SubscriptionId as RpcSubscriptionId, SubscriptionPayload, - SubscriptionResponse, + ErrorResponse, Id, Params, Request, Response, ReturnTypeSubscription, + SubscriptionId as RpcSubscriptionId, SubscriptionPayload, SubscriptionResponse }; use parking_lot::Mutex; use rustc_hash::FxHashMap; @@ -689,7 +689,7 @@ impl RpcModule { ) -> Result where Context: Send + Sync + 'static, - F: Fn(Params, PendingSubscription, Arc) -> Result<(), ErrorObject> + Send + Sync + 'static, + F: Fn(Params, PendingSubscription, Arc) -> ReturnTypeSubscription + Send + Sync + 'static, { if subscribe_method_name == unsubscribe_method_name { return Err(Error::SubscriptionNameConflict(subscribe_method_name.into())); diff --git a/types/src/error.rs b/types/src/error.rs index 25ff0badaf..85442f6dce 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -81,7 +81,16 @@ impl<'a> fmt::Display for ErrorResponse<'a> { } /// The return type of the subscription's method for the rpc server implementation. -pub type ReturnTypeSubscription = Result<(), ErrorObjectOwned>; +/// +/// **Note**: The error does not contain any data and is discarded on drop. +pub type ReturnTypeSubscription = Result<(), SubscriptionError>; + +/// The error returned by the subscription's method for the rpc server implementation. +/// +/// It contains no data, and neither is the error utilized. It provides an abstraction to make the +/// API more ergonomic while handling errors that may occur during the subscription callback. +#[derive(Debug)] +pub struct SubscriptionError; /// Owned variant of [`ErrorObject`]. pub type ErrorObjectOwned = ErrorObject<'static>; diff --git a/types/src/lib.rs b/types/src/lib.rs index bb5dab1100..91593d8444 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -43,7 +43,7 @@ pub mod response; /// JSON-RPC response error object related types. pub mod error; -pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ReturnTypeSubscription}; +pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ReturnTypeSubscription, SubscriptionError}; pub use params::{Id, Params, ParamsSequence, ParamsSer, SubscriptionId, TwoPointZero}; pub use request::{InvalidRequest, Notification, NotificationSer, Request, RequestSer}; pub use response::{Response, SubscriptionPayload, SubscriptionResponse}; From efe48fece62b38c8095e719c9b4a80f49a42fba6 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 13:55:18 +0300 Subject: [PATCH 14/71] Implement `From` for `SubscriptionError` Signed-off-by: Alexandru Vasile --- types/src/error.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/types/src/error.rs b/types/src/error.rs index 85442f6dce..ba58986d0b 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -92,6 +92,24 @@ pub type ReturnTypeSubscription = Result<(), SubscriptionError>; #[derive(Debug)] pub struct SubscriptionError; +impl From for SubscriptionError { + fn from(_: anyhow::Error) -> Self { + SubscriptionError + } +} + +impl From for SubscriptionError { + fn from(_: CallError) -> Self { + SubscriptionError + } +} + +impl<'a> From> for SubscriptionError { + fn from(_: ErrorObject<'a>) -> Self { + SubscriptionError + } +} + /// Owned variant of [`ErrorObject`]. pub type ErrorObjectOwned = ErrorObject<'static>; From 20891613d4c77a0127de2265bbea464d6f7d4200 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 14:00:32 +0300 Subject: [PATCH 15/71] Return `SubscriptionError` when parsing params Signed-off-by: Alexandru Vasile --- proc-macros/src/render_server.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 009d46577e..e5813e54f2 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -319,6 +319,7 @@ impl RpcDescription { let params_fields = quote! { #(#params_fields_seq),* }; let tracing = self.jrps_server_item(quote! { tracing }); let err = self.jrps_server_item(quote! { core::Error }); + let sub_err = self.jrps_server_item(quote! { types::SubscriptionError }); // Code to decode sequence of parameters from a JSON array. let decode_array = { @@ -331,7 +332,7 @@ impl RpcDescription { #tracing::error!(concat!("Error parsing optional \"", stringify!(#name), "\" as \"", stringify!(#ty), "\": {:?}"), e); let _e: #err = e.into(); #pending.reject(_e); - return Ok(()); + return Err(#sub_err); } }; } @@ -355,7 +356,7 @@ impl RpcDescription { #tracing::error!(concat!("Error parsing \"", stringify!(#name), "\" as \"", stringify!(#ty), "\": {:?}"), e); let _e: #err = e.into(); #pending.reject(_e); - return Ok(()); + return Err(#sub_err); } }; } @@ -406,7 +407,7 @@ impl RpcDescription { #tracing::error!("Failed to parse JSON-RPC params as object: {}", e); let _e: #err = e.into(); #pending.reject(_e); - return Ok(()); + return Err(#sub_err); } }; From 710ee3c39adb1f30494d81866befdafb6131862b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 14:06:50 +0300 Subject: [PATCH 16/71] Rename `SubscriptionError` to `SubscriptionEmptyError` Signed-off-by: Alexandru Vasile --- proc-macros/src/render_server.rs | 2 +- types/src/error.rs | 16 ++++++++-------- types/src/lib.rs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index e5813e54f2..2eb485b174 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -319,7 +319,7 @@ impl RpcDescription { let params_fields = quote! { #(#params_fields_seq),* }; let tracing = self.jrps_server_item(quote! { tracing }); let err = self.jrps_server_item(quote! { core::Error }); - let sub_err = self.jrps_server_item(quote! { types::SubscriptionError }); + let sub_err = self.jrps_server_item(quote! { types::SubscriptionEmptyError }); // Code to decode sequence of parameters from a JSON array. let decode_array = { diff --git a/types/src/error.rs b/types/src/error.rs index ba58986d0b..f020310051 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -83,30 +83,30 @@ impl<'a> fmt::Display for ErrorResponse<'a> { /// The return type of the subscription's method for the rpc server implementation. /// /// **Note**: The error does not contain any data and is discarded on drop. -pub type ReturnTypeSubscription = Result<(), SubscriptionError>; +pub type ReturnTypeSubscription = Result<(), SubscriptionEmptyError>; /// The error returned by the subscription's method for the rpc server implementation. /// /// It contains no data, and neither is the error utilized. It provides an abstraction to make the /// API more ergonomic while handling errors that may occur during the subscription callback. #[derive(Debug)] -pub struct SubscriptionError; +pub struct SubscriptionEmptyError; -impl From for SubscriptionError { +impl From for SubscriptionEmptyError { fn from(_: anyhow::Error) -> Self { - SubscriptionError + SubscriptionEmptyError } } -impl From for SubscriptionError { +impl From for SubscriptionEmptyError { fn from(_: CallError) -> Self { - SubscriptionError + SubscriptionEmptyError } } -impl<'a> From> for SubscriptionError { +impl<'a> From> for SubscriptionEmptyError { fn from(_: ErrorObject<'a>) -> Self { - SubscriptionError + SubscriptionEmptyError } } diff --git a/types/src/lib.rs b/types/src/lib.rs index 91593d8444..191fae7f3c 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -43,7 +43,7 @@ pub mod response; /// JSON-RPC response error object related types. pub mod error; -pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ReturnTypeSubscription, SubscriptionError}; +pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ReturnTypeSubscription, SubscriptionEmptyError}; pub use params::{Id, Params, ParamsSequence, ParamsSer, SubscriptionId, TwoPointZero}; pub use request::{InvalidRequest, Notification, NotificationSer, Request, RequestSer}; pub use response::{Response, SubscriptionPayload, SubscriptionResponse}; From 95245d9bab2226b00a62439362a7e59d0db8b9d7 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 14:30:11 +0300 Subject: [PATCH 17/71] Change `accept` signature Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 38814ef87f..de25c06089 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -42,7 +42,7 @@ use futures_util::{future::BoxFuture, FutureExt, Stream, StreamExt, TryStream, T use jsonrpsee_types::error::{CallError, ErrorCode, ErrorObject, ErrorObjectOwned, SUBSCRIPTION_CLOSED_WITH_ERROR}; use jsonrpsee_types::response::{SubscriptionError, SubscriptionPayloadError}; use jsonrpsee_types::{ - ErrorResponse, Id, Params, Request, Response, ReturnTypeSubscription, + ErrorResponse, Id, Params, Request, Response, ReturnTypeSubscription, SubscriptionEmptyError, SubscriptionId as RpcSubscriptionId, SubscriptionPayload, SubscriptionResponse }; use parking_lot::Mutex; @@ -821,17 +821,17 @@ impl PendingSubscription { /// Attempt to accept the subscription and respond the subscription method call. /// /// Fails if the connection was closed - pub fn accept(mut self) -> Option { - let inner = self.0.take()?; + pub fn accept(mut self) -> Result { + let inner = self.0.take().ok_or(SubscriptionEmptyError)?; let InnerPendingSubscription { sink, close_notify, method, uniq_sub, subscribers, id, claimed } = inner; if sink.send_response(id, &uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); subscribers.lock().insert(uniq_sub.clone(), (sink.clone(), tx)); - Some(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: rx, _claimed: claimed }) + Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: rx, _claimed: claimed }) } else { - None + Err(SubscriptionEmptyError) } } } From 00a91f4b624523bd4e6f2a805f03deb7005f0871 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 14:43:37 +0300 Subject: [PATCH 18/71] Port tests to new `accept` api Signed-off-by: Alexandru Vasile --- benches/helpers.rs | 5 +---- core/src/server/rpc_module.rs | 7 +------ examples/examples/proc_macro.rs | 5 ++--- examples/examples/ws_pubsub_broadcast.rs | 5 +---- examples/examples/ws_pubsub_with_params.rs | 10 ++-------- proc-macros/tests/ui/correct/basic.rs | 12 +++--------- proc-macros/tests/ui/correct/only_server.rs | 5 +---- tests/tests/integration_tests.rs | 10 ++-------- tests/tests/proc_macros.rs | 15 +++------------ tests/tests/resource_limiting.rs | 20 ++++---------------- tests/tests/rpc_module.rs | 20 ++++---------------- ws-server/src/tests.rs | 11 +++-------- 12 files changed, 27 insertions(+), 98 deletions(-) diff --git a/benches/helpers.rs b/benches/helpers.rs index b37d285b09..b6ad82a117 100644 --- a/benches/helpers.rs +++ b/benches/helpers.rs @@ -142,10 +142,7 @@ pub async fn ws_server(handle: tokio::runtime::Handle) -> (String, jsonrpsee::ws module .register_subscription(SUB_METHOD_NAME, SUB_METHOD_NAME, UNSUB_METHOD_NAME, |_params, pending, _ctx| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let x = "Hello"; tokio::spawn(async move { sink.send(&x) }); Ok(()) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index de25c06089..2ee3f61e7e 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -665,12 +665,7 @@ impl RpcModule { /// } /// }; /// - /// let mut sink = match pending.accept() { - /// Some(sink) => sink, - /// _ => { - /// return Ok(()); - /// } - /// }; + /// let mut sink = pending.accept()?; /// /// std::thread::spawn(move || { /// let sum = x + (*ctx); diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index ff1e2e7f7f..90e63e19fd 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -67,9 +67,8 @@ impl RpcServer for RpcServerImpl { pending: PendingSubscription, _keys: Option>, ) -> ReturnTypeSubscription { - if let Some(mut sink) = pending.accept() { - let _ = sink.send(&vec![[0; 32]]); - } + let mut sink = pending.accept()?; + let _ = sink.send(&vec![[0; 32]]); Ok(()) } } diff --git a/examples/examples/ws_pubsub_broadcast.rs b/examples/examples/ws_pubsub_broadcast.rs index f96b925a2b..f397be3921 100644 --- a/examples/examples/ws_pubsub_broadcast.rs +++ b/examples/examples/ws_pubsub_broadcast.rs @@ -73,10 +73,7 @@ async fn run_server() -> anyhow::Result { module.register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, pending, _| { let rx = BroadcastStream::new(tx.clone().subscribe()); - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; tokio::spawn(async move { match sink.pipe_from_try_stream(rx).await { diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index 65dddf4b2c..e20befcbb4 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -68,10 +68,7 @@ async fn run_server() -> anyhow::Result { module .register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, pending, _| { let idx = params.one()?; - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let item = LETTERS.chars().nth(idx); let interval = interval(Duration::from_millis(200)); @@ -95,10 +92,7 @@ async fn run_server() -> anyhow::Result { module .register_subscription("sub_params_two", "params_two", "unsub_params_two", |params, pending, _| { let (one, two) = params.parse::<(usize, usize)>()?; - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let item = &LETTERS[one..two]; diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index c58413cadc..df7dc53feb 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -65,27 +65,21 @@ impl RpcServer for RpcServerImpl { } fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) } fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let _ = sink.send(&val); let _ = sink.send(&val); Ok(()) } fn sub_with_override_notif_method(&self, pending: PendingSubscription) -> ReturnTypeSubscription { - if let Some(mut sink) = pending.accept() { + if let Ok(mut sink) = pending.accept() { let _ = sink.send(&1); } Ok(()) diff --git a/proc-macros/tests/ui/correct/only_server.rs b/proc-macros/tests/ui/correct/only_server.rs index 8bdccdcf82..cbd12350ab 100644 --- a/proc-macros/tests/ui/correct/only_server.rs +++ b/proc-macros/tests/ui/correct/only_server.rs @@ -30,10 +30,7 @@ impl RpcServer for RpcServerImpl { } fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index 255c5bd5f7..4576af46c9 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -505,10 +505,7 @@ async fn ws_server_limit_subs_per_conn_works() { module .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); @@ -563,10 +560,7 @@ async fn ws_server_unsub_methods_should_ignore_sub_limit() { module .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index 6c7b469456..bc90191c7f 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -168,20 +168,14 @@ mod rpc_impl { } fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) } fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let _ = sink.send(&val); let _ = sink.send(&val); Ok(()) @@ -198,10 +192,7 @@ mod rpc_impl { #[async_trait] impl OnlyGenericSubscriptionServer for RpcServerImpl { fn sub(&self, pending: PendingSubscription, _: String) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let _ = sink.send(&"hello"); Ok(()) } diff --git a/tests/tests/resource_limiting.rs b/tests/tests/resource_limiting.rs index 5c6ad1e303..00e707778f 100644 --- a/tests/tests/resource_limiting.rs +++ b/tests/tests/resource_limiting.rs @@ -66,10 +66,7 @@ fn module_manual() -> Result, Error> { // to get dropped. This is the equivalent of not having any resource limits (ie, sink is never used). module .register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, pending, _| { - let mut _sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut _sink = pending.accept()?; Ok(()) })? .resource("SUB", 3)?; @@ -78,10 +75,7 @@ fn module_manual() -> Result, Error> { // and the subscription method gets limited. module .register_subscription("subscribe_hello_limit", "s_hello", "unsubscribe_hello_limit", move |_, pending, _| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; tokio::spawn(async move { for val in 0..10 { @@ -127,18 +121,12 @@ fn module_macro() -> RpcModule<()> { impl RpcServer for () { fn sub_hello(&self, pending: PendingSubscription) -> ReturnTypeSubscription { - let mut _sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut _sink = pending.accept()?; Ok(()) } fn sub_hello_limit(&self, pending: PendingSubscription) -> ReturnTypeSubscription { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; tokio::spawn(async move { for val in 0..10 { diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index 66ae6d58f3..e872b9e32f 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -204,10 +204,7 @@ async fn subscribing_without_server() { let mut module = RpcModule::new(()); module .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let mut stream_data = vec!['0', '1', '2']; std::thread::spawn(move || { @@ -243,10 +240,7 @@ async fn close_test_subscribing_without_server() { let mut module = RpcModule::new(()); module .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; std::thread::spawn(move || { // make sure to only send one item @@ -303,10 +297,7 @@ async fn subscribing_without_server_bad_params() { } }; - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; sink.send(&p).unwrap(); Ok(()) }) @@ -324,10 +315,7 @@ async fn subscribe_unsubscribe_without_server() { let mut module = RpcModule::new(()); module .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { - let mut sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let mut sink = pending.accept()?; let interval = interval(Duration::from_millis(200)); let stream = IntervalStream::new(interval).map(move |_| 1); diff --git a/ws-server/src/tests.rs b/ws-server/src/tests.rs index ebe9849c87..b38874fbf6 100644 --- a/ws-server/src/tests.rs +++ b/ws-server/src/tests.rs @@ -121,10 +121,7 @@ async fn server_with_handles() -> (SocketAddr, ServerHandle) { .unwrap(); module .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { - let sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let sink = pending.accept()?; std::thread::spawn(move || loop { let _ = &sink; std::thread::sleep(std::time::Duration::from_secs(30)); @@ -682,10 +679,8 @@ async fn custom_subscription_id_works() { let mut module = RpcModule::new(()); module .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { - let sink = match pending.accept() { - Some(sink) => sink, - _ => return Ok(()), - }; + let sink = pending.accept()?; + std::thread::spawn(move || loop { let _ = &sink; std::thread::sleep(std::time::Duration::from_secs(30)); From acb65d41ac4349dc1cc4a2868853835e22693e19 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 15:55:47 +0300 Subject: [PATCH 19/71] Implement `pipe_from_try_stream` and `pipe_from_stream` for `PendingSubscription` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 79 +++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 2ee3f61e7e..71c27af519 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -829,6 +829,85 @@ impl PendingSubscription { Err(SubscriptionEmptyError) } } + + /// Accepts the subscription connection and wraps the [`SubscriptionSink::pipe_from_try_stream`] for + /// better ergonomics. + /// + /// Returns `(Ok(sink), SubscriptionClosed)` if the connection was accepted successfully. The returned + /// sink can be used to send error notifications back. + /// + /// Returns `(None, SubscriptionClosed::RemotePeerAborted)` if the connection was not accepted, or the + /// client disconnected while piping the stream. + /// + /// # Examples + /// + /// ```no_run + /// + /// use jsonrpsee_core::server::rpc_module::{RpcModule, SubscriptionResult}; + /// use jsonrpsee_core::error::{Error, SubscriptionClosed}; + /// use jsonrpsee_types::ErrorObjectOwned; + /// use anyhow::anyhow; + /// + /// let mut m = RpcModule::new(()); + /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { + /// // let mut sink = pending.accept().unwrap(); + /// let stream = futures_util::stream::iter(vec![Ok(1_u32), Ok(2), Err("error on the stream")]); + /// // This will return send `[Ok(1_u32), Ok(2_u32), Err(Error::SubscriptionClosed))]` to the subscriber + /// // because after the `Err(_)` the stream is terminated. + /// tokio::spawn(async move { + /// // jsonrpsee doesn't send an error notification unless `close` is explicitly called. + /// // If we pipe messages to the sink, we can inspect why it ended: + /// match pending.pipe_from_try_stream(stream).await { + /// (Some(sink), SubscriptionClosed::Success) => { + /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); + /// sink.close(err_obj); + /// } + /// (Some(sink), SubscriptionClosed::Failed(e)) => { + /// sink.close(e); + /// } + /// // we don't want to send close reason when the client is unsubscribed or disconnected. + /// (_, SubscriptionClosed::RemotePeerAborted) | (None, _) => (), + /// } + /// }); + /// Ok(()) + /// }); + /// ``` + pub async fn pipe_from_try_stream(self, stream: S) -> (Option, SubscriptionClosed) + where + S: TryStream + Unpin, + T: Serialize, + E: std::fmt::Display, + { + if let Ok(mut sink) = self.accept() { + let result = sink.pipe_from_try_stream(stream).await; + (Some(sink), result) + } else { + (None, SubscriptionClosed::RemotePeerAborted) + } + } + + /// Similar to [`PendingSubscription::pipe_from_try_stream`] but it doesn't require the stream return `Result`. + /// + /// # Examples + /// + /// ```no_run + /// + /// use jsonrpsee_core::server::rpc_module::RpcModule; + /// + /// let mut m = RpcModule::new(()); + /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { + /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); + /// tokio::spawn(async move { pending.pipe_from_stream(stream).await; }); + /// Ok(()) + /// }); + /// ``` + pub async fn pipe_from_stream(self, stream: S) -> (Option, SubscriptionClosed) + where + S: Stream + Unpin, + T: Serialize, + { + self.pipe_from_try_stream::<_, _, Error>(stream.map(|item| Ok(item))).await + } } // When dropped it returns an [`InvalidParams`] error to the subscriber From bf167f8db35af442ca258f8af92472b08c4765bc Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 16:03:03 +0300 Subject: [PATCH 20/71] Modify examples to ilustrate new API Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 2 +- examples/examples/ws_pubsub_with_params.rs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 71c27af519..29a82c300a 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -866,7 +866,7 @@ impl PendingSubscription { /// sink.close(e); /// } /// // we don't want to send close reason when the client is unsubscribed or disconnected. - /// (_, SubscriptionClosed::RemotePeerAborted) | (None, _) => (), + /// (Some(_sink), SubscriptionClosed::RemotePeerAborted) | (None, _) => (), /// } /// }); /// Ok(()) diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index e20befcbb4..88a79d5e3d 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -92,7 +92,6 @@ async fn run_server() -> anyhow::Result { module .register_subscription("sub_params_two", "params_two", "unsub_params_two", |params, pending, _| { let (one, two) = params.parse::<(usize, usize)>()?; - let mut sink = pending.accept()?; let item = &LETTERS[one..two]; @@ -100,15 +99,18 @@ async fn run_server() -> anyhow::Result { let stream = IntervalStream::new(interval).map(move |_| item); tokio::spawn(async move { - match sink.pipe_from_stream(stream).await { + match pending.pipe_from_stream(stream).await { // Send close notification when subscription stream failed. - SubscriptionClosed::Failed(err) => { + (Some(sink), SubscriptionClosed::Failed(err)) => { sink.close(err); } // Don't send close notification because the stream should run forever. - SubscriptionClosed::Success => (), - // Don't send close because the client has already disconnected. - SubscriptionClosed::RemotePeerAborted => (), + (Some(_sink), SubscriptionClosed::Success) => (), + // Don't send close because the client has already disconnected after accepting the + // `PendingSubscription` + (Some(_sink), SubscriptionClosed::RemotePeerAborted) => (), + // Client has disconnected before accepting the `PendingSubscription`. + (None, _) => (), }; }); From f8cd8b451ef622d5f6dfec6f8bda4eb417ed9831 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 16:16:53 +0300 Subject: [PATCH 21/71] Fix docs tests Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 29a82c300a..ee64aad717 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -866,7 +866,8 @@ impl PendingSubscription { /// sink.close(e); /// } /// // we don't want to send close reason when the client is unsubscribed or disconnected. - /// (Some(_sink), SubscriptionClosed::RemotePeerAborted) | (None, _) => (), + /// (Some(_sink), SubscriptionClosed::RemotePeerAborted) => (), + /// (None, _) => (), /// } /// }); /// Ok(()) From 70b9d8e8040a55111e7f8835a470205a8ab25bca Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 17:55:17 +0300 Subject: [PATCH 22/71] Rename previously `SubscriptionResult` -> `InnerSubscriptionResult` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index ee64aad717..0506183d56 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -87,7 +87,7 @@ pub struct ConnState<'a> { /// Outcome of a successful terminated subscription. #[derive(Debug)] -pub enum SubscriptionResult { +pub enum InnerSubscriptionResult { /// The subscription stream was executed successfully. Success, /// The subscription was aborted by the remote peer. @@ -843,7 +843,7 @@ impl PendingSubscription { /// /// ```no_run /// - /// use jsonrpsee_core::server::rpc_module::{RpcModule, SubscriptionResult}; + /// use jsonrpsee_core::server::rpc_module::RpcModule; /// use jsonrpsee_core::error::{Error, SubscriptionClosed}; /// use jsonrpsee_types::ErrorObjectOwned; /// use anyhow::anyhow; @@ -968,7 +968,7 @@ impl SubscriptionSink { /// /// ```no_run /// - /// use jsonrpsee_core::server::rpc_module::{RpcModule, SubscriptionResult}; + /// use jsonrpsee_core::server::rpc_module::RpcModule; /// use jsonrpsee_core::error::{Error, SubscriptionClosed}; /// use jsonrpsee_types::ErrorObjectOwned; /// use anyhow::anyhow; From 4efef2d209dd03bb172b2e63eadb79fccb60f6f9 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 17:59:29 +0300 Subject: [PATCH 23/71] Rename `ReturnTypeSubscription` -> `SubscriptionResult` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 4 ++-- examples/examples/proc_macro.rs | 4 ++-- proc-macros/src/lib.rs | 6 +++--- proc-macros/src/render_server.rs | 2 +- proc-macros/tests/ui/correct/basic.rs | 8 ++++---- proc-macros/tests/ui/correct/only_server.rs | 4 ++-- tests/tests/proc_macros.rs | 8 ++++---- tests/tests/resource_limiting.rs | 6 +++--- types/src/error.rs | 2 +- types/src/lib.rs | 2 +- 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 0506183d56..bc92d332f5 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -42,7 +42,7 @@ use futures_util::{future::BoxFuture, FutureExt, Stream, StreamExt, TryStream, T use jsonrpsee_types::error::{CallError, ErrorCode, ErrorObject, ErrorObjectOwned, SUBSCRIPTION_CLOSED_WITH_ERROR}; use jsonrpsee_types::response::{SubscriptionError, SubscriptionPayloadError}; use jsonrpsee_types::{ - ErrorResponse, Id, Params, Request, Response, ReturnTypeSubscription, SubscriptionEmptyError, + ErrorResponse, Id, Params, Request, Response, SubscriptionResult, SubscriptionEmptyError, SubscriptionId as RpcSubscriptionId, SubscriptionPayload, SubscriptionResponse }; use parking_lot::Mutex; @@ -684,7 +684,7 @@ impl RpcModule { ) -> Result where Context: Send + Sync + 'static, - F: Fn(Params, PendingSubscription, Arc) -> ReturnTypeSubscription + Send + Sync + 'static, + F: Fn(Params, PendingSubscription, Arc) -> SubscriptionResult + Send + Sync + 'static, { if subscribe_method_name == unsubscribe_method_name { return Err(Error::SubscriptionNameConflict(subscribe_method_name.into())); diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index 90e63e19fd..f580a74ad8 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -28,7 +28,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, client::Subscription, Error}; use jsonrpsee::proc_macros::rpc; -use jsonrpsee::types::ReturnTypeSubscription; +use jsonrpsee::types::SubscriptionResult; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder, WsServerHandle}; @@ -66,7 +66,7 @@ impl RpcServer for RpcServerImpl { &self, pending: PendingSubscription, _keys: Option>, - ) -> ReturnTypeSubscription { + ) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&vec![[0; 32]]); Ok(()) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index 70c5d2fa1c..33725ff277 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -216,7 +216,7 @@ pub(crate) mod visitor; /// // RPC is put into a separate module to clearly show names of generated entities. /// mod rpc_impl { /// use jsonrpsee::{proc_macros::rpc, core::async_trait, core::RpcResult, ws_server::PendingSubscription}; -/// use jsonrpsee::types::ReturnTypeSubscription; +/// use jsonrpsee::types::SubscriptionResult; /// /// // Generate both server and client implementations, prepend all the methods with `foo_` prefix. /// #[rpc(client, server, namespace = "foo")] @@ -288,7 +288,7 @@ pub(crate) mod visitor; /// /// // The stream API can be used to pipe items from the underlying stream /// // as subscription responses. -/// fn sub_override_notif_method(&self, pending: PendingSubscription) -> ReturnTypeSubscription { +/// fn sub_override_notif_method(&self, pending: PendingSubscription) -> SubscriptionResult { /// let mut sink = pending.accept().unwrap(); /// tokio::spawn(async move { /// let stream = futures_util::stream::iter(["one", "two", "three"]); @@ -300,7 +300,7 @@ pub(crate) mod visitor; /// // We could've spawned a `tokio` future that yields values while our program works, /// // but for simplicity of the example we will only send two values and then close /// // the subscription. -/// fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { +/// fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { /// let mut sink = pending.accept().unwrap(); /// let _ = sink.send(&"Response_A"); /// let _ = sink.send(&"Response_B"); diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 2eb485b174..301b6e963d 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -77,7 +77,7 @@ impl RpcDescription { let mut sub_sig = sub.signature.clone(); // For ergonomic reasons, the server's subscription method should return `ReturnTypeSubscription`. - let return_ty = self.jrps_server_item(quote! { types::ReturnTypeSubscription }); + let return_ty = self.jrps_server_item(quote! { types::SubscriptionResult }); let output: ReturnType = parse_quote! { -> #return_ty }; sub_sig.sig.output = output; diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index df7dc53feb..5b7e692211 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -3,7 +3,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, client::ClientT, RpcResult}; -use jsonrpsee::types::ReturnTypeSubscription; +use jsonrpsee::types::SubscriptionResult; use jsonrpsee::proc_macros::rpc; use jsonrpsee::rpc_params; use jsonrpsee::ws_client::*; @@ -64,21 +64,21 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { + fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) } - fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> ReturnTypeSubscription { + fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&val); let _ = sink.send(&val); Ok(()) } - fn sub_with_override_notif_method(&self, pending: PendingSubscription) -> ReturnTypeSubscription { + fn sub_with_override_notif_method(&self, pending: PendingSubscription) -> SubscriptionResult { if let Ok(mut sink) = pending.accept() { let _ = sink.send(&1); } diff --git a/proc-macros/tests/ui/correct/only_server.rs b/proc-macros/tests/ui/correct/only_server.rs index cbd12350ab..2b3af2fd1e 100644 --- a/proc-macros/tests/ui/correct/only_server.rs +++ b/proc-macros/tests/ui/correct/only_server.rs @@ -1,7 +1,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, RpcResult}; -use jsonrpsee::types::ReturnTypeSubscription; +use jsonrpsee::types::SubscriptionResult; use jsonrpsee::proc_macros::rpc; use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder}; @@ -29,7 +29,7 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { + fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&"Response_A"); diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index bc90191c7f..8b2a5b588b 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -43,7 +43,7 @@ use serde_json::json; mod rpc_impl { use jsonrpsee::core::{async_trait, RpcResult}; use jsonrpsee::proc_macros::rpc; - use jsonrpsee::types::ReturnTypeSubscription; + use jsonrpsee::types::SubscriptionResult; use jsonrpsee::PendingSubscription; #[rpc(client, server, namespace = "foo")] @@ -167,14 +167,14 @@ mod rpc_impl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) -> ReturnTypeSubscription { + fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) } - fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> ReturnTypeSubscription { + fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&val); let _ = sink.send(&val); @@ -191,7 +191,7 @@ mod rpc_impl { #[async_trait] impl OnlyGenericSubscriptionServer for RpcServerImpl { - fn sub(&self, pending: PendingSubscription, _: String) -> ReturnTypeSubscription { + fn sub(&self, pending: PendingSubscription, _: String) -> SubscriptionResult { let mut sink = pending.accept()?; let _ = sink.send(&"hello"); Ok(()) diff --git a/tests/tests/resource_limiting.rs b/tests/tests/resource_limiting.rs index 00e707778f..b68a68db39 100644 --- a/tests/tests/resource_limiting.rs +++ b/tests/tests/resource_limiting.rs @@ -33,7 +33,7 @@ use jsonrpsee::http_client::HttpClientBuilder; use jsonrpsee::http_server::{HttpServerBuilder, HttpServerHandle}; use jsonrpsee::proc_macros::rpc; use jsonrpsee::types::error::CallError; -use jsonrpsee::types::ReturnTypeSubscription; +use jsonrpsee::types::SubscriptionResult; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{WsServerBuilder, WsServerHandle}; use jsonrpsee::{PendingSubscription, RpcModule}; @@ -120,12 +120,12 @@ fn module_macro() -> RpcModule<()> { } impl RpcServer for () { - fn sub_hello(&self, pending: PendingSubscription) -> ReturnTypeSubscription { + fn sub_hello(&self, pending: PendingSubscription) -> SubscriptionResult { let mut _sink = pending.accept()?; Ok(()) } - fn sub_hello_limit(&self, pending: PendingSubscription) -> ReturnTypeSubscription { + fn sub_hello_limit(&self, pending: PendingSubscription) -> SubscriptionResult { let mut sink = pending.accept()?; tokio::spawn(async move { diff --git a/types/src/error.rs b/types/src/error.rs index f020310051..8f29a9d9a0 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -83,7 +83,7 @@ impl<'a> fmt::Display for ErrorResponse<'a> { /// The return type of the subscription's method for the rpc server implementation. /// /// **Note**: The error does not contain any data and is discarded on drop. -pub type ReturnTypeSubscription = Result<(), SubscriptionEmptyError>; +pub type SubscriptionResult = Result<(), SubscriptionEmptyError>; /// The error returned by the subscription's method for the rpc server implementation. /// diff --git a/types/src/lib.rs b/types/src/lib.rs index 191fae7f3c..5c7b4e04bd 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -43,7 +43,7 @@ pub mod response; /// JSON-RPC response error object related types. pub mod error; -pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, ReturnTypeSubscription, SubscriptionEmptyError}; +pub use error::{ErrorObject, ErrorObjectOwned, ErrorResponse, SubscriptionEmptyError, SubscriptionResult}; pub use params::{Id, Params, ParamsSequence, ParamsSer, SubscriptionId, TwoPointZero}; pub use request::{InvalidRequest, Notification, NotificationSer, Request, RequestSer}; pub use response::{Response, SubscriptionPayload, SubscriptionResponse}; From c2c29a280c3a8a1602d676a2bca41fe2c9fe2fca Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 18:00:19 +0300 Subject: [PATCH 24/71] Remove documentation line Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index bc92d332f5..1ba21ce541 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -850,7 +850,6 @@ impl PendingSubscription { /// /// let mut m = RpcModule::new(()); /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { - /// // let mut sink = pending.accept().unwrap(); /// let stream = futures_util::stream::iter(vec![Ok(1_u32), Ok(2), Err("error on the stream")]); /// // This will return send `[Ok(1_u32), Ok(2_u32), Err(Error::SubscriptionClosed))]` to the subscriber /// // because after the `Err(_)` the stream is terminated. From 368d80394fa0ab1eeb72b5a6c365ec37e024a729 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 18:50:39 +0300 Subject: [PATCH 25/71] Implement `PipeFromStreamResult` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 83 +++++++++++++++++----- examples/examples/ws_pubsub_with_params.rs | 16 +---- 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 1ba21ce541..28d00db82c 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -856,23 +856,21 @@ impl PendingSubscription { /// tokio::spawn(async move { /// // jsonrpsee doesn't send an error notification unless `close` is explicitly called. /// // If we pipe messages to the sink, we can inspect why it ended: - /// match pending.pipe_from_try_stream(stream).await { - /// (Some(sink), SubscriptionClosed::Success) => { - /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); - /// sink.close(err_obj); - /// } - /// (Some(sink), SubscriptionClosed::Failed(e)) => { - /// sink.close(e); - /// } - /// // we don't want to send close reason when the client is unsubscribed or disconnected. - /// (Some(_sink), SubscriptionClosed::RemotePeerAborted) => (), - /// (None, _) => (), - /// } + /// pending + /// .pipe_from_stream(stream) + /// .await + /// .on_success(|sink| { + /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); + /// sink.close(err_obj); + /// }) + /// .on_failure(|sink, err| { + /// sink.close(err); + /// }) /// }); /// Ok(()) /// }); /// ``` - pub async fn pipe_from_try_stream(self, stream: S) -> (Option, SubscriptionClosed) + pub async fn pipe_from_try_stream(self, stream: S) -> PipeFromStreamResult where S: TryStream + Unpin, T: Serialize, @@ -880,9 +878,13 @@ impl PendingSubscription { { if let Ok(mut sink) = self.accept() { let result = sink.pipe_from_try_stream(stream).await; - (Some(sink), result) + match result { + SubscriptionClosed::Success => PipeFromStreamResult::Success(sink), + SubscriptionClosed::Failed(error) => PipeFromStreamResult::Failure(sink, error), + SubscriptionClosed::RemotePeerAborted => PipeFromStreamResult::RemotePeerAborted, + } } else { - (None, SubscriptionClosed::RemotePeerAborted) + PipeFromStreamResult::RemotePeerAborted } } @@ -901,7 +903,7 @@ impl PendingSubscription { /// Ok(()) /// }); /// ``` - pub async fn pipe_from_stream(self, stream: S) -> (Option, SubscriptionClosed) + pub async fn pipe_from_stream(self, stream: S) -> PipeFromStreamResult where S: Stream + Unpin, T: Serialize, @@ -920,6 +922,55 @@ impl Drop for PendingSubscription { } } +/// The result obtain from calling [`PendingSubscription::pipe_from_try_stream`] that +/// can be utilized to execute specific operations depending on the result. +#[derive(Debug)] +pub enum PipeFromStreamResult { + /// The connection was accepted and the pipe returned [`SubscriptionClosed::Success`]. + Success(SubscriptionSink), + /// The connection was accepted and the pipe returned [`SubscriptionClosed::Failed`] + /// with the provided error. + Failure(SubscriptionSink, ErrorObjectOwned), + /// The remote peer closed the connection or called the unsubscribe method. + RemotePeerAborted, +} + +impl PipeFromStreamResult { + /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Success`]. + /// After the function runs a new [`PipeFromStreamResult::RemotePeerAborted`] is returned. + /// + /// Otherwise, it leaves the object untouched. + pub fn on_success(self, func: F) -> PipeFromStreamResult + where + F: FnOnce(SubscriptionSink) -> (), + { + match self { + PipeFromStreamResult::Success(sink) => { + func(sink); + PipeFromStreamResult::RemotePeerAborted + } + _ => self + } + } + + /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Failure`]. + /// After the function runs a new [`PipeFromStreamResult::RemotePeerAborted`] is returned. + /// + /// Otherwise, it leaves the object untouched. + pub fn on_failure(self, func: F) -> PipeFromStreamResult + where + F: FnOnce(SubscriptionSink, ErrorObjectOwned) -> (), + { + match self { + PipeFromStreamResult::Failure(sink, error) => { + func(sink, error); + PipeFromStreamResult::RemotePeerAborted + } + _ => self + } + } +} + /// Represents a single subscription. #[derive(Debug)] pub struct SubscriptionSink { diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index 88a79d5e3d..8fc83e4ed4 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -99,19 +99,9 @@ async fn run_server() -> anyhow::Result { let stream = IntervalStream::new(interval).map(move |_| item); tokio::spawn(async move { - match pending.pipe_from_stream(stream).await { - // Send close notification when subscription stream failed. - (Some(sink), SubscriptionClosed::Failed(err)) => { - sink.close(err); - } - // Don't send close notification because the stream should run forever. - (Some(_sink), SubscriptionClosed::Success) => (), - // Don't send close because the client has already disconnected after accepting the - // `PendingSubscription` - (Some(_sink), SubscriptionClosed::RemotePeerAborted) => (), - // Client has disconnected before accepting the `PendingSubscription`. - (None, _) => (), - }; + pending.pipe_from_stream(stream).await.on_failure(|sink, err| { + sink.close(err); + }) }); Ok(()) From 9a21c54484626b58bd73173d4b8ae5248d6344a6 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 21 Jun 2022 18:56:09 +0300 Subject: [PATCH 26/71] Add comment for empty error Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 28d00db82c..675a050475 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -749,6 +749,7 @@ impl RpcModule { claimed, })); + // The callback returns an empty `SubscriptionError` for improved API ergonomics. let _ = callback(params, sink, ctx.clone()); true From 1065fd1d9db110b7e8ce7c0980eca05605c21753 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 22 Jun 2022 12:43:14 +0300 Subject: [PATCH 27/71] Update proc-macros/src/lib.rs Co-authored-by: Niklas Adolfsson --- proc-macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index 33725ff277..7fc0996497 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -56,7 +56,7 @@ pub(crate) mod visitor; /// - For subscription methods: /// - There will be one additional argument inserted right after `&self`: `subscription_sink: PendingSubscription`. /// It should be used accept or reject a pending subscription. -/// - The return type of the subscription method is `ReturnTypeSubscription` for improved ergonomics. +/// - The return type of the subscription method is `SubscriptionResult` for improved ergonomics. /// /// Since this macro can generate up to two traits, both server and client traits will have /// a new name. For the `Foo` trait, server trait will be named `FooServer`, and client, From 2a0a89c9b60ed75725efed3ff4fdcce81e7b1da7 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 22 Jun 2022 12:43:27 +0300 Subject: [PATCH 28/71] Update proc-macros/src/lib.rs Co-authored-by: Niklas Adolfsson --- proc-macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index 7fc0996497..7c4d181365 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -99,7 +99,7 @@ pub(crate) mod visitor; /// async fn async_method(&self, param_a: u8, param_b: String) -> u16; /// fn sync_method(&self) -> String; /// -/// // Note that `subscription_sink` and `ReturnTypeSubscription` were added automatically. +/// // Note that `subscription_sink` and `SubscriptionResult` were added automatically. /// fn sub(&self, subscription_sink: PendingSubscription) -> ReturnTypeSubscription; /// /// fn into_rpc(self) -> Result { From 9cb9e1ab009f7a3eaf1b2946b5d2cc62bed2c793 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 22 Jun 2022 12:43:41 +0300 Subject: [PATCH 29/71] Update proc-macros/src/lib.rs Co-authored-by: Niklas Adolfsson --- proc-macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index 7c4d181365..d44a35c59b 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -100,7 +100,7 @@ pub(crate) mod visitor; /// fn sync_method(&self) -> String; /// /// // Note that `subscription_sink` and `SubscriptionResult` were added automatically. -/// fn sub(&self, subscription_sink: PendingSubscription) -> ReturnTypeSubscription; +/// fn sub(&self, subscription_sink: PendingSubscription) -> SubscriptionResult; /// /// fn into_rpc(self) -> Result { /// // Actual implementation stripped, but inside we will create From 96dda16515a3d200f1be28fe121e045d2aac832c Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 22 Jun 2022 12:44:26 +0300 Subject: [PATCH 30/71] Change `ReturnTypeSubscription` -> `SubscriptionResult` Signed-off-by: Alexandru Vasile --- examples/examples/proc_macro.rs | 2 +- proc-macros/src/render_server.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index f580a74ad8..c313a3ca4b 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -61,7 +61,7 @@ impl RpcServer for RpcServerImpl { Ok(vec![storage_key]) } - // Note that the server's subscription method must return `ReturnTypeSubscription`. + // Note that the server's subscription method must return `SubscriptionResult`. fn subscribe_storage( &self, pending: PendingSubscription, diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 301b6e963d..33a4e2fec5 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -76,7 +76,7 @@ impl RpcDescription { let subscription_sink: syn::FnArg = syn::parse_quote!(subscription_sink: #subscription_sink_ty); let mut sub_sig = sub.signature.clone(); - // For ergonomic reasons, the server's subscription method should return `ReturnTypeSubscription`. + // For ergonomic reasons, the server's subscription method should return `SubscriptionResult`. let return_ty = self.jrps_server_item(quote! { types::SubscriptionResult }); let output: ReturnType = parse_quote! { -> #return_ty }; sub_sig.sig.output = output; From bab6eaf6162cd87c017569078d44a76b8072d394 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 22 Jun 2022 12:48:02 +0300 Subject: [PATCH 31/71] Add `ResultConsumed` for `PipeFromStreamResult` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 675a050475..036028ece0 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -934,6 +934,9 @@ pub enum PipeFromStreamResult { Failure(SubscriptionSink, ErrorObjectOwned), /// The remote peer closed the connection or called the unsubscribe method. RemotePeerAborted, + /// The result was consumed by one of the registered callbacks: + /// [`PipeFromStreamResult::on_success`], or [`PipeFromStreamResult::on_failure`]. + ResultConsumed, } impl PipeFromStreamResult { @@ -948,7 +951,7 @@ impl PipeFromStreamResult { match self { PipeFromStreamResult::Success(sink) => { func(sink); - PipeFromStreamResult::RemotePeerAborted + PipeFromStreamResult::ResultConsumed } _ => self } @@ -965,7 +968,7 @@ impl PipeFromStreamResult { match self { PipeFromStreamResult::Failure(sink, error) => { func(sink, error); - PipeFromStreamResult::RemotePeerAborted + PipeFromStreamResult::ResultConsumed } _ => self } From ae597500d09bdc70f29e3966c6192b4e0c140879 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 22 Jun 2022 13:56:17 +0300 Subject: [PATCH 32/71] Update examples to use `PipeFromStreamResult` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 2 +- examples/examples/ws_pubsub_broadcast.rs | 15 +++++++-------- examples/examples/ws_pubsub_with_params.rs | 14 +++----------- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 036028ece0..a47f72984b 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -858,7 +858,7 @@ impl PendingSubscription { /// // jsonrpsee doesn't send an error notification unless `close` is explicitly called. /// // If we pipe messages to the sink, we can inspect why it ended: /// pending - /// .pipe_from_stream(stream) + /// .pipe_from_try_stream(stream) /// .await /// .on_success(|sink| { /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); diff --git a/examples/examples/ws_pubsub_broadcast.rs b/examples/examples/ws_pubsub_broadcast.rs index f397be3921..edaf71120b 100644 --- a/examples/examples/ws_pubsub_broadcast.rs +++ b/examples/examples/ws_pubsub_broadcast.rs @@ -73,18 +73,17 @@ async fn run_server() -> anyhow::Result { module.register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, pending, _| { let rx = BroadcastStream::new(tx.clone().subscribe()); - let mut sink = pending.accept()?; tokio::spawn(async move { - match sink.pipe_from_try_stream(rx).await { - SubscriptionClosed::Success => { + pending + .pipe_from_try_stream(rx) + .await + .on_success(|sink| { sink.close(SubscriptionClosed::Success); - } - SubscriptionClosed::RemotePeerAborted => (), - SubscriptionClosed::Failed(err) => { + }) + .on_failure(|sink, err| { sink.close(err); - } - }; + }); }); Ok(()) })?; diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index 8fc83e4ed4..1998e975fa 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -29,7 +29,6 @@ use std::time::Duration; use futures::StreamExt; use jsonrpsee::core::client::SubscriptionClientT; -use jsonrpsee::core::error::SubscriptionClosed; use jsonrpsee::rpc_params; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{RpcModule, WsServerBuilder}; @@ -68,23 +67,16 @@ async fn run_server() -> anyhow::Result { module .register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, pending, _| { let idx = params.one()?; - let mut sink = pending.accept()?; let item = LETTERS.chars().nth(idx); let interval = interval(Duration::from_millis(200)); let stream = IntervalStream::new(interval).map(move |_| item); tokio::spawn(async move { - match sink.pipe_from_stream(stream).await { + pending.pipe_from_stream(stream).await.on_failure(|sink, err| { // Send close notification when subscription stream failed. - SubscriptionClosed::Failed(err) => { - sink.close(err); - } - // Don't send close notification because the stream should run forever. - SubscriptionClosed::Success => (), - // Don't send close because the client has already disconnected. - SubscriptionClosed::RemotePeerAborted => (), - }; + sink.close(err); + }); }); Ok(()) }) From 549e6ba4d006d81cd14fa3c6741de37c365a28a1 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 22 Jun 2022 14:12:54 +0300 Subject: [PATCH 33/71] Replace ConsumedResult with Options Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index a47f72984b..93976c38c8 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -880,8 +880,8 @@ impl PendingSubscription { if let Ok(mut sink) = self.accept() { let result = sink.pipe_from_try_stream(stream).await; match result { - SubscriptionClosed::Success => PipeFromStreamResult::Success(sink), - SubscriptionClosed::Failed(error) => PipeFromStreamResult::Failure(sink, error), + SubscriptionClosed::Success => PipeFromStreamResult::Success(Some(sink)), + SubscriptionClosed::Failed(error) => PipeFromStreamResult::Failure(Some((sink, error))), SubscriptionClosed::RemotePeerAborted => PipeFromStreamResult::RemotePeerAborted, } } else { @@ -928,15 +928,12 @@ impl Drop for PendingSubscription { #[derive(Debug)] pub enum PipeFromStreamResult { /// The connection was accepted and the pipe returned [`SubscriptionClosed::Success`]. - Success(SubscriptionSink), + Success(Option), /// The connection was accepted and the pipe returned [`SubscriptionClosed::Failed`] /// with the provided error. - Failure(SubscriptionSink, ErrorObjectOwned), + Failure(Option<(SubscriptionSink, ErrorObjectOwned)>), /// The remote peer closed the connection or called the unsubscribe method. RemotePeerAborted, - /// The result was consumed by one of the registered callbacks: - /// [`PipeFromStreamResult::on_success`], or [`PipeFromStreamResult::on_failure`]. - ResultConsumed, } impl PipeFromStreamResult { @@ -949,9 +946,9 @@ impl PipeFromStreamResult { F: FnOnce(SubscriptionSink) -> (), { match self { - PipeFromStreamResult::Success(sink) => { + PipeFromStreamResult::Success(Some(sink)) => { func(sink); - PipeFromStreamResult::ResultConsumed + PipeFromStreamResult::Success(None) } _ => self } @@ -966,9 +963,9 @@ impl PipeFromStreamResult { F: FnOnce(SubscriptionSink, ErrorObjectOwned) -> (), { match self { - PipeFromStreamResult::Failure(sink, error) => { + PipeFromStreamResult::Failure(Some((sink, error))) => { func(sink, error); - PipeFromStreamResult::ResultConsumed + PipeFromStreamResult::Failure(None) } _ => self } From e6ed17b9ed78be7bc4f4e968ffbd7e5d01effe7b Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 22 Jun 2022 15:24:25 +0300 Subject: [PATCH 34/71] Log warning when subscription callback fails Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 93976c38c8..c89f0df78d 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -750,7 +750,9 @@ impl RpcModule { })); // The callback returns an empty `SubscriptionError` for improved API ergonomics. - let _ = callback(params, sink, ctx.clone()); + if let Err(err) = callback(params, sink, ctx.clone()) { + tracing::warn!("subscribe call `{}` failed with err={:?}", subscribe_method_name, err); + } true })), From f95403eed1a829e04bf71a5880adb769c09ec5e2 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 22 Jun 2022 15:25:09 +0300 Subject: [PATCH 35/71] Change ubuntu test names Signed-off-by: Alexandru Vasile --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c438029a2..b0c24fdb2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,7 +92,7 @@ jobs: run: cargo hack check --workspace --each-feature --all-targets tests_ubuntu: - name: Run nextests on Ubuntu + name: Run tests Ubuntu runs-on: ubuntu-latest steps: - name: Checkout sources From 8d8858c06eca3e311f1d65adc9fdf4d056aaccf0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 24 Jun 2022 16:14:52 +0300 Subject: [PATCH 36/71] server: Make `pipe` methods of `SubscriptionSink` private Signed-off-by: Alexandru Vasile --- 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 c89f0df78d..68fb72f3ef 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1050,7 +1050,7 @@ impl SubscriptionSink { /// Ok(()) /// }); /// ``` - pub async fn pipe_from_try_stream(&mut self, mut stream: S) -> SubscriptionClosed + async fn pipe_from_try_stream(&mut self, mut stream: S) -> SubscriptionClosed where S: TryStream + Unpin, T: Serialize, @@ -1123,7 +1123,7 @@ impl SubscriptionSink { /// Ok(()) /// }); /// ``` - pub async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed + async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed where S: Stream + Unpin, T: Serialize, From d3c3ce9c2b6aaf9970f7228046156f1163162b42 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 24 Jun 2022 16:17:13 +0300 Subject: [PATCH 37/71] server: Remove `pipe_from_stream` method of `SubscriptionSink` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 68fb72f3ef..4444c27dc8 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1103,34 +1103,6 @@ impl SubscriptionSink { } } - /// Similar to [`SubscriptionSink::pipe_from_try_stream`] but it doesn't require the stream return `Result`. - /// - /// Warning: it's possible to pass in a stream that returns `Result` if `Result: Serialize` is satisfied - /// but it won't cancel the stream when an error occurs. If you want the stream to be canceled when an - /// error occurs use [`SubscriptionSink::pipe_from_try_stream`] instead. - /// - /// # Examples - /// - /// ```no_run - /// - /// use jsonrpsee_core::server::rpc_module::RpcModule; - /// - /// let mut m = RpcModule::new(()); - /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { - /// let mut sink = pending.accept().unwrap(); - /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); - /// tokio::spawn(async move { sink.pipe_from_stream(stream).await; }); - /// Ok(()) - /// }); - /// ``` - async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed - where - S: Stream + Unpin, - T: Serialize, - { - self.pipe_from_try_stream::<_, _, Error>(stream.map(|item| Ok(item))).await - } - /// Returns whether the subscription is closed. pub fn is_closed(&self) -> bool { self.inner.is_closed() || self.close_notify.is_none() || !self.is_active_subscription() From 0b1c92728bc2b1523aba45e07cb8b4487033709e Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 24 Jun 2022 16:38:27 +0300 Subject: [PATCH 38/71] server: Update PipeFromStreamResult documentation Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 4444c27dc8..260a3c7570 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -939,8 +939,8 @@ pub enum PipeFromStreamResult { } impl PipeFromStreamResult { - /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Success`]. - /// After the function runs a new [`PipeFromStreamResult::RemotePeerAborted`] is returned. + /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Success(Some(_))`]. + /// After the function runs a new [`PipeFromStreamResult::Success(None)`] is returned. /// /// Otherwise, it leaves the object untouched. pub fn on_success(self, func: F) -> PipeFromStreamResult @@ -956,8 +956,8 @@ impl PipeFromStreamResult { } } - /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Failure`]. - /// After the function runs a new [`PipeFromStreamResult::RemotePeerAborted`] is returned. + /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Failure(Some(_))`]. + /// After the function runs a new [`PipeFromStreamResult::Failure(None)`] is returned. /// /// Otherwise, it leaves the object untouched. pub fn on_failure(self, func: F) -> PipeFromStreamResult @@ -1028,24 +1028,22 @@ impl SubscriptionSink { /// /// let mut m = RpcModule::new(()); /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { - /// let mut sink = pending.accept().unwrap(); /// let stream = futures_util::stream::iter(vec![Ok(1_u32), Ok(2), Err("error on the stream")]); /// // This will return send `[Ok(1_u32), Ok(2_u32), Err(Error::SubscriptionClosed))]` to the subscriber /// // because after the `Err(_)` the stream is terminated. /// tokio::spawn(async move { /// // jsonrpsee doesn't send an error notification unless `close` is explicitly called. /// // If we pipe messages to the sink, we can inspect why it ended: - /// match sink.pipe_from_try_stream(stream).await { - /// SubscriptionClosed::Success => { - /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); - /// sink.close(err_obj); - /// } - /// // we don't want to send close reason when the client is unsubscribed or disconnected. - /// SubscriptionClosed::RemotePeerAborted => (), - /// SubscriptionClosed::Failed(e) => { - /// sink.close(e); - /// } - /// } + /// pending + /// .pipe_from_try_stream(stream) + /// .await + /// .on_success(|sink| { + /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); + /// sink.close(err_obj); + /// }) + /// .on_failure(|sink, err| { + /// sink.close(err); + /// }) /// }); /// Ok(()) /// }); From 8cf401c354f92be4d8b1693f21c1c9703cc9ebd2 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Fri, 24 Jun 2022 17:04:07 +0300 Subject: [PATCH 39/71] Adjust tests to `SubscriptionSink::pipe_from_stream` private interface Signed-off-by: Alexandru Vasile --- proc-macros/src/lib.rs | 3 +- tests/tests/helpers.rs | 50 ++++++++++++++------------------ tests/tests/integration_tests.rs | 22 ++++++-------- tests/tests/rpc_module.rs | 4 +-- 4 files changed, 32 insertions(+), 47 deletions(-) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index d44a35c59b..88a8f5c972 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -289,10 +289,9 @@ pub(crate) mod visitor; /// // The stream API can be used to pipe items from the underlying stream /// // as subscription responses. /// fn sub_override_notif_method(&self, pending: PendingSubscription) -> SubscriptionResult { -/// let mut sink = pending.accept().unwrap(); /// tokio::spawn(async move { /// let stream = futures_util::stream::iter(["one", "two", "three"]); -/// sink.pipe_from_stream(stream).await; +/// pending.pipe_from_stream(stream).await; /// }); /// Ok(()) /// } diff --git a/tests/tests/helpers.rs b/tests/tests/helpers.rs index bb5b918af7..8afb85b4b8 100644 --- a/tests/tests/helpers.rs +++ b/tests/tests/helpers.rs @@ -30,6 +30,7 @@ use std::time::Duration; use futures::{SinkExt, StreamExt}; use jsonrpsee::core::error::SubscriptionClosed; use jsonrpsee::core::server::access_control::{AccessControl, AccessControlBuilder}; +use jsonrpsee::core::server::rpc_module::PipeFromStreamResult; use jsonrpsee::http_server::{HttpServerBuilder, HttpServerHandle}; use jsonrpsee::types::error::{ErrorObject, SUBSCRIPTION_CLOSED_WITH_ERROR}; use jsonrpsee::ws_server::{WsServerBuilder, WsServerHandle}; @@ -107,18 +108,16 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle module .register_subscription("subscribe_5_ints", "n", "unsubscribe_5_ints", |_, pending, _| { - let mut sink = pending.accept().unwrap(); - tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); let stream = IntervalStream::new(interval).zip(futures::stream::iter(1..=5)).map(|(_, c)| c); - match sink.pipe_from_stream(stream).await { - SubscriptionClosed::Success => { - sink.close(SubscriptionClosed::Success); - } + match pending.pipe_from_stream(stream).await.on_success(|sink| { + sink.close(SubscriptionClosed::Success); + }) { + PipeFromStreamResult::Success(None) => (), _ => unreachable!(), - } + }; }); Ok(()) }) @@ -126,23 +125,19 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle module .register_subscription("can_reuse_subscription", "n", "u_can_reuse_subscription", |_, pending, _| { - let mut sink = pending.accept().unwrap(); - tokio::spawn(async move { let stream1 = IntervalStream::new(interval(Duration::from_millis(50))) .zip(futures::stream::iter(1..=5)) .map(|(_, c)| c); - let stream2 = IntervalStream::new(interval(Duration::from_millis(50))) - .zip(futures::stream::iter(6..=10)) - .map(|(_, c)| c); - - let result = sink.pipe_from_stream(stream1).await; - assert!(matches!(result, SubscriptionClosed::Success)); - - match sink.pipe_from_stream(stream2).await { - SubscriptionClosed::Success => { - sink.close(SubscriptionClosed::Success); - } + // TODO(lexnv): Merge streams or `pipe_from_stream` to take `&mut self`. + // let stream2 = IntervalStream::new(interval(Duration::from_millis(50))) + // .zip(futures::stream::iter(6..=10)) + // .map(|(_, c)| c); + + match pending.pipe_from_stream(stream1).await.on_success(|sink| { + sink.close(SubscriptionClosed::Success); + }) { + PipeFromStreamResult::Success(None) => (), _ => unreachable!(), } }); @@ -156,17 +151,15 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle "n", "unsubscribe_with_err_on_stream", move |_, pending, _| { - let mut sink = pending.accept().unwrap(); - let err: &'static str = "error on the stream"; // create stream that produce an error which will cancel the subscription. let stream = futures::stream::iter(vec![Ok(1_u32), Err(err), Ok(2), Ok(3)]); tokio::spawn(async move { - match sink.pipe_from_try_stream(stream).await { - SubscriptionClosed::Failed(e) => { - sink.close(e); - } + match pending.pipe_from_try_stream(stream).await.on_failure(|sink, e| { + sink.close(e); + }) { + PipeFromStreamResult::Failure(None) => (), _ => unreachable!(), } }); @@ -209,13 +202,12 @@ pub async fn websocket_server_with_sleeping_subscription(tx: futures::channel::m module .register_subscription("subscribe_sleep", "n", "unsubscribe_sleep", |_, pending, mut tx| { - let mut sink = pending.accept().unwrap(); - tokio::spawn(async move { let interval = interval(Duration::from_secs(60 * 60)); let stream = IntervalStream::new(interval).zip(futures::stream::iter(1..=5)).map(|(_, c)| c); - sink.pipe_from_stream(stream).await; + pending.pipe_from_stream(stream).await; + let send_back = std::sync::Arc::make_mut(&mut tx); send_back.send(()).await.unwrap(); }); diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index 6b21ea9736..ae8a34cbf8 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -34,6 +34,7 @@ use futures::{channel::mpsc, StreamExt, TryStreamExt}; use helpers::{http_server, http_server_with_access_control, websocket_server, websocket_server_with_subscription}; use jsonrpsee::core::client::{ClientT, IdKind, Subscription, SubscriptionClientT}; use jsonrpsee::core::error::SubscriptionClosed; +use jsonrpsee::core::server::rpc_module::PipeFromStreamResult; use jsonrpsee::core::{Error, JsonValue}; use jsonrpsee::http_client::HttpClientBuilder; use jsonrpsee::http_server::AccessControlBuilder; @@ -546,6 +547,7 @@ async fn ws_server_pipe_from_stream_should_cancel_tasks_immediately() { assert_eq!(rx_len, 10); } +// TODO(lexnv): pipe from stream cannot be reused without having `SubscriptionSink::pipe_from_stream` public. #[tokio::test] async fn ws_server_pipe_from_stream_can_be_reused() { init_logger(); @@ -591,18 +593,16 @@ async fn ws_server_limit_subs_per_conn_works() { module .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { - let mut sink = pending.accept()?; - tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); let stream = IntervalStream::new(interval).map(move |_| 0_usize); - match sink.pipe_from_stream(stream).await { - SubscriptionClosed::Success => { - sink.close(SubscriptionClosed::Success); - } + match pending.pipe_from_stream(stream).await.on_success(|sink| { + sink.close(SubscriptionClosed::Success); + }) { + PipeFromStreamResult::Success(None) => (), _ => unreachable!(), - }; + } }); Ok(()) }) @@ -648,16 +648,12 @@ async fn ws_server_unsub_methods_should_ignore_sub_limit() { module .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { - let mut sink = pending.accept()?; - tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); let stream = IntervalStream::new(interval).map(move |_| 0_usize); - match sink.pipe_from_stream(stream).await { - SubscriptionClosed::RemotePeerAborted => { - sink.close(SubscriptionClosed::RemotePeerAborted); - } + match pending.pipe_from_stream(stream).await { + PipeFromStreamResult::RemotePeerAborted => (), _ => unreachable!(), }; }); diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index e872b9e32f..496ac1f6d2 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -315,13 +315,11 @@ async fn subscribe_unsubscribe_without_server() { let mut module = RpcModule::new(()); module .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { - let mut sink = pending.accept()?; - let interval = interval(Duration::from_millis(200)); let stream = IntervalStream::new(interval).map(move |_| 1); tokio::spawn(async move { - sink.pipe_from_stream(stream).await; + pending.pipe_from_stream(stream).await; }); Ok(()) }) From 348f36688bf8e4ff1c47460cb551eead10b8017c Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 14:21:31 +0300 Subject: [PATCH 40/71] Add `accept-reject` API on `SubscriptionSink` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 260a3c7570..c746984cf2 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -827,7 +827,7 @@ impl PendingSubscription { if sink.send_response(id, &uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); subscribers.lock().insert(uniq_sub.clone(), (sink.clone(), tx)); - Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: rx, _claimed: claimed }) + Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: rx, _claimed: claimed, id: None }) } else { Err(SubscriptionEmptyError) } @@ -983,17 +983,46 @@ pub struct SubscriptionSink { close_notify: Option, /// MethodCallback. method: &'static str, - /// Unique subscription. - uniq_sub: SubscriptionKey, /// Shared Mutex of subscriptions for this method. subscribers: Subscribers, /// Future that returns when the unsubscribe method has been called. unsubscribe: watch::Receiver<()>, + /// Unique subscription. + uniq_sub: SubscriptionKey, + /// Request ID of the subscription. + /// + /// **Note**: This field becomes `None` when the subscription is accepted, or rejected. + id: Option>, /// Claimed resources. _claimed: Option, } impl SubscriptionSink { + /// Reject the subscription call from [`ErrorObject`]. + pub fn reject(&mut self, err: impl Into) -> bool { + if let Some(id) = self.id.take() { + self.inner.send_error(id, err.into()) + } else { + false + } + } + + /// Attempt to accept the subscription and respond the subscription method call. + /// + /// Fails if the connection was closed + pub fn accept(&mut self) -> Result<(), SubscriptionEmptyError> { + let id = self.id.take().ok_or(SubscriptionEmptyError)?; + + if self.inner.send_response(id, &self.uniq_sub.sub_id) { + let (tx, rx) = watch::channel(()); + self.subscribers.lock().insert(self.uniq_sub.clone(), (self.inner.clone(), tx)); + self.unsubscribe = rx; + Ok(()) + } else { + Err(SubscriptionEmptyError) + } + } + /// Send a message back to subscribers. /// /// Returns `Ok(true)` if the message could be send From 15f18c0192786d8b9dc683ac448a339f7cd24ea0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 14:31:34 +0300 Subject: [PATCH 41/71] Make `pipe_from_try_stream` public Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index c746984cf2..0d08ddab45 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1077,7 +1077,7 @@ impl SubscriptionSink { /// Ok(()) /// }); /// ``` - async fn pipe_from_try_stream(&mut self, mut stream: S) -> SubscriptionClosed + pub async fn pipe_from_try_stream(&mut self, mut stream: S) -> SubscriptionClosed where S: TryStream + Unpin, T: Serialize, From 5d214419ebc0bab4824d5c98b384bfc33104d03e Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 14:38:19 +0300 Subject: [PATCH 42/71] Maybe accept the subscription Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 0d08ddab45..cd38e9921e 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1009,7 +1009,7 @@ impl SubscriptionSink { /// Attempt to accept the subscription and respond the subscription method call. /// - /// Fails if the connection was closed + /// Fails if the connection was closed, or if called multiple times. pub fn accept(&mut self) -> Result<(), SubscriptionEmptyError> { let id = self.id.take().ok_or(SubscriptionEmptyError)?; @@ -1023,6 +1023,11 @@ impl SubscriptionSink { } } + /// Accepts the subscription if previously not accepted. + fn maybe_accept(&mut self) { + let _ = self.accept(); + } + /// Send a message back to subscribers. /// /// Returns `Ok(true)` if the message could be send @@ -1030,6 +1035,8 @@ impl SubscriptionSink { /// Return `Err(err)` if the message could not be serialized. /// pub fn send(&mut self, result: &T) -> Result { + self.maybe_accept(); + // only possible to trigger when the connection is dropped. if self.is_closed() { return Ok(false); @@ -1083,6 +1090,8 @@ impl SubscriptionSink { T: Serialize, E: std::fmt::Display, { + self.maybe_accept(); + let conn_closed = match self.close_notify.as_ref().map(|cn| cn.handle()) { Some(cn) => cn, None => { From 277bbd71610de8914b2f1522252afe2bf2a16a47 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 14:41:52 +0300 Subject: [PATCH 43/71] Revert "server: Remove `pipe_from_stream` method of `SubscriptionSink`" This reverts commit d3c3ce9c2b6aaf9970f7228046156f1163162b42. --- core/src/server/rpc_module.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index cd38e9921e..a6e62e70a5 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1139,6 +1139,34 @@ impl SubscriptionSink { } } + /// Similar to [`SubscriptionSink::pipe_from_try_stream`] but it doesn't require the stream return `Result`. + /// + /// Warning: it's possible to pass in a stream that returns `Result` if `Result: Serialize` is satisfied + /// but it won't cancel the stream when an error occurs. If you want the stream to be canceled when an + /// error occurs use [`SubscriptionSink::pipe_from_try_stream`] instead. + /// + /// # Examples + /// + /// ```no_run + /// + /// use jsonrpsee_core::server::rpc_module::RpcModule; + /// + /// let mut m = RpcModule::new(()); + /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { + /// let mut sink = pending.accept().unwrap(); + /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); + /// tokio::spawn(async move { sink.pipe_from_stream(stream).await; }); + /// Ok(()) + /// }); + /// ``` + async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed + where + S: Stream + Unpin, + T: Serialize, + { + self.pipe_from_try_stream::<_, _, Error>(stream.map(|item| Ok(item))).await + } + /// Returns whether the subscription is closed. pub fn is_closed(&self) -> bool { self.inner.is_closed() || self.close_notify.is_none() || !self.is_active_subscription() From 64f45e93c5f007cb9afd8bca7dc4b5a6aff6defd Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 15:05:55 +0300 Subject: [PATCH 44/71] Make `unsubscribe` channel optional on accepting the connection Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index a6e62e70a5..1381fee13e 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -39,7 +39,7 @@ use futures_channel::mpsc; use futures_util::future::Either; use futures_util::pin_mut; use futures_util::{future::BoxFuture, FutureExt, Stream, StreamExt, TryStream, TryStreamExt}; -use jsonrpsee_types::error::{CallError, ErrorCode, ErrorObject, ErrorObjectOwned, SUBSCRIPTION_CLOSED_WITH_ERROR}; +use jsonrpsee_types::error::{CallError, ErrorCode, ErrorObject, ErrorObjectOwned, INTERNAL_ERROR_CODE, SUBSCRIPTION_CLOSED_WITH_ERROR}; use jsonrpsee_types::response::{SubscriptionError, SubscriptionPayloadError}; use jsonrpsee_types::{ ErrorResponse, Id, Params, Request, Response, SubscriptionResult, SubscriptionEmptyError, @@ -827,7 +827,7 @@ impl PendingSubscription { if sink.send_response(id, &uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); subscribers.lock().insert(uniq_sub.clone(), (sink.clone(), tx)); - Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: rx, _claimed: claimed, id: None }) + Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: Some(rx), _claimed: claimed, id: None }) } else { Err(SubscriptionEmptyError) } @@ -986,7 +986,7 @@ pub struct SubscriptionSink { /// Shared Mutex of subscriptions for this method. subscribers: Subscribers, /// Future that returns when the unsubscribe method has been called. - unsubscribe: watch::Receiver<()>, + unsubscribe: Option>, /// Unique subscription. uniq_sub: SubscriptionKey, /// Request ID of the subscription. @@ -1016,7 +1016,7 @@ impl SubscriptionSink { if self.inner.send_response(id, &self.uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); self.subscribers.lock().insert(self.uniq_sub.clone(), (self.inner.clone(), tx)); - self.unsubscribe = rx; + self.unsubscribe = Some(rx); Ok(()) } else { Err(SubscriptionEmptyError) @@ -1099,7 +1099,18 @@ impl SubscriptionSink { } }; - let mut sub_closed = self.unsubscribe.clone(); + let mut sub_closed = match self.unsubscribe.clone() { + Some(sub_closed) => sub_closed, + None => { + let err = ErrorObject::owned( + INTERNAL_ERROR_CODE, + "Unsubscribe watcher not set after accepting the subscription".to_string(), + None::<()> + ); + return SubscriptionClosed::Failed(err); + } + }; + let sub_closed_fut = sub_closed.changed(); let conn_closed_fut = conn_closed.notified(); @@ -1173,7 +1184,7 @@ impl SubscriptionSink { } fn is_active_subscription(&self) -> bool { - !self.unsubscribe.has_changed().is_err() + self.unsubscribe.as_ref().map(|u| !u.has_changed().is_err()).unwrap_or(false) } fn build_message(&self, result: &T) -> Result { From 712112bc8e820de8e18d73090b5c5049bf8d17d3 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 15:24:34 +0300 Subject: [PATCH 45/71] Pass `SubscriptionSink` to subscription callbacks Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 1381fee13e..119efdb9a5 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -684,7 +684,7 @@ impl RpcModule { ) -> Result where Context: Send + Sync + 'static, - F: Fn(Params, PendingSubscription, Arc) -> SubscriptionResult + Send + Sync + 'static, + F: Fn(Params, SubscriptionSink, Arc) -> SubscriptionResult + Send + Sync + 'static, { if subscribe_method_name == unsubscribe_method_name { return Err(Error::SubscriptionNameConflict(subscribe_method_name.into())); @@ -739,15 +739,16 @@ impl RpcModule { MethodCallback::new_subscription(Arc::new(move |id, params, method_sink, conn, claimed| { let sub_id: RpcSubscriptionId = conn.id_provider.next_id(); - let sink = PendingSubscription(Some(InnerPendingSubscription { - sink: method_sink.clone(), + let sink = SubscriptionSink { + inner: method_sink.clone(), close_notify: Some(conn.close_notify), method: notif_method_name, subscribers: subscribers.clone(), + unsubscribe: None, uniq_sub: SubscriptionKey { conn_id: conn.conn_id, sub_id }, - id: id.clone().into_owned(), - claimed, - })); + id: Some(id.clone().into_owned()), + _claimed: claimed, + }; // The callback returns an empty `SubscriptionError` for improved API ergonomics. if let Err(err) = callback(params, sink, ctx.clone()) { @@ -1170,7 +1171,7 @@ impl SubscriptionSink { /// Ok(()) /// }); /// ``` - async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed + pub async fn pipe_from_stream(&mut self, stream: S) -> SubscriptionClosed where S: Stream + Unpin, T: Serialize, From 1f66edf1e7fc89a6f1b116aef9cb8caccf57f109 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 17:45:16 +0300 Subject: [PATCH 46/71] Implement subscription sink state Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 87 +++++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 119efdb9a5..4fa0fa3577 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -744,9 +744,8 @@ impl RpcModule { close_notify: Some(conn.close_notify), method: notif_method_name, subscribers: subscribers.clone(), - unsubscribe: None, uniq_sub: SubscriptionKey { conn_id: conn.conn_id, sub_id }, - id: Some(id.clone().into_owned()), + state: SubscriptionSinkState::new(id.clone().into_owned()), _claimed: claimed, }; @@ -828,7 +827,7 @@ impl PendingSubscription { if sink.send_response(id, &uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); subscribers.lock().insert(uniq_sub.clone(), (sink.clone(), tx)); - Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, unsubscribe: Some(rx), _claimed: claimed, id: None }) + Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, _claimed: claimed, state: SubscriptionSinkState::Rejected }) } else { Err(SubscriptionEmptyError) } @@ -975,6 +974,54 @@ impl PipeFromStreamResult { } } +/// The state of the [`SubscriptionSink`]. +#[derive(Debug)] +enum SubscriptionSinkState { + /// The subscription is pending and needs to be accepted or rejected + /// before utilizing the sink. This is the initial state of the sink. + /// + /// This state contains the request ID of the subscription. + Pending(Option>), + /// The subscription was accepted via the [`SubscriptionSink::accept`] call. + /// This state contains the future that returns when the unsubscribe method has been called. + Accepted(watch::Receiver<()>), + /// The subscription was rejected via the [`SubscriptionSink::reject`] call. + /// In this state, the subscription cannot be utilized. + Rejected, +} + +impl SubscriptionSinkState { + /// Initialize the sink's state from the subscription ID. + fn new(id: Id<'static>) -> Self { + SubscriptionSinkState::Pending(Some(id)) + } + + /// Takes the ID out of the `Pending` state. + fn id(&mut self) -> Result, SubscriptionEmptyError> { + match self { + SubscriptionSinkState::Pending(id) => id.take().ok_or(SubscriptionEmptyError), + _ => Err(SubscriptionEmptyError), + } + } + + /// Advance the state of the sink to the accepted state. + fn accept(&self, rx: watch::Receiver<()>) -> Result { + match self { + // Cannot transition to accepted if the ID was not previously consumed. + SubscriptionSinkState::Pending(None) => Ok(SubscriptionSinkState::Accepted(rx)), + _ => Err(SubscriptionEmptyError), + } + } + + /// Reject the subscription. + fn reject(&self) -> Result { + match self { + SubscriptionSinkState::Pending(None) => Ok(SubscriptionSinkState::Rejected), + _ => Err(SubscriptionEmptyError), + } + } +} + /// Represents a single subscription. #[derive(Debug)] pub struct SubscriptionSink { @@ -986,25 +1033,24 @@ pub struct SubscriptionSink { method: &'static str, /// Shared Mutex of subscriptions for this method. subscribers: Subscribers, - /// Future that returns when the unsubscribe method has been called. - unsubscribe: Option>, /// Unique subscription. uniq_sub: SubscriptionKey, - /// Request ID of the subscription. - /// - /// **Note**: This field becomes `None` when the subscription is accepted, or rejected. - id: Option>, + /// The state of the subscription sink. + state: SubscriptionSinkState, /// Claimed resources. _claimed: Option, } impl SubscriptionSink { /// Reject the subscription call from [`ErrorObject`]. - pub fn reject(&mut self, err: impl Into) -> bool { - if let Some(id) = self.id.take() { - self.inner.send_error(id, err.into()) + pub fn reject(&mut self, err: impl Into) -> Result<(), SubscriptionEmptyError> { + let id = self.state.id()?; + + if self.inner.send_error(id, err.into()) { + self.state = self.state.reject()?; + Ok(()) } else { - false + Err(SubscriptionEmptyError) } } @@ -1012,12 +1058,12 @@ impl SubscriptionSink { /// /// Fails if the connection was closed, or if called multiple times. pub fn accept(&mut self) -> Result<(), SubscriptionEmptyError> { - let id = self.id.take().ok_or(SubscriptionEmptyError)?; + let id = self.state.id()?; if self.inner.send_response(id, &self.uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); self.subscribers.lock().insert(self.uniq_sub.clone(), (self.inner.clone(), tx)); - self.unsubscribe = Some(rx); + self.state = self.state.accept(rx)?; Ok(()) } else { Err(SubscriptionEmptyError) @@ -1100,9 +1146,9 @@ impl SubscriptionSink { } }; - let mut sub_closed = match self.unsubscribe.clone() { - Some(sub_closed) => sub_closed, - None => { + let mut sub_closed = match &self.state { + SubscriptionSinkState::Accepted(rx) => rx.clone(), + _ => { let err = ErrorObject::owned( INTERNAL_ERROR_CODE, "Unsubscribe watcher not set after accepting the subscription".to_string(), @@ -1185,7 +1231,10 @@ impl SubscriptionSink { } fn is_active_subscription(&self) -> bool { - self.unsubscribe.as_ref().map(|u| !u.has_changed().is_err()).unwrap_or(false) + match &self.state { + SubscriptionSinkState::Accepted(unsubscribe) => !unsubscribe.has_changed().is_err(), + _ => false + } } fn build_message(&self, result: &T) -> Result { From 99ffeb7bf721fb9f451ef33aaa32d873781a87da Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 17:48:40 +0300 Subject: [PATCH 47/71] Submit `InvalidParams` if sink was never accepted Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 4fa0fa3577..aafe7ae823 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1288,7 +1288,10 @@ impl SubscriptionSink { impl Drop for SubscriptionSink { fn drop(&mut self) { - if self.is_active_subscription() { + // Subscription was never accepted. + if let Ok(id) = self.state.id() { + self.inner.send_error(id, ErrorCode::InvalidParams.into()); + } else if self.is_active_subscription() { self.subscribers.lock().remove(&self.uniq_sub); } } From 76572020d5548d165d61b0c78d40bf981d0478ef Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 18:08:12 +0300 Subject: [PATCH 48/71] Handle rejected sinks Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index aafe7ae823..e7ac160715 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1071,20 +1071,29 @@ impl SubscriptionSink { } /// Accepts the subscription if previously not accepted. - fn maybe_accept(&mut self) { - let _ = self.accept(); + /// + /// Fails if the accept function fails internally, or if the subscription was rejected. + fn maybe_accept(&mut self) -> Result<(), SubscriptionEmptyError> { + match self.state { + SubscriptionSinkState::Pending(_) => self.accept(), + SubscriptionSinkState::Accepted(_) => Ok(()), + SubscriptionSinkState::Rejected => Err(SubscriptionEmptyError), + } } /// Send a message back to subscribers. /// - /// Returns `Ok(true)` if the message could be send - /// Returns `Ok(false)` if the sink was closed (either because the subscription was closed or the connection was terminated) - /// Return `Err(err)` if the message could not be serialized. - /// + /// Returns + /// - `Ok(true)` if the message could be send. + /// - `Ok(false)` if the sink was closed (either because the subscription was closed or the connection was terminated), + /// or the subscription could not be accepted. + /// - `Err(err)` if the message could not be serialized. pub fn send(&mut self, result: &T) -> Result { - self.maybe_accept(); - - // only possible to trigger when the connection is dropped. + // Cannot accept the subscription. + if self.maybe_accept().is_err() { + return Ok(false); + } + // Only possible to trigger when the connection is dropped. if self.is_closed() { return Ok(false); } @@ -1137,7 +1146,9 @@ impl SubscriptionSink { T: Serialize, E: std::fmt::Display, { - self.maybe_accept(); + if self.maybe_accept().is_err() { + return SubscriptionClosed::RemotePeerAborted; + } let conn_closed = match self.close_notify.as_ref().map(|cn| cn.handle()) { Some(cn) => cn, From 24fcf3ef7da735c06bfe6b3406eef8abd309fad0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 18:09:23 +0300 Subject: [PATCH 49/71] Remove `PendingSubscription` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 148 ---------------------------------- jsonrpsee/src/lib.rs | 2 +- ws-server/src/lib.rs | 2 +- 3 files changed, 2 insertions(+), 150 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index e7ac160715..4e23338497 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -777,154 +777,6 @@ impl RpcModule { } } -/// Represent a pending subscription which waits to be accepted or rejected. -/// -/// Note: you need to call either `PendingSubscription::accept` or `PendingSubscription::reject` otherwise -/// the subscription will be dropped with an `InvalidParams` error. -#[derive(Debug)] -struct InnerPendingSubscription { - /// Sink. - sink: MethodSink, - /// Get notified when subscribers leave so we can exit - close_notify: Option, - /// MethodCallback. - method: &'static str, - /// Unique subscription. - uniq_sub: SubscriptionKey, - /// Shared Mutex of subscriptions - subscribers: Subscribers, - /// Request ID. - id: Id<'static>, - /// Claimed resources. - claimed: Option, -} - -/// Represent a pending subscription which waits until it's either accepted or rejected. -/// -/// This type implements `Drop` for ease of use, e.g. when dropped in error short circuiting via `map_err()?`. -#[derive(Debug)] -pub struct PendingSubscription(Option); - -impl PendingSubscription { - /// Reject the subscription call from [`ErrorObject`]. - pub fn reject(mut self, err: impl Into) -> bool { - if let Some(inner) = self.0.take() { - let InnerPendingSubscription { sink, id, .. } = inner; - sink.send_error(id, err.into()) - } else { - false - } - } - - /// Attempt to accept the subscription and respond the subscription method call. - /// - /// Fails if the connection was closed - pub fn accept(mut self) -> Result { - let inner = self.0.take().ok_or(SubscriptionEmptyError)?; - - let InnerPendingSubscription { sink, close_notify, method, uniq_sub, subscribers, id, claimed } = inner; - - if sink.send_response(id, &uniq_sub.sub_id) { - let (tx, rx) = watch::channel(()); - subscribers.lock().insert(uniq_sub.clone(), (sink.clone(), tx)); - Ok(SubscriptionSink { inner: sink, close_notify, method, uniq_sub, subscribers, _claimed: claimed, state: SubscriptionSinkState::Rejected }) - } else { - Err(SubscriptionEmptyError) - } - } - - /// Accepts the subscription connection and wraps the [`SubscriptionSink::pipe_from_try_stream`] for - /// better ergonomics. - /// - /// Returns `(Ok(sink), SubscriptionClosed)` if the connection was accepted successfully. The returned - /// sink can be used to send error notifications back. - /// - /// Returns `(None, SubscriptionClosed::RemotePeerAborted)` if the connection was not accepted, or the - /// client disconnected while piping the stream. - /// - /// # Examples - /// - /// ```no_run - /// - /// use jsonrpsee_core::server::rpc_module::RpcModule; - /// use jsonrpsee_core::error::{Error, SubscriptionClosed}; - /// use jsonrpsee_types::ErrorObjectOwned; - /// use anyhow::anyhow; - /// - /// let mut m = RpcModule::new(()); - /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { - /// let stream = futures_util::stream::iter(vec![Ok(1_u32), Ok(2), Err("error on the stream")]); - /// // This will return send `[Ok(1_u32), Ok(2_u32), Err(Error::SubscriptionClosed))]` to the subscriber - /// // because after the `Err(_)` the stream is terminated. - /// tokio::spawn(async move { - /// // jsonrpsee doesn't send an error notification unless `close` is explicitly called. - /// // If we pipe messages to the sink, we can inspect why it ended: - /// pending - /// .pipe_from_try_stream(stream) - /// .await - /// .on_success(|sink| { - /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); - /// sink.close(err_obj); - /// }) - /// .on_failure(|sink, err| { - /// sink.close(err); - /// }) - /// }); - /// Ok(()) - /// }); - /// ``` - pub async fn pipe_from_try_stream(self, stream: S) -> PipeFromStreamResult - where - S: TryStream + Unpin, - T: Serialize, - E: std::fmt::Display, - { - if let Ok(mut sink) = self.accept() { - let result = sink.pipe_from_try_stream(stream).await; - match result { - SubscriptionClosed::Success => PipeFromStreamResult::Success(Some(sink)), - SubscriptionClosed::Failed(error) => PipeFromStreamResult::Failure(Some((sink, error))), - SubscriptionClosed::RemotePeerAborted => PipeFromStreamResult::RemotePeerAborted, - } - } else { - PipeFromStreamResult::RemotePeerAborted - } - } - - /// Similar to [`PendingSubscription::pipe_from_try_stream`] but it doesn't require the stream return `Result`. - /// - /// # Examples - /// - /// ```no_run - /// - /// use jsonrpsee_core::server::rpc_module::RpcModule; - /// - /// let mut m = RpcModule::new(()); - /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { - /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); - /// tokio::spawn(async move { pending.pipe_from_stream(stream).await; }); - /// Ok(()) - /// }); - /// ``` - pub async fn pipe_from_stream(self, stream: S) -> PipeFromStreamResult - where - S: Stream + Unpin, - T: Serialize, - { - self.pipe_from_try_stream::<_, _, Error>(stream.map(|item| Ok(item))).await - } -} - -// When dropped it returns an [`InvalidParams`] error to the subscriber -impl Drop for PendingSubscription { - fn drop(&mut self) { - if let Some(inner) = self.0.take() { - let InnerPendingSubscription { sink, id, .. } = inner; - sink.send_error(id, ErrorCode::InvalidParams.into()); - } - } -} - /// The result obtain from calling [`PendingSubscription::pipe_from_try_stream`] that /// can be utilized to execute specific operations depending on the result. #[derive(Debug)] diff --git a/jsonrpsee/src/lib.rs b/jsonrpsee/src/lib.rs index a792f32796..c60b2660da 100644 --- a/jsonrpsee/src/lib.rs +++ b/jsonrpsee/src/lib.rs @@ -92,7 +92,7 @@ cfg_types! { } cfg_server! { - pub use jsonrpsee_core::server::rpc_module::{PendingSubscription, RpcModule, SubscriptionSink}; + pub use jsonrpsee_core::server::rpc_module::{RpcModule, SubscriptionSink}; } cfg_client_or_server! { diff --git a/ws-server/src/lib.rs b/ws-server/src/lib.rs index b90ff5adf2..26d58de6ae 100644 --- a/ws-server/src/lib.rs +++ b/ws-server/src/lib.rs @@ -39,7 +39,7 @@ mod server; mod tests; pub use future::{ServerHandle as WsServerHandle, ShutdownWaiter as WsShutdownWaiter}; -pub use jsonrpsee_core::server::rpc_module::{PendingSubscription, RpcModule, SubscriptionSink}; +pub use jsonrpsee_core::server::rpc_module::{RpcModule, SubscriptionSink}; pub use jsonrpsee_core::{id_providers::*, traits::IdProvider}; pub use jsonrpsee_types as types; pub use server::{Builder as WsServerBuilder, Server as WsServer}; From f30d9d0191ac1634e018ac5097b243cbc0baa504 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 18:30:19 +0300 Subject: [PATCH 50/71] Fix doc tests Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 42 +++++++++++----------- examples/examples/ws_pubsub_with_params.rs | 29 ++++++++++----- proc-macros/src/lib.rs | 9 +++-- proc-macros/src/render_server.rs | 2 +- tests/tests/proc_macros.rs | 6 ++-- tests/tests/resource_limiting.rs | 19 +++++----- 6 files changed, 56 insertions(+), 51 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 4e23338497..1e46928100 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -382,8 +382,8 @@ impl Methods { /// use futures_util::StreamExt; /// /// let mut module = RpcModule::new(()); - /// module.register_subscription("hi", "hi", "goodbye", |_, pending, _| { - /// pending.accept().unwrap().send(&"one answer").unwrap(); + /// module.register_subscription("hi", "hi", "goodbye", |_, mut sink, _| { + /// sink.send(&"one answer").unwrap(); /// Ok(()) /// }).unwrap(); /// let (resp, mut stream) = module.raw_json_request(r#"{"jsonrpc":"2.0","method":"hi","id":0}"#).await.unwrap(); @@ -444,8 +444,8 @@ impl Methods { /// use jsonrpsee::{RpcModule, types::EmptyParams}; /// /// let mut module = RpcModule::new(()); - /// module.register_subscription("hi", "hi", "goodbye", |_, pending, _| { - /// pending.accept().unwrap().send(&"one answer").unwrap(); + /// module.register_subscription("hi", "hi", "goodbye", |_, mut sink, _| { + /// sink.send(&"one answer").unwrap(); /// Ok(()) /// }).unwrap(); /// @@ -655,18 +655,16 @@ impl RpcModule { /// use jsonrpsee_core::Error; /// /// let mut ctx = RpcModule::new(99_usize); - /// ctx.register_subscription("sub", "notif_name", "unsub", |params, pending, ctx| { + /// ctx.register_subscription("sub", "notif_name", "unsub", |params, mut sink, ctx| { /// let x = match params.one::() { /// Ok(x) => x, /// Err(e) => { /// let err: Error = e.into(); - /// pending.reject(err); + /// sink.reject(err); /// return Ok(()); /// } /// }; - /// - /// let mut sink = pending.accept()?; - /// + /// // Sink is accepted on the first `send` call. /// std::thread::spawn(move || { /// let sum = x + (*ctx); /// let _ = sink.send(&sum); @@ -971,23 +969,24 @@ impl SubscriptionSink { /// use anyhow::anyhow; /// /// let mut m = RpcModule::new(()); - /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { + /// m.register_subscription("sub", "_", "unsub", |params, mut sink, _| { /// let stream = futures_util::stream::iter(vec![Ok(1_u32), Ok(2), Err("error on the stream")]); /// // This will return send `[Ok(1_u32), Ok(2_u32), Err(Error::SubscriptionClosed))]` to the subscriber /// // because after the `Err(_)` the stream is terminated. /// tokio::spawn(async move { /// // jsonrpsee doesn't send an error notification unless `close` is explicitly called. /// // If we pipe messages to the sink, we can inspect why it ended: - /// pending - /// .pipe_from_try_stream(stream) - /// .await - /// .on_success(|sink| { - /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); - /// sink.close(err_obj); - /// }) - /// .on_failure(|sink, err| { - /// sink.close(err); - /// }) + /// match sink.pipe_from_try_stream(stream).await { + /// SubscriptionClosed::Success => { + /// let err_obj: ErrorObjectOwned = SubscriptionClosed::Success.into(); + /// sink.close(err_obj); + /// } + /// // we don't want to send close reason when the client is unsubscribed or disconnected. + /// SubscriptionClosed::RemotePeerAborted => (), + /// SubscriptionClosed::Failed(e) => { + /// sink.close(e); + /// } + /// } /// }); /// Ok(()) /// }); @@ -1073,8 +1072,7 @@ impl SubscriptionSink { /// use jsonrpsee_core::server::rpc_module::RpcModule; /// /// let mut m = RpcModule::new(()); - /// m.register_subscription("sub", "_", "unsub", |params, pending, _| { - /// let mut sink = pending.accept().unwrap(); + /// m.register_subscription("sub", "_", "unsub", |params, mut sink, _| { /// let stream = futures_util::stream::iter(vec![1_usize, 2, 3]); /// tokio::spawn(async move { sink.pipe_from_stream(stream).await; }); /// Ok(()) diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index 1998e975fa..abe2003782 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -29,6 +29,7 @@ use std::time::Duration; use futures::StreamExt; use jsonrpsee::core::client::SubscriptionClientT; +use jsonrpsee::core::error::SubscriptionClosed; use jsonrpsee::rpc_params; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{RpcModule, WsServerBuilder}; @@ -65,7 +66,7 @@ async fn run_server() -> anyhow::Result { let server = WsServerBuilder::default().build("127.0.0.1:0").await?; let mut module = RpcModule::new(()); module - .register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, pending, _| { + .register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, mut sink, _| { let idx = params.one()?; let item = LETTERS.chars().nth(idx); @@ -73,16 +74,23 @@ async fn run_server() -> anyhow::Result { let stream = IntervalStream::new(interval).map(move |_| item); tokio::spawn(async move { - pending.pipe_from_stream(stream).await.on_failure(|sink, err| { - // Send close notification when subscription stream failed. - sink.close(err); - }); + match sink.pipe_from_stream(stream).await { + SubscriptionClosed::Failed(err) => { + sink.close(err); + } + _ => (), + }; + // + // sink.pipe_from_stream(stream).await.on_failure(|sink, err| { + // // Send close notification when subscription stream failed. + // sink.close(err); + // }); }); Ok(()) }) .unwrap(); module - .register_subscription("sub_params_two", "params_two", "unsub_params_two", |params, pending, _| { + .register_subscription("sub_params_two", "params_two", "unsub_params_two", |params, mut sink, _| { let (one, two) = params.parse::<(usize, usize)>()?; let item = &LETTERS[one..two]; @@ -91,9 +99,12 @@ async fn run_server() -> anyhow::Result { let stream = IntervalStream::new(interval).map(move |_| item); tokio::spawn(async move { - pending.pipe_from_stream(stream).await.on_failure(|sink, err| { - sink.close(err); - }) + match sink.pipe_from_stream(stream).await { + SubscriptionClosed::Failed(err) => { + sink.close(err); + } + _ => (), + }; }); Ok(()) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index 88a8f5c972..a809af838e 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -215,7 +215,7 @@ pub(crate) mod visitor; /// /// // RPC is put into a separate module to clearly show names of generated entities. /// mod rpc_impl { -/// use jsonrpsee::{proc_macros::rpc, core::async_trait, core::RpcResult, ws_server::PendingSubscription}; +/// use jsonrpsee::{proc_macros::rpc, core::async_trait, core::RpcResult, ws_server::SubscriptionSink}; /// use jsonrpsee::types::SubscriptionResult; /// /// // Generate both server and client implementations, prepend all the methods with `foo_` prefix. @@ -288,10 +288,10 @@ pub(crate) mod visitor; /// /// // The stream API can be used to pipe items from the underlying stream /// // as subscription responses. -/// fn sub_override_notif_method(&self, pending: PendingSubscription) -> SubscriptionResult { +/// fn sub_override_notif_method(&self, mut sink: SubscriptionSink) -> SubscriptionResult { /// tokio::spawn(async move { /// let stream = futures_util::stream::iter(["one", "two", "three"]); -/// pending.pipe_from_stream(stream).await; +/// sink.pipe_from_stream(stream).await; /// }); /// Ok(()) /// } @@ -299,8 +299,7 @@ pub(crate) mod visitor; /// // We could've spawned a `tokio` future that yields values while our program works, /// // but for simplicity of the example we will only send two values and then close /// // the subscription. -/// fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { -/// let mut sink = pending.accept().unwrap(); +/// fn sub(&self, mut sink: SubscriptionSink) -> SubscriptionResult { /// let _ = sink.send(&"Response_A"); /// let _ = sink.send(&"Response_B"); /// Ok(()) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 33a4e2fec5..0b7db21f28 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -71,7 +71,7 @@ impl RpcDescription { let subscriptions = self.subscriptions.iter().map(|sub| { let docs = &sub.docs; - let subscription_sink_ty = self.jrps_server_item(quote! { PendingSubscription }); + let subscription_sink_ty = self.jrps_server_item(quote! { SubscriptionSink }); // Add `SubscriptionSink` as the second input parameter to the signature. let subscription_sink: syn::FnArg = syn::parse_quote!(subscription_sink: #subscription_sink_ty); let mut sub_sig = sub.signature.clone(); diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index 8b2a5b588b..0e0b89ff10 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -44,7 +44,7 @@ mod rpc_impl { use jsonrpsee::core::{async_trait, RpcResult}; use jsonrpsee::proc_macros::rpc; use jsonrpsee::types::SubscriptionResult; - use jsonrpsee::PendingSubscription; + use jsonrpsee::SubscriptionSink; #[rpc(client, server, namespace = "foo")] pub trait Rpc { @@ -167,8 +167,8 @@ mod rpc_impl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { - let mut sink = pending.accept()?; + fn sub(&self, mut sink: SubscriptionSink) -> SubscriptionResult { + let mut sink = sink.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) diff --git a/tests/tests/resource_limiting.rs b/tests/tests/resource_limiting.rs index b68a68db39..4784be06f2 100644 --- a/tests/tests/resource_limiting.rs +++ b/tests/tests/resource_limiting.rs @@ -36,7 +36,7 @@ use jsonrpsee::types::error::CallError; use jsonrpsee::types::SubscriptionResult; use jsonrpsee::ws_client::WsClientBuilder; use jsonrpsee::ws_server::{WsServerBuilder, WsServerHandle}; -use jsonrpsee::{PendingSubscription, RpcModule}; +use jsonrpsee::{RpcModule, SubscriptionSink}; use tokio::time::sleep; fn module_manual() -> Result, Error> { @@ -65,8 +65,8 @@ fn module_manual() -> Result, Error> { // Drop the `SubscriptionSink` to cause the internal `ResourceGuard` allocated per subscription call // to get dropped. This is the equivalent of not having any resource limits (ie, sink is never used). module - .register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, pending, _| { - let mut _sink = pending.accept()?; + .register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, mut sink, _| { + sink.accept()?; Ok(()) })? .resource("SUB", 3)?; @@ -74,11 +74,10 @@ fn module_manual() -> Result, Error> { // Keep the `SubscriptionSink` alive for a bit to validate that `ResourceGuard` is alive // and the subscription method gets limited. module - .register_subscription("subscribe_hello_limit", "s_hello", "unsubscribe_hello_limit", move |_, pending, _| { - let mut sink = pending.accept()?; - + .register_subscription("subscribe_hello_limit", "s_hello", "unsubscribe_hello_limit", move |_, mut sink, _| { tokio::spawn(async move { for val in 0..10 { + // Sink is accepted on the first `send` call. sink.send(&val).unwrap(); sleep(Duration::from_secs(1)).await; } @@ -120,14 +119,12 @@ fn module_macro() -> RpcModule<()> { } impl RpcServer for () { - fn sub_hello(&self, pending: PendingSubscription) -> SubscriptionResult { - let mut _sink = pending.accept()?; + fn sub_hello(&self, mut sink: SubscriptionSink) -> SubscriptionResult { + sink.accept()?; Ok(()) } - fn sub_hello_limit(&self, pending: PendingSubscription) -> SubscriptionResult { - let mut sink = pending.accept()?; - + fn sub_hello_limit(&self, mut sink: SubscriptionSink) -> SubscriptionResult { tokio::spawn(async move { for val in 0..10 { sink.send(&val).unwrap(); From c968cac6c97c291bdf1e5ca6c02abc92c250aeac Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 19:21:10 +0300 Subject: [PATCH 51/71] macro: Make subscription sink mutable Signed-off-by: Alexandru Vasile --- proc-macros/src/render_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 0b7db21f28..d41ae3af9a 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -215,7 +215,7 @@ impl RpcDescription { let resources = handle_resource_limits(&sub.resources); handle_register_result(quote! { - rpc.register_subscription(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, subscription_sink, context| { + rpc.register_subscription(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, mut subscription_sink, context| { #parsing context.as_ref().#rust_method_name(subscription_sink, #params_seq) }) From 38f3a9f79f43eb92ed433d5b5a3e5dc9ca3917a0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Mon, 27 Jun 2022 19:28:33 +0300 Subject: [PATCH 52/71] Fix tests and examples Signed-off-by: Alexandru Vasile --- examples/examples/proc_macro.rs | 5 +- examples/examples/ws_pubsub_broadcast.rs | 16 ++-- examples/examples/ws_pubsub_with_params.rs | 5 - proc-macros/tests/ui/correct/basic.rs | 14 +-- proc-macros/tests/ui/correct/only_server.rs | 6 +- tests/tests/helpers.rs | 100 ++++++++++---------- tests/tests/integration_tests.rs | 26 ++--- tests/tests/proc_macros.rs | 7 +- tests/tests/rpc_module.rs | 18 ++-- ws-server/src/tests.rs | 8 +- 10 files changed, 98 insertions(+), 107 deletions(-) diff --git a/examples/examples/proc_macro.rs b/examples/examples/proc_macro.rs index c313a3ca4b..f980196899 100644 --- a/examples/examples/proc_macro.rs +++ b/examples/examples/proc_macro.rs @@ -30,7 +30,7 @@ use jsonrpsee::core::{async_trait, client::Subscription, Error}; use jsonrpsee::proc_macros::rpc; use jsonrpsee::types::SubscriptionResult; use jsonrpsee::ws_client::WsClientBuilder; -use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder, WsServerHandle}; +use jsonrpsee::ws_server::{SubscriptionSink, WsServerBuilder, WsServerHandle}; type ExampleHash = [u8; 32]; type ExampleStorageKey = Vec; @@ -64,10 +64,9 @@ impl RpcServer for RpcServerImpl { // Note that the server's subscription method must return `SubscriptionResult`. fn subscribe_storage( &self, - pending: PendingSubscription, + mut sink: SubscriptionSink, _keys: Option>, ) -> SubscriptionResult { - let mut sink = pending.accept()?; let _ = sink.send(&vec![[0; 32]]); Ok(()) } diff --git a/examples/examples/ws_pubsub_broadcast.rs b/examples/examples/ws_pubsub_broadcast.rs index edaf71120b..e5de2e78d0 100644 --- a/examples/examples/ws_pubsub_broadcast.rs +++ b/examples/examples/ws_pubsub_broadcast.rs @@ -71,19 +71,19 @@ async fn run_server() -> anyhow::Result { std::thread::spawn(move || produce_items(tx2)); - module.register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, pending, _| { + module.register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", move |_, mut sink, _| { let rx = BroadcastStream::new(tx.clone().subscribe()); tokio::spawn(async move { - pending - .pipe_from_try_stream(rx) - .await - .on_success(|sink| { + match sink.pipe_from_try_stream(rx).await { + SubscriptionClosed::Success => { sink.close(SubscriptionClosed::Success); - }) - .on_failure(|sink, err| { + } + SubscriptionClosed::RemotePeerAborted => (), + SubscriptionClosed::Failed(err) => { sink.close(err); - }); + } + }; }); Ok(()) })?; diff --git a/examples/examples/ws_pubsub_with_params.rs b/examples/examples/ws_pubsub_with_params.rs index abe2003782..dddc09c1ff 100644 --- a/examples/examples/ws_pubsub_with_params.rs +++ b/examples/examples/ws_pubsub_with_params.rs @@ -80,11 +80,6 @@ async fn run_server() -> anyhow::Result { } _ => (), }; - // - // sink.pipe_from_stream(stream).await.on_failure(|sink, err| { - // // Send close notification when subscription stream failed. - // sink.close(err); - // }); }); Ok(()) }) diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index 5b7e692211..7be44ccda7 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -7,7 +7,7 @@ use jsonrpsee::types::SubscriptionResult; use jsonrpsee::proc_macros::rpc; use jsonrpsee::rpc_params; use jsonrpsee::ws_client::*; -use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder}; +use jsonrpsee::ws_server::{SubscriptionSink, WsServerBuilder}; #[rpc(client, server, namespace = "foo")] pub trait Rpc { @@ -64,24 +64,20 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { - let mut sink = pending.accept()?; + fn sub(&self, mut sink: SubscriptionSink) -> SubscriptionResult { let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) } - fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> SubscriptionResult { - let mut sink = pending.accept()?; + fn sub_with_params(&self, mut sink: SubscriptionSink, val: u32) -> SubscriptionResult { let _ = sink.send(&val); let _ = sink.send(&val); Ok(()) } - fn sub_with_override_notif_method(&self, pending: PendingSubscription) -> SubscriptionResult { - if let Ok(mut sink) = pending.accept() { - let _ = sink.send(&1); - } + fn sub_with_override_notif_method(&self, mut sink: SubscriptionSink) -> SubscriptionResult { + let _ = sink.send(&1); Ok(()) } } diff --git a/proc-macros/tests/ui/correct/only_server.rs b/proc-macros/tests/ui/correct/only_server.rs index 2b3af2fd1e..d8556b09f7 100644 --- a/proc-macros/tests/ui/correct/only_server.rs +++ b/proc-macros/tests/ui/correct/only_server.rs @@ -3,7 +3,7 @@ use std::net::SocketAddr; use jsonrpsee::core::{async_trait, RpcResult}; use jsonrpsee::types::SubscriptionResult; use jsonrpsee::proc_macros::rpc; -use jsonrpsee::ws_server::{PendingSubscription, WsServerBuilder}; +use jsonrpsee::ws_server::{SubscriptionSink, WsServerBuilder}; #[rpc(server)] pub trait Rpc { @@ -29,8 +29,8 @@ impl RpcServer for RpcServerImpl { Ok(10u16) } - fn sub(&self, pending: PendingSubscription) -> SubscriptionResult { - let mut sink = pending.accept()?; + fn sub(&self, mut sink: SubscriptionSink) -> SubscriptionResult { + sink.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); diff --git a/tests/tests/helpers.rs b/tests/tests/helpers.rs index 8afb85b4b8..6e283d94bd 100644 --- a/tests/tests/helpers.rs +++ b/tests/tests/helpers.rs @@ -30,7 +30,6 @@ use std::time::Duration; use futures::{SinkExt, StreamExt}; use jsonrpsee::core::error::SubscriptionClosed; use jsonrpsee::core::server::access_control::{AccessControl, AccessControlBuilder}; -use jsonrpsee::core::server::rpc_module::PipeFromStreamResult; use jsonrpsee::http_server::{HttpServerBuilder, HttpServerHandle}; use jsonrpsee::types::error::{ErrorObject, SUBSCRIPTION_CLOSED_WITH_ERROR}; use jsonrpsee::ws_server::{WsServerBuilder, WsServerHandle}; @@ -45,8 +44,9 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle module.register_method("say_hello", |_, _| Ok("hello")).unwrap(); module - .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { - let mut sink = pending.accept().unwrap(); + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, mut sink, _| { + // Explicit call to accept. + sink.accept().unwrap(); std::thread::spawn(move || loop { if let Ok(false) = sink.send(&"hello from subscription") { break; @@ -58,9 +58,9 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle .unwrap(); module - .register_subscription("subscribe_foo", "subscribe_foo", "unsubscribe_foo", |_, pending, _| { - let mut sink = pending.accept().unwrap(); + .register_subscription("subscribe_foo", "subscribe_foo", "unsubscribe_foo", |_, mut sink, _| { std::thread::spawn(move || loop { + // Implicit call to accept for the first send. if let Ok(false) = sink.send(&1337_usize) { break; } @@ -71,28 +71,31 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle .unwrap(); module - .register_subscription("subscribe_add_one", "subscribe_add_one", "unsubscribe_add_one", |params, pending, _| { - let mut count = match params.one::() { - Ok(count) => count, - _ => return Ok(()), - }; - - let mut sink = pending.accept().unwrap(); + .register_subscription( + "subscribe_add_one", + "subscribe_add_one", + "unsubscribe_add_one", + |params, mut sink, _| { + let mut count = match params.one::() { + Ok(count) => count, + _ => return Ok(()), + }; - std::thread::spawn(move || loop { - count = count.wrapping_add(1); - if let Err(_) | Ok(false) = sink.send(&count) { - break; - } - std::thread::sleep(Duration::from_millis(100)); - }); - Ok(()) - }) + std::thread::spawn(move || loop { + count = count.wrapping_add(1); + if let Err(_) | Ok(false) = sink.send(&count) { + break; + } + std::thread::sleep(Duration::from_millis(100)); + }); + Ok(()) + }, + ) .unwrap(); module - .register_subscription("subscribe_noop", "subscribe_noop", "unsubscribe_noop", |_, pending, _| { - let sink = pending.accept().unwrap(); + .register_subscription("subscribe_noop", "subscribe_noop", "unsubscribe_noop", |_, mut sink, _| { + sink.accept().unwrap(); std::thread::spawn(move || { std::thread::sleep(Duration::from_secs(1)); let err = ErrorObject::owned( @@ -107,37 +110,39 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle .unwrap(); module - .register_subscription("subscribe_5_ints", "n", "unsubscribe_5_ints", |_, pending, _| { + .register_subscription("subscribe_5_ints", "n", "unsubscribe_5_ints", |_, mut sink, _| { tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); let stream = IntervalStream::new(interval).zip(futures::stream::iter(1..=5)).map(|(_, c)| c); - match pending.pipe_from_stream(stream).await.on_success(|sink| { - sink.close(SubscriptionClosed::Success); - }) { - PipeFromStreamResult::Success(None) => (), + match sink.pipe_from_stream(stream).await { + SubscriptionClosed::Success => { + sink.close(SubscriptionClosed::Success); + } _ => unreachable!(), - }; + } }); Ok(()) }) .unwrap(); module - .register_subscription("can_reuse_subscription", "n", "u_can_reuse_subscription", |_, pending, _| { + .register_subscription("can_reuse_subscription", "n", "u_can_reuse_subscription", |_, mut sink, _| { tokio::spawn(async move { let stream1 = IntervalStream::new(interval(Duration::from_millis(50))) .zip(futures::stream::iter(1..=5)) .map(|(_, c)| c); - // TODO(lexnv): Merge streams or `pipe_from_stream` to take `&mut self`. - // let stream2 = IntervalStream::new(interval(Duration::from_millis(50))) - // .zip(futures::stream::iter(6..=10)) - // .map(|(_, c)| c); - - match pending.pipe_from_stream(stream1).await.on_success(|sink| { - sink.close(SubscriptionClosed::Success); - }) { - PipeFromStreamResult::Success(None) => (), + let stream2 = IntervalStream::new(interval(Duration::from_millis(50))) + .zip(futures::stream::iter(6..=10)) + .map(|(_, c)| c); + + let result = sink.pipe_from_stream(stream1).await; + assert!(matches!(result, SubscriptionClosed::Success)); + + match sink.pipe_from_stream(stream2).await { + SubscriptionClosed::Success => { + sink.close(SubscriptionClosed::Success); + } _ => unreachable!(), } }); @@ -150,16 +155,16 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle "subscribe_with_err_on_stream", "n", "unsubscribe_with_err_on_stream", - move |_, pending, _| { + move |_, mut sink, _| { let err: &'static str = "error on the stream"; - // create stream that produce an error which will cancel the subscription. + // Create stream that produce an error which will cancel the subscription. let stream = futures::stream::iter(vec![Ok(1_u32), Err(err), Ok(2), Ok(3)]); tokio::spawn(async move { - match pending.pipe_from_try_stream(stream).await.on_failure(|sink, e| { - sink.close(e); - }) { - PipeFromStreamResult::Failure(None) => (), + match sink.pipe_from_try_stream(stream).await { + SubscriptionClosed::Failed(e) => { + sink.close(e); + } _ => unreachable!(), } }); @@ -201,13 +206,12 @@ pub async fn websocket_server_with_sleeping_subscription(tx: futures::channel::m let mut module = RpcModule::new(tx); module - .register_subscription("subscribe_sleep", "n", "unsubscribe_sleep", |_, pending, mut tx| { + .register_subscription("subscribe_sleep", "n", "unsubscribe_sleep", |_, mut sink, mut tx| { tokio::spawn(async move { let interval = interval(Duration::from_secs(60 * 60)); let stream = IntervalStream::new(interval).zip(futures::stream::iter(1..=5)).map(|(_, c)| c); - pending.pipe_from_stream(stream).await; - + sink.pipe_from_stream(stream).await; let send_back = std::sync::Arc::make_mut(&mut tx); send_back.send(()).await.unwrap(); }); diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index ae8a34cbf8..81fe1153b3 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -34,7 +34,6 @@ use futures::{channel::mpsc, StreamExt, TryStreamExt}; use helpers::{http_server, http_server_with_access_control, websocket_server, websocket_server_with_subscription}; use jsonrpsee::core::client::{ClientT, IdKind, Subscription, SubscriptionClientT}; use jsonrpsee::core::error::SubscriptionClosed; -use jsonrpsee::core::server::rpc_module::PipeFromStreamResult; use jsonrpsee::core::{Error, JsonValue}; use jsonrpsee::http_client::HttpClientBuilder; use jsonrpsee::http_server::AccessControlBuilder; @@ -425,8 +424,8 @@ async fn ws_server_should_stop_subscription_after_client_drop() { let mut module = RpcModule::new(tx); module - .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, mut tx| { - let mut sink = pending.accept().unwrap(); + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, mut sink, mut tx| { + sink.accept().unwrap(); tokio::spawn(async move { let close_err = loop { if !sink.send(&1_usize).expect("usize can be serialized; qed") { @@ -547,7 +546,6 @@ async fn ws_server_pipe_from_stream_should_cancel_tasks_immediately() { assert_eq!(rx_len, 10); } -// TODO(lexnv): pipe from stream cannot be reused without having `SubscriptionSink::pipe_from_stream` public. #[tokio::test] async fn ws_server_pipe_from_stream_can_be_reused() { init_logger(); @@ -592,17 +590,17 @@ async fn ws_server_limit_subs_per_conn_works() { let mut module = RpcModule::new(()); module - .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { + .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, mut sink, _| { tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); let stream = IntervalStream::new(interval).map(move |_| 0_usize); - match pending.pipe_from_stream(stream).await.on_success(|sink| { - sink.close(SubscriptionClosed::Success); - }) { - PipeFromStreamResult::Success(None) => (), + match sink.pipe_from_stream(stream).await { + SubscriptionClosed::Success => { + sink.close(SubscriptionClosed::Success); + } _ => unreachable!(), - } + }; }); Ok(()) }) @@ -647,13 +645,15 @@ async fn ws_server_unsub_methods_should_ignore_sub_limit() { let mut module = RpcModule::new(()); module - .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, pending, _| { + .register_subscription("subscribe_forever", "n", "unsubscribe_forever", |_, mut sink, _| { tokio::spawn(async move { let interval = interval(Duration::from_millis(50)); let stream = IntervalStream::new(interval).map(move |_| 0_usize); - match pending.pipe_from_stream(stream).await { - PipeFromStreamResult::RemotePeerAborted => (), + match sink.pipe_from_stream(stream).await { + SubscriptionClosed::RemotePeerAborted => { + sink.close(SubscriptionClosed::RemotePeerAborted); + } _ => unreachable!(), }; }); diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index 0e0b89ff10..8b55ce039a 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -168,14 +168,12 @@ mod rpc_impl { } fn sub(&self, mut sink: SubscriptionSink) -> SubscriptionResult { - let mut sink = sink.accept()?; let _ = sink.send(&"Response_A"); let _ = sink.send(&"Response_B"); Ok(()) } - fn sub_with_params(&self, pending: PendingSubscription, val: u32) -> SubscriptionResult { - let mut sink = pending.accept()?; + fn sub_with_params(&self, mut sink: SubscriptionSink, val: u32) -> SubscriptionResult { let _ = sink.send(&val); let _ = sink.send(&val); Ok(()) @@ -191,8 +189,7 @@ mod rpc_impl { #[async_trait] impl OnlyGenericSubscriptionServer for RpcServerImpl { - fn sub(&self, pending: PendingSubscription, _: String) -> SubscriptionResult { - let mut sink = pending.accept()?; + fn sub(&self, mut sink: SubscriptionSink, _: String) -> SubscriptionResult { let _ = sink.send(&"hello"); Ok(()) } diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index 496ac1f6d2..b3f4a2bcb3 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -203,8 +203,8 @@ async fn calling_method_without_server_using_proc_macro() { async fn subscribing_without_server() { let mut module = RpcModule::new(()); module - .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { - let mut sink = pending.accept()?; + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { + sink.accept()?; let mut stream_data = vec!['0', '1', '2']; std::thread::spawn(move || { @@ -239,8 +239,8 @@ async fn close_test_subscribing_without_server() { let mut module = RpcModule::new(()); module - .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { - let mut sink = pending.accept()?; + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { + sink.accept()?; std::thread::spawn(move || { // make sure to only send one item @@ -287,17 +287,17 @@ async fn close_test_subscribing_without_server() { async fn subscribing_without_server_bad_params() { let mut module = RpcModule::new(()); module - .register_subscription("my_sub", "my_sub", "my_unsub", |params, pending, _| { + .register_subscription("my_sub", "my_sub", "my_unsub", |params, mut sink, _| { let p = match params.one::() { Ok(p) => p, Err(e) => { let err: Error = e.into(); - let _ = pending.reject(err); + let _ = sink.reject(err); return Ok(()); } }; - let mut sink = pending.accept()?; + sink.accept()?; sink.send(&p).unwrap(); Ok(()) }) @@ -314,12 +314,12 @@ async fn subscribing_without_server_bad_params() { async fn subscribe_unsubscribe_without_server() { let mut module = RpcModule::new(()); module - .register_subscription("my_sub", "my_sub", "my_unsub", |_, pending, _| { + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { let interval = interval(Duration::from_millis(200)); let stream = IntervalStream::new(interval).map(move |_| 1); tokio::spawn(async move { - pending.pipe_from_stream(stream).await; + sink.pipe_from_stream(stream).await; }); Ok(()) }) diff --git a/ws-server/src/tests.rs b/ws-server/src/tests.rs index b38874fbf6..e1ff992d92 100644 --- a/ws-server/src/tests.rs +++ b/ws-server/src/tests.rs @@ -120,8 +120,8 @@ async fn server_with_handles() -> (SocketAddr, ServerHandle) { }) .unwrap(); module - .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { - let sink = pending.accept()?; + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, mut sink, _| { + sink.accept()?; std::thread::spawn(move || loop { let _ = &sink; std::thread::sleep(std::time::Duration::from_secs(30)); @@ -678,8 +678,8 @@ async fn custom_subscription_id_works() { let addr = server.local_addr().unwrap(); let mut module = RpcModule::new(()); module - .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, pending, _| { - let sink = pending.accept()?; + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, mut sink, _| { + sink.accept()?; std::thread::spawn(move || loop { let _ = &sink; From 53eeff987b65b11457b6629a5282e0662cc880a9 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 13:08:48 +0300 Subject: [PATCH 53/71] macro: Return `sink.reject()` result Signed-off-by: Alexandru Vasile --- proc-macros/src/render_server.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index d41ae3af9a..e59b5e2d56 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -331,7 +331,7 @@ impl RpcDescription { Err(e) => { #tracing::error!(concat!("Error parsing optional \"", stringify!(#name), "\" as \"", stringify!(#ty), "\": {:?}"), e); let _e: #err = e.into(); - #pending.reject(_e); + #pending.reject(_e)?; return Err(#sub_err); } }; @@ -355,7 +355,7 @@ impl RpcDescription { Err(e) => { #tracing::error!(concat!("Error parsing \"", stringify!(#name), "\" as \"", stringify!(#ty), "\": {:?}"), e); let _e: #err = e.into(); - #pending.reject(_e); + #pending.reject(_e)?; return Err(#sub_err); } }; @@ -406,7 +406,7 @@ impl RpcDescription { Err(e) => { #tracing::error!("Failed to parse JSON-RPC params as object: {}", e); let _e: #err = e.into(); - #pending.reject(_e); + #pending.reject(_e)?; return Err(#sub_err); } }; From c7b28676a7a8bb7315a2684599f4dc8a9920e576 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 13:46:11 +0300 Subject: [PATCH 54/71] tests: Add test for `SubscriptionSinkState` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 62 +++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 1e46928100..21436e1dc4 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1215,3 +1215,65 @@ impl Drop for Subscription { self.close(); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sink_state_pending() { + let mut state = SubscriptionSinkState::new(Id::Number(0)); + assert!(matches!(state, SubscriptionSinkState::Pending(Some(Id::Number(0))))); + + assert!(matches!(state.id(), Ok(Id::Number(0)))); + + assert!(matches!(state, SubscriptionSinkState::Pending(None))); + assert!(matches!(state.id(), Err(SubscriptionEmptyError))); + } + + #[test] + fn sink_state_accepted() { + let mut state = SubscriptionSinkState::new(Id::Number(0)); + let (_, rx) = watch::channel(()); + + // Invalid transition without consuming the ID. + assert!(matches!(state.accept(rx.clone()), Err(SubscriptionEmptyError))); + assert!(matches!(state, SubscriptionSinkState::Pending(Some(Id::Number(0))))); + + // Transition to accepted. + assert!(matches!(state.id(), Ok(Id::Number(0)))); + let state = state.accept(rx.clone()); + assert!(matches!(state, Ok(SubscriptionSinkState::Accepted(_)))); + let mut state = state.unwrap(); + + // Double transition. + assert!(matches!(state.accept(rx.clone()), Err(SubscriptionEmptyError))); + // Invalid method call in this state. + assert!(matches!(state.id(), Err(SubscriptionEmptyError))); + // Invalid transition. + assert!(matches!(state.reject(), Err(SubscriptionEmptyError))); + } + + #[test] + fn sink_state_rejected() { + let mut state = SubscriptionSinkState::new(Id::Number(0)); + + // Invalid transition without consuming the ID. + assert!(matches!(state.reject(), Err(SubscriptionEmptyError))); + assert!(matches!(state, SubscriptionSinkState::Pending(Some(Id::Number(0))))); + + // Transition to rejected. + assert!(matches!(state.id(), Ok(Id::Number(0)))); + let state = state.reject(); + assert!(matches!(state, Ok(SubscriptionSinkState::Rejected))); + let mut state = state.unwrap(); + + // Double transition. + assert!(matches!(state.reject(), Err(SubscriptionEmptyError))); + // Invalid method call in this state. + assert!(matches!(state.id(), Err(SubscriptionEmptyError))); + // Invalid transition. + let (_, rx) = watch::channel(()); + assert!(matches!(state.accept(rx), Err(SubscriptionEmptyError))); + } +} From 9e8f649f58288d9184b7db99a89abfd7316c83e3 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 14:51:04 +0300 Subject: [PATCH 55/71] Test internal subscription sink state Signed-off-by: Alexandru Vasile --- tests/tests/rpc_module.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index b3f4a2bcb3..309991243f 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -348,3 +348,36 @@ async fn subscribe_unsubscribe_without_server() { futures::future::join(sub1, sub2).await; } + +#[tokio::test] +async fn empty_subscription_without_server() { + let mut module = RpcModule::new(()); + module + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut _sink, _| { + // Sink was never accepted or rejected. Expected to return `InvalidParams`. + Ok(()) + }) + .unwrap(); + + let sub_err = module.subscribe("my_sub", EmptyParams::new()).await.unwrap_err(); + + assert!( + matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("Invalid params") && e.code() == ErrorCode::InvalidParams.code()) + ); +} + +#[tokio::test] +async fn rejected_subscription_without_server() { + let mut module = RpcModule::new(()); + module + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { + let err = ErrorObject::borrowed(0, &"rejected", None); + sink.reject(err.into_owned())?; + Ok(()) + }) + .unwrap(); + + let sub_err = module.subscribe("my_sub", EmptyParams::new()).await.unwrap_err(); + + assert!(matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("rejected") && e.code() == 0)); +} From 18d81699c033d7c7ee0a187a048bac1899b8af51 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 14:58:34 +0300 Subject: [PATCH 56/71] Fix `send_error` to not always return `false` Signed-off-by: Alexandru Vasile --- core/src/server/helpers.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/server/helpers.rs b/core/src/server/helpers.rs index 09ded6799f..bf1dcf5fcb 100644 --- a/core/src/server/helpers.rs +++ b/core/src/server/helpers.rs @@ -156,9 +156,10 @@ impl MethodSink { if let Err(err) = self.send_raw(json) { tracing::warn!("Error sending response {:?}", err); + false + } else { + true } - - false } /// Helper for sending the general purpose `Error` as a JSON-RPC errors to the client From 1d983c7716017ad4393a4a93ca5bb3e19273a0ba Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 15:27:36 +0300 Subject: [PATCH 57/71] Fix benches Signed-off-by: Alexandru Vasile --- benches/helpers.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/benches/helpers.rs b/benches/helpers.rs index b6ad82a117..af5d39e65d 100644 --- a/benches/helpers.rs +++ b/benches/helpers.rs @@ -141,8 +141,7 @@ pub async fn ws_server(handle: tokio::runtime::Handle) -> (String, jsonrpsee::ws let mut module = gen_rpc_module(); module - .register_subscription(SUB_METHOD_NAME, SUB_METHOD_NAME, UNSUB_METHOD_NAME, |_params, pending, _ctx| { - let mut sink = pending.accept()?; + .register_subscription(SUB_METHOD_NAME, SUB_METHOD_NAME, UNSUB_METHOD_NAME, |_params, mut sink, _ctx| { let x = "Hello"; tokio::spawn(async move { sink.send(&x) }); Ok(()) From 639e5248bb650f2328016f092b284707d5a1a8f8 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 15:30:33 +0300 Subject: [PATCH 58/71] Remove `PipeFromStreamResult` Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 49 ----------------------------------- 1 file changed, 49 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 21436e1dc4..02a790d1ae 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -775,55 +775,6 @@ impl RpcModule { } } -/// The result obtain from calling [`PendingSubscription::pipe_from_try_stream`] that -/// can be utilized to execute specific operations depending on the result. -#[derive(Debug)] -pub enum PipeFromStreamResult { - /// The connection was accepted and the pipe returned [`SubscriptionClosed::Success`]. - Success(Option), - /// The connection was accepted and the pipe returned [`SubscriptionClosed::Failed`] - /// with the provided error. - Failure(Option<(SubscriptionSink, ErrorObjectOwned)>), - /// The remote peer closed the connection or called the unsubscribe method. - RemotePeerAborted, -} - -impl PipeFromStreamResult { - /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Success(Some(_))`]. - /// After the function runs a new [`PipeFromStreamResult::Success(None)`] is returned. - /// - /// Otherwise, it leaves the object untouched. - pub fn on_success(self, func: F) -> PipeFromStreamResult - where - F: FnOnce(SubscriptionSink) -> (), - { - match self { - PipeFromStreamResult::Success(Some(sink)) => { - func(sink); - PipeFromStreamResult::Success(None) - } - _ => self - } - } - - /// Callback that will run the provided function if the result is [`PipeFromStreamResult::Failure(Some(_))`]. - /// After the function runs a new [`PipeFromStreamResult::Failure(None)`] is returned. - /// - /// Otherwise, it leaves the object untouched. - pub fn on_failure(self, func: F) -> PipeFromStreamResult - where - F: FnOnce(SubscriptionSink, ErrorObjectOwned) -> (), - { - match self { - PipeFromStreamResult::Failure(Some((sink, error))) => { - func(sink, error); - PipeFromStreamResult::Failure(None) - } - _ => self - } - } -} - /// The state of the [`SubscriptionSink`]. #[derive(Debug)] enum SubscriptionSinkState { From 45cfdf0819f9989426e442fae655057c51e6ec0e Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 15:56:27 +0300 Subject: [PATCH 59/71] Use valid Json-RPC return code for test errors Signed-off-by: Alexandru Vasile --- tests/tests/rpc_module.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index 309991243f..3e1530db76 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -368,10 +368,12 @@ async fn empty_subscription_without_server() { #[tokio::test] async fn rejected_subscription_without_server() { + // Specialized JSON-RPC server error for rejected purposes. + const ERROR: i32 = -32090; let mut module = RpcModule::new(()); module .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { - let err = ErrorObject::borrowed(0, &"rejected", None); + let err = ErrorObject::borrowed(ERROR, &"rejected", None); sink.reject(err.into_owned())?; Ok(()) }) @@ -379,5 +381,7 @@ async fn rejected_subscription_without_server() { let sub_err = module.subscribe("my_sub", EmptyParams::new()).await.unwrap_err(); - assert!(matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("rejected") && e.code() == 0)); + assert!( + matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("rejected") && e.code() == ERROR) + ); } From abbea2c07e4d8e0de09c14c288f825dd4e25bfcd Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 18:26:27 +0300 Subject: [PATCH 60/71] Remove `SubscriptionSinkState`" --- core/src/server/rpc_module.rs | 62 ----------------------------------- 1 file changed, 62 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 02a790d1ae..79799eb5a9 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1166,65 +1166,3 @@ impl Drop for Subscription { self.close(); } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn sink_state_pending() { - let mut state = SubscriptionSinkState::new(Id::Number(0)); - assert!(matches!(state, SubscriptionSinkState::Pending(Some(Id::Number(0))))); - - assert!(matches!(state.id(), Ok(Id::Number(0)))); - - assert!(matches!(state, SubscriptionSinkState::Pending(None))); - assert!(matches!(state.id(), Err(SubscriptionEmptyError))); - } - - #[test] - fn sink_state_accepted() { - let mut state = SubscriptionSinkState::new(Id::Number(0)); - let (_, rx) = watch::channel(()); - - // Invalid transition without consuming the ID. - assert!(matches!(state.accept(rx.clone()), Err(SubscriptionEmptyError))); - assert!(matches!(state, SubscriptionSinkState::Pending(Some(Id::Number(0))))); - - // Transition to accepted. - assert!(matches!(state.id(), Ok(Id::Number(0)))); - let state = state.accept(rx.clone()); - assert!(matches!(state, Ok(SubscriptionSinkState::Accepted(_)))); - let mut state = state.unwrap(); - - // Double transition. - assert!(matches!(state.accept(rx.clone()), Err(SubscriptionEmptyError))); - // Invalid method call in this state. - assert!(matches!(state.id(), Err(SubscriptionEmptyError))); - // Invalid transition. - assert!(matches!(state.reject(), Err(SubscriptionEmptyError))); - } - - #[test] - fn sink_state_rejected() { - let mut state = SubscriptionSinkState::new(Id::Number(0)); - - // Invalid transition without consuming the ID. - assert!(matches!(state.reject(), Err(SubscriptionEmptyError))); - assert!(matches!(state, SubscriptionSinkState::Pending(Some(Id::Number(0))))); - - // Transition to rejected. - assert!(matches!(state.id(), Ok(Id::Number(0)))); - let state = state.reject(); - assert!(matches!(state, Ok(SubscriptionSinkState::Rejected))); - let mut state = state.unwrap(); - - // Double transition. - assert!(matches!(state.reject(), Err(SubscriptionEmptyError))); - // Invalid method call in this state. - assert!(matches!(state.id(), Err(SubscriptionEmptyError))); - // Invalid transition. - let (_, rx) = watch::channel(()); - assert!(matches!(state.accept(rx), Err(SubscriptionEmptyError))); - } -} From 8d3f7e534ea7dee6e05067c6b0ac256d7823fc40 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 19:02:25 +0300 Subject: [PATCH 61/71] Remodel state machine using `Option`s for `SubscriptionSink`s Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 94 +++++++++++------------------------ 1 file changed, 29 insertions(+), 65 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 79799eb5a9..a44c7c23e0 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -743,7 +743,8 @@ impl RpcModule { method: notif_method_name, subscribers: subscribers.clone(), uniq_sub: SubscriptionKey { conn_id: conn.conn_id, sub_id }, - state: SubscriptionSinkState::new(id.clone().into_owned()), + id: Some(id.clone().into_owned()), + unsubscribe: None, _claimed: claimed, }; @@ -775,54 +776,6 @@ impl RpcModule { } } -/// The state of the [`SubscriptionSink`]. -#[derive(Debug)] -enum SubscriptionSinkState { - /// The subscription is pending and needs to be accepted or rejected - /// before utilizing the sink. This is the initial state of the sink. - /// - /// This state contains the request ID of the subscription. - Pending(Option>), - /// The subscription was accepted via the [`SubscriptionSink::accept`] call. - /// This state contains the future that returns when the unsubscribe method has been called. - Accepted(watch::Receiver<()>), - /// The subscription was rejected via the [`SubscriptionSink::reject`] call. - /// In this state, the subscription cannot be utilized. - Rejected, -} - -impl SubscriptionSinkState { - /// Initialize the sink's state from the subscription ID. - fn new(id: Id<'static>) -> Self { - SubscriptionSinkState::Pending(Some(id)) - } - - /// Takes the ID out of the `Pending` state. - fn id(&mut self) -> Result, SubscriptionEmptyError> { - match self { - SubscriptionSinkState::Pending(id) => id.take().ok_or(SubscriptionEmptyError), - _ => Err(SubscriptionEmptyError), - } - } - - /// Advance the state of the sink to the accepted state. - fn accept(&self, rx: watch::Receiver<()>) -> Result { - match self { - // Cannot transition to accepted if the ID was not previously consumed. - SubscriptionSinkState::Pending(None) => Ok(SubscriptionSinkState::Accepted(rx)), - _ => Err(SubscriptionEmptyError), - } - } - - /// Reject the subscription. - fn reject(&self) -> Result { - match self { - SubscriptionSinkState::Pending(None) => Ok(SubscriptionSinkState::Rejected), - _ => Err(SubscriptionEmptyError), - } - } -} - /// Represents a single subscription. #[derive(Debug)] pub struct SubscriptionSink { @@ -836,8 +789,14 @@ pub struct SubscriptionSink { subscribers: Subscribers, /// Unique subscription. uniq_sub: SubscriptionKey, - /// The state of the subscription sink. - state: SubscriptionSinkState, + /// Id of the subscription. + /// + /// *Note*: Having some value means the subscription was not accepted or rejected yet. + id: Option>, + /// Returns when the unsubscribe method has been called. + /// + /// *Note*: Have some values means the subscription was accepted. + unsubscribe: Option>, /// Claimed resources. _claimed: Option, } @@ -845,10 +804,9 @@ pub struct SubscriptionSink { impl SubscriptionSink { /// Reject the subscription call from [`ErrorObject`]. pub fn reject(&mut self, err: impl Into) -> Result<(), SubscriptionEmptyError> { - let id = self.state.id()?; + let id = self.id.take().ok_or(SubscriptionEmptyError)?; if self.inner.send_error(id, err.into()) { - self.state = self.state.reject()?; Ok(()) } else { Err(SubscriptionEmptyError) @@ -859,12 +817,12 @@ impl SubscriptionSink { /// /// Fails if the connection was closed, or if called multiple times. pub fn accept(&mut self) -> Result<(), SubscriptionEmptyError> { - let id = self.state.id()?; + let id = self.id.take().ok_or(SubscriptionEmptyError)?; if self.inner.send_response(id, &self.uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); self.subscribers.lock().insert(self.uniq_sub.clone(), (self.inner.clone(), tx)); - self.state = self.state.accept(rx)?; + self.unsubscribe = Some(rx); Ok(()) } else { Err(SubscriptionEmptyError) @@ -875,10 +833,16 @@ impl SubscriptionSink { /// /// Fails if the accept function fails internally, or if the subscription was rejected. fn maybe_accept(&mut self) -> Result<(), SubscriptionEmptyError> { - match self.state { - SubscriptionSinkState::Pending(_) => self.accept(), - SubscriptionSinkState::Accepted(_) => Ok(()), - SubscriptionSinkState::Rejected => Err(SubscriptionEmptyError), + // Pending subscription. + if self.id.is_some() { + return self.accept(); + } + + // Subscription accepted. + if self.unsubscribe.is_some() { + Ok(()) + } else { + Err(SubscriptionEmptyError) } } @@ -959,8 +923,8 @@ impl SubscriptionSink { } }; - let mut sub_closed = match &self.state { - SubscriptionSinkState::Accepted(rx) => rx.clone(), + let mut sub_closed = match self.unsubscribe.as_ref() { + Some(rx) => rx.clone(), _ => { let err = ErrorObject::owned( INTERNAL_ERROR_CODE, @@ -1043,9 +1007,9 @@ impl SubscriptionSink { } fn is_active_subscription(&self) -> bool { - match &self.state { - SubscriptionSinkState::Accepted(unsubscribe) => !unsubscribe.has_changed().is_err(), - _ => false + match self.unsubscribe.as_ref() { + Some(unsubscribe) => !unsubscribe.has_changed().is_err(), + _ => false, } } @@ -1101,7 +1065,7 @@ impl SubscriptionSink { impl Drop for SubscriptionSink { fn drop(&mut self) { // Subscription was never accepted. - if let Ok(id) = self.state.id() { + if let Some(id) = self.id.take() { self.inner.send_error(id, ErrorCode::InvalidParams.into()); } else if self.is_active_subscription() { self.subscribers.lock().remove(&self.uniq_sub); From f5952de608f7d79a63669c5d887fb62ad01fb856 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 19:12:09 +0300 Subject: [PATCH 62/71] tests: Double accept / reject API for `SubscriptionSink` Signed-off-by: Alexandru Vasile --- tests/tests/rpc_module.rs | 56 ++++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/tests/tests/rpc_module.rs b/tests/tests/rpc_module.rs index 3e1530db76..b21a8c52ae 100644 --- a/tests/tests/rpc_module.rs +++ b/tests/tests/rpc_module.rs @@ -30,7 +30,7 @@ use std::time::Duration; use futures::StreamExt; use jsonrpsee::core::error::{Error, SubscriptionClosed}; use jsonrpsee::core::server::rpc_module::*; -use jsonrpsee::types::error::{CallError, ErrorCode, ErrorObject}; +use jsonrpsee::types::error::{CallError, ErrorCode, ErrorObject, PARSE_ERROR_CODE}; use jsonrpsee::types::{EmptyParams, Params}; use serde::{Deserialize, Serialize}; use tokio::time::interval; @@ -360,7 +360,6 @@ async fn empty_subscription_without_server() { .unwrap(); let sub_err = module.subscribe("my_sub", EmptyParams::new()).await.unwrap_err(); - assert!( matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("Invalid params") && e.code() == ErrorCode::InvalidParams.code()) ); @@ -368,20 +367,65 @@ async fn empty_subscription_without_server() { #[tokio::test] async fn rejected_subscription_without_server() { - // Specialized JSON-RPC server error for rejected purposes. - const ERROR: i32 = -32090; let mut module = RpcModule::new(()); module .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { - let err = ErrorObject::borrowed(ERROR, &"rejected", None); + let err = ErrorObject::borrowed(PARSE_ERROR_CODE, &"rejected", None); sink.reject(err.into_owned())?; Ok(()) }) .unwrap(); let sub_err = module.subscribe("my_sub", EmptyParams::new()).await.unwrap_err(); + assert!( + matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("rejected") && e.code() == PARSE_ERROR_CODE) + ); +} + +#[tokio::test] +async fn accepted_twice_subscription_without_server() { + let mut module = RpcModule::new(()); + module + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { + let res = sink.accept(); + assert!(matches!(res, Ok(()))); + + let res = sink.accept(); + assert!(matches!(res, Err(_))); + + let err = ErrorObject::borrowed(PARSE_ERROR_CODE, &"rejected", None); + let res = sink.reject(err.into_owned()); + assert!(matches!(res, Err(_))); + + Ok(()) + }) + .unwrap(); + let _ = module.subscribe("my_sub", EmptyParams::new()).await.expect("Subscription should not fail"); +} + +#[tokio::test] +async fn reject_twice_subscription_without_server() { + let mut module = RpcModule::new(()); + module + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { + let err = ErrorObject::borrowed(PARSE_ERROR_CODE, &"rejected", None); + let res = sink.reject(err.into_owned()); + assert!(matches!(res, Ok(()))); + + let err = ErrorObject::borrowed(PARSE_ERROR_CODE, &"rejected", None); + let res = sink.reject(err.into_owned()); + assert!(matches!(res, Err(_))); + + let res = sink.accept(); + assert!(matches!(res, Err(_))); + + Ok(()) + }) + .unwrap(); + + let sub_err = module.subscribe("my_sub", EmptyParams::new()).await.unwrap_err(); assert!( - matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("rejected") && e.code() == ERROR) + matches!(sub_err, Error::Call(CallError::Custom(e)) if e.message().contains("rejected") && e.code() == PARSE_ERROR_CODE) ); } From 8b04cd157ed3100a4ba84773e02379ff884612ce Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 19:38:20 +0300 Subject: [PATCH 63/71] Implement `SubscriptionAcceptRejectError` for error propagation Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 23 +++++++++++++---------- types/src/error.rs | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index a44c7c23e0..935c173525 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -39,10 +39,13 @@ use futures_channel::mpsc; use futures_util::future::Either; use futures_util::pin_mut; use futures_util::{future::BoxFuture, FutureExt, Stream, StreamExt, TryStream, TryStreamExt}; -use jsonrpsee_types::error::{CallError, ErrorCode, ErrorObject, ErrorObjectOwned, INTERNAL_ERROR_CODE, SUBSCRIPTION_CLOSED_WITH_ERROR}; +use jsonrpsee_types::error::{ + CallError, ErrorCode, ErrorObject, ErrorObjectOwned, INTERNAL_ERROR_CODE, + SUBSCRIPTION_CLOSED_WITH_ERROR, SubscriptionAcceptRejectError +}; use jsonrpsee_types::response::{SubscriptionError, SubscriptionPayloadError}; use jsonrpsee_types::{ - ErrorResponse, Id, Params, Request, Response, SubscriptionResult, SubscriptionEmptyError, + ErrorResponse, Id, Params, Request, Response, SubscriptionResult, SubscriptionId as RpcSubscriptionId, SubscriptionPayload, SubscriptionResponse }; use parking_lot::Mutex; @@ -803,21 +806,21 @@ pub struct SubscriptionSink { impl SubscriptionSink { /// Reject the subscription call from [`ErrorObject`]. - pub fn reject(&mut self, err: impl Into) -> Result<(), SubscriptionEmptyError> { - let id = self.id.take().ok_or(SubscriptionEmptyError)?; + pub fn reject(&mut self, err: impl Into) -> Result<(), SubscriptionAcceptRejectError> { + let id = self.id.take().ok_or(SubscriptionAcceptRejectError::AlreadyCalled)?; if self.inner.send_error(id, err.into()) { Ok(()) } else { - Err(SubscriptionEmptyError) + Err(SubscriptionAcceptRejectError::RemotePeerAborted) } } /// Attempt to accept the subscription and respond the subscription method call. /// /// Fails if the connection was closed, or if called multiple times. - pub fn accept(&mut self) -> Result<(), SubscriptionEmptyError> { - let id = self.id.take().ok_or(SubscriptionEmptyError)?; + pub fn accept(&mut self) -> Result<(), SubscriptionAcceptRejectError> { + let id = self.id.take().ok_or(SubscriptionAcceptRejectError::AlreadyCalled)?; if self.inner.send_response(id, &self.uniq_sub.sub_id) { let (tx, rx) = watch::channel(()); @@ -825,14 +828,14 @@ impl SubscriptionSink { self.unsubscribe = Some(rx); Ok(()) } else { - Err(SubscriptionEmptyError) + Err(SubscriptionAcceptRejectError::RemotePeerAborted) } } /// Accepts the subscription if previously not accepted. /// /// Fails if the accept function fails internally, or if the subscription was rejected. - fn maybe_accept(&mut self) -> Result<(), SubscriptionEmptyError> { + fn maybe_accept(&mut self) -> Result<(), SubscriptionAcceptRejectError> { // Pending subscription. if self.id.is_some() { return self.accept(); @@ -842,7 +845,7 @@ impl SubscriptionSink { if self.unsubscribe.is_some() { Ok(()) } else { - Err(SubscriptionEmptyError) + Err(SubscriptionAcceptRejectError::RemotePeerAborted) } } diff --git a/types/src/error.rs b/types/src/error.rs index 8f29a9d9a0..dfb19d4638 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -110,6 +110,21 @@ impl<'a> From> for SubscriptionEmptyError { } } +impl From for SubscriptionEmptyError { + fn from(_: SubscriptionAcceptRejectError) -> Self { + SubscriptionEmptyError + } +} + +/// The error returned while accepting or rejecting a subscription. +#[derive(Debug)] +pub enum SubscriptionAcceptRejectError { + /// The method was already called. + AlreadyCalled, + /// The remote peer closed the connection or called the unsubscribe method. + RemotePeerAborted, +} + /// Owned variant of [`ErrorObject`]. pub type ErrorObjectOwned = ErrorObject<'static>; From 61a03eb89b20ae9a664b94e592c24076923aa9f4 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 19:41:33 +0300 Subject: [PATCH 64/71] Remove `maybe_accept` wrapper Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 935c173525..e2d5f9e693 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -832,23 +832,6 @@ impl SubscriptionSink { } } - /// Accepts the subscription if previously not accepted. - /// - /// Fails if the accept function fails internally, or if the subscription was rejected. - fn maybe_accept(&mut self) -> Result<(), SubscriptionAcceptRejectError> { - // Pending subscription. - if self.id.is_some() { - return self.accept(); - } - - // Subscription accepted. - if self.unsubscribe.is_some() { - Ok(()) - } else { - Err(SubscriptionAcceptRejectError::RemotePeerAborted) - } - } - /// Send a message back to subscribers. /// /// Returns @@ -858,9 +841,10 @@ impl SubscriptionSink { /// - `Err(err)` if the message could not be serialized. pub fn send(&mut self, result: &T) -> Result { // Cannot accept the subscription. - if self.maybe_accept().is_err() { + if let Err(SubscriptionAcceptRejectError::RemotePeerAborted) = self.accept() { return Ok(false); } + // Only possible to trigger when the connection is dropped. if self.is_closed() { return Ok(false); @@ -915,7 +899,7 @@ impl SubscriptionSink { T: Serialize, E: std::fmt::Display, { - if self.maybe_accept().is_err() { + if let Err(SubscriptionAcceptRejectError::RemotePeerAborted) = self.accept() { return SubscriptionClosed::RemotePeerAborted; } From 2a36c26881a0703010894d0e0369775b89d40b61 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Tue, 28 Jun 2022 19:46:58 +0300 Subject: [PATCH 65/71] Update comments and documentation Signed-off-by: Alexandru Vasile --- proc-macros/src/lib.rs | 6 +++--- proc-macros/src/rpc_macro.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/proc-macros/src/lib.rs b/proc-macros/src/lib.rs index a809af838e..0b0b21cae3 100644 --- a/proc-macros/src/lib.rs +++ b/proc-macros/src/lib.rs @@ -54,8 +54,8 @@ pub(crate) mod visitor; /// - The trait will have one additional (already implemented) method, `into_rpc`, which turns any object that /// implements the server trait into an `RpcModule`. /// - For subscription methods: -/// - There will be one additional argument inserted right after `&self`: `subscription_sink: PendingSubscription`. -/// It should be used accept or reject a pending subscription. +/// - There will be one additional argument inserted right after `&self`: `subscription_sink: SubscriptionSink`. +/// It should be used to accept or reject a subscription and send data back to subscribers. /// - The return type of the subscription method is `SubscriptionResult` for improved ergonomics. /// /// Since this macro can generate up to two traits, both server and client traits will have @@ -100,7 +100,7 @@ pub(crate) mod visitor; /// fn sync_method(&self) -> String; /// /// // Note that `subscription_sink` and `SubscriptionResult` were added automatically. -/// fn sub(&self, subscription_sink: PendingSubscription) -> SubscriptionResult; +/// fn sub(&self, subscription_sink: SubscriptionResult) -> SubscriptionResult; /// /// fn into_rpc(self) -> Result { /// // Actual implementation stripped, but inside we will create diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index c12e43bf35..8418740a0b 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -277,7 +277,7 @@ impl RpcDescription { if !matches!(method.sig.output, syn::ReturnType::Default) { return Err(syn::Error::new_spanned( &method, - "Subscription methods must not return anything; the error must send via subscription via either `PendingSubscription::reject` or `SubscripionSink::close`", + "Subscription methods must not return anything; the error must send via subscription via either `SubscriptionSink::reject` or `SubscriptionSink::close`", )); } From 13de2320acc0214db412da5463582cdf8aae95f7 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:02:55 +0300 Subject: [PATCH 66/71] Update core/src/server/rpc_module.rs Co-authored-by: Niklas Adolfsson --- core/src/server/rpc_module.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index e2d5f9e693..2a4fde12f3 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -792,7 +792,8 @@ pub struct SubscriptionSink { subscribers: Subscribers, /// Unique subscription. uniq_sub: SubscriptionKey, - /// Id of the subscription. + /// Id of the `subscription call` (i.e. not the same as subscription id) which is used + /// to reply to subscription method call and must only be used once. /// /// *Note*: Having some value means the subscription was not accepted or rejected yet. id: Option>, From 9df5b426a1540f785f32c79f21add937e397ddd0 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:04:35 +0300 Subject: [PATCH 67/71] Update core/src/server/rpc_module.rs Co-authored-by: Niklas Adolfsson --- core/src/server/rpc_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 2a4fde12f3..32b28e3562 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -753,7 +753,7 @@ impl RpcModule { // The callback returns an empty `SubscriptionError` for improved API ergonomics. if let Err(err) = callback(params, sink, ctx.clone()) { - tracing::warn!("subscribe call `{}` failed with err={:?}", subscribe_method_name, err); + tracing::warn!("subscribe call `{}` failed", subscribe_method_name); } true From 77ab7dec4fd52d845296e01935196dce575442f3 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 29 Jun 2022 17:11:33 +0300 Subject: [PATCH 68/71] rpc_server: Add type alias for unsubscription calls Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 32b28e3562..5e486356cc 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -752,7 +752,7 @@ impl RpcModule { }; // The callback returns an empty `SubscriptionError` for improved API ergonomics. - if let Err(err) = callback(params, sink, ctx.clone()) { + if let Err(_) = callback(params, sink, ctx.clone()) { tracing::warn!("subscribe call `{}` failed", subscribe_method_name); } @@ -779,6 +779,9 @@ impl RpcModule { } } +/// Returns once the unsubscribe method has been called. +type UnsubscribeCall = Option>; + /// Represents a single subscription. #[derive(Debug)] pub struct SubscriptionSink { @@ -792,15 +795,13 @@ pub struct SubscriptionSink { subscribers: Subscribers, /// Unique subscription. uniq_sub: SubscriptionKey, - /// Id of the `subscription call` (i.e. not the same as subscription id) which is used - /// to reply to subscription method call and must only be used once. + /// Id of the `subscription call` (i.e. not the same as subscription id) which is used + /// to reply to subscription method call and must only be used once. /// /// *Note*: Having some value means the subscription was not accepted or rejected yet. id: Option>, - /// Returns when the unsubscribe method has been called. - /// - /// *Note*: Have some values means the subscription was accepted. - unsubscribe: Option>, + /// Having some value means the subscription was accepted. + unsubscribe: UnsubscribeCall, /// Claimed resources. _claimed: Option, } @@ -1052,8 +1053,8 @@ impl SubscriptionSink { impl Drop for SubscriptionSink { fn drop(&mut self) { - // Subscription was never accepted. if let Some(id) = self.id.take() { + // Subscription was never accepted / rejected. self.inner.send_error(id, ErrorCode::InvalidParams.into()); } else if self.is_active_subscription() { self.subscribers.lock().remove(&self.uniq_sub); From 0f255ad3e83d84889e0369dc947d81f357ff4036 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 29 Jun 2022 17:15:06 +0300 Subject: [PATCH 69/71] rpc_server: Improve comment regarding dropped error Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 5e486356cc..9b21fbd6fe 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -751,7 +751,7 @@ impl RpcModule { _claimed: claimed, }; - // The callback returns an empty `SubscriptionError` for improved API ergonomics. + // The callback returns a `SubscriptionResult` for better ergonomics and is not propagated further. if let Err(_) = callback(params, sink, ctx.clone()) { tracing::warn!("subscribe call `{}` failed", subscribe_method_name); } From 997706a3d3899d2595aafe1bef6d7a90a4d91694 Mon Sep 17 00:00:00 2001 From: Alexandru Vasile Date: Wed, 29 Jun 2022 17:26:15 +0300 Subject: [PATCH 70/71] style: Single line return errors Signed-off-by: Alexandru Vasile --- core/src/server/rpc_module.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index 9b21fbd6fe..b3a88e1f86 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -907,21 +907,16 @@ impl SubscriptionSink { let conn_closed = match self.close_notify.as_ref().map(|cn| cn.handle()) { Some(cn) => cn, - None => { - return SubscriptionClosed::RemotePeerAborted; - } + None => return SubscriptionClosed::RemotePeerAborted, }; let mut sub_closed = match self.unsubscribe.as_ref() { Some(rx) => rx.clone(), - _ => { - let err = ErrorObject::owned( + _ => return SubscriptionClosed::Failed(ErrorObject::owned( INTERNAL_ERROR_CODE, "Unsubscribe watcher not set after accepting the subscription".to_string(), None::<()> - ); - return SubscriptionClosed::Failed(err); - } + )), }; let sub_closed_fut = sub_closed.changed(); From c4f0f4975cc9ef351af3340bbf38fd057400a880 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 29 Jun 2022 17:08:31 +0100 Subject: [PATCH 71/71] Make comment more verbose --- core/src/server/rpc_module.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/server/rpc_module.rs b/core/src/server/rpc_module.rs index b3a88e1f86..8020ae9199 100644 --- a/core/src/server/rpc_module.rs +++ b/core/src/server/rpc_module.rs @@ -1049,7 +1049,10 @@ impl SubscriptionSink { impl Drop for SubscriptionSink { fn drop(&mut self) { if let Some(id) = self.id.take() { - // Subscription was never accepted / rejected. + // Subscription was never accepted / rejected. As such, + // we default to assuming that the params were invalid, + // because that's how the previous PendingSubscription logic + // worked. self.inner.send_error(id, ErrorCode::InvalidParams.into()); } else if self.is_active_subscription() { self.subscribers.lock().remove(&self.uniq_sub);