Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc module: report error on invalid subscription #561

Merged
merged 4 commits into from
Nov 18, 2021
Merged

rpc module: report error on invalid subscription #561

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71eb83d
rpc module: report error on invalid subscription
niklasad1 Nov 15, 2021
8d5730e
fix tests
niklasad1 Nov 15, 2021
be1c302
remove some boiler plate
niklasad1 Nov 16, 2021
4e25662
remove unused code
niklasad1 Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
rpc module: report error on invalid subscription
  • Loading branch information
@niklasad1
niklasad1 committed Nov 15, 2021
commit 71eb83d53f5057365e521d381100fda7b7c8e70a
4 changes: 4 additions & 0 deletions types/src/v2/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub const PARSE_ERROR_CODE: i32 = -32700;
pub const OVERSIZED_REQUEST_CODE: i32 = -32701;
/// Oversized response error code.
pub const OVERSIZED_RESPONSE_CODE: i32 = -32702;
/// Oversized response error code.
pub const INVALID_SUBSCRIPTION_CODE: i32 = -32703;
Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, technically these error codes should be used for application errors.

To be really nitpicky I guess we should use -32000 to -32099 Server defined errors here

Copy link
Contributor

Choose a reason for hiding this comment

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

Given this is a new code, do you think we should do the right thing for new error codes and stay within the right range?

Copy link
Member Author

@niklasad1 niklasad1 Nov 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

yeah, I used -32002 for this however the message (according to the spec it should be "Server error") is poor so we would have add everything in the data field

/// Internal error code.
pub const INTERNAL_ERROR_CODE: i32 = -32603;
/// Invalid params error code.
Expand All @@ -105,6 +107,8 @@ pub const PARSE_ERROR_MSG: &str = "Parse error";
pub const OVERSIZED_REQUEST_MSG: &str = "Request is too big";
/// Oversized response message
pub const OVERSIZED_RESPONSE_MSG: &str = "Response is too big";
/// Invalid subscription ID in unsubscription.
pub const INVALID_SUBSCRIPTION_MSG: &str = "Invalid subscription ID";
/// Internal error message.
pub const INTERNAL_ERROR_MSG: &str = "Internal error";
/// Invalid params error message.
Expand Down
49 changes: 46 additions & 3 deletions utils/src/server/rpc_module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ use crate::server::resource_limiting::{ResourceGuard, ResourceTable, ResourceVec
use beef::Cow;
use futures_channel::{mpsc, oneshot};
use futures_util::{future::BoxFuture, FutureExt, StreamExt};
use jsonrpsee_types::to_json_raw_value;
use jsonrpsee_types::v2::error::{INVALID_SUBSCRIPTION_CODE, INVALID_SUBSCRIPTION_MSG};
use jsonrpsee_types::v2::ErrorObject;
use jsonrpsee_types::{
error::{Error, SubscriptionClosedError},
traits::ToRpcParams,
Expand Down Expand Up @@ -609,8 +612,19 @@ impl<Context: Send + Sync + 'static> RpcModule<Context> {
return;
}
};
subscribers.lock().remove(&SubscriptionKey { conn_id, sub_id });
send_response(id, tx, "Unsubscribed", max_response_size);
if subscribers.lock().remove(&SubscriptionKey { conn_id, sub_id }).is_some() {
send_response(id, tx, "Unsubscribed", max_response_size);
} else {
send_error(
id,
tx,
ErrorObject {
code: ErrorCode::ServerError(INVALID_SUBSCRIPTION_CODE),
message: INVALID_SUBSCRIPTION_MSG,
data: to_json_raw_value(&sub_id).ok().as_deref(),
},
)
}
})),
);
}
Expand Down Expand Up @@ -749,7 +763,7 @@ impl Drop for TestSubscription {
#[cfg(test)]
mod tests {
use super::*;
use jsonrpsee_types::v2;
use jsonrpsee_types::v2::{self, error::INVALID_SUBSCRIPTION_MSG, RpcError};
use serde::Deserialize;
use std::collections::HashMap;

Expand Down Expand Up @@ -938,4 +952,33 @@ mod tests {
assert_eq!(sub_closed_err.subscription_id(), my_sub.subscription_id());
assert_eq!(sub_closed_err.close_reason(), "Closed by the server");
}

#[tokio::test]
async fn unsubscribe_twice_errors() {
let mut module = RpcModule::new(());
module.register_subscription("my_sub", "my_unsub", |_, _, _| Ok(())).unwrap();

fn deser_call<T: DeserializeOwned>(raw: String) -> T {
let out: Response<T> = serde_json::from_str(&raw).unwrap();
out.result
}

let sub_id: u64 = deser_call(module.call_with("my_sub", Vec::<()>::new()).await.unwrap());
let unsub: String = deser_call(module.call_with("my_unsub", [sub_id]).await.unwrap());
assert_eq!(&unsub, "Unsubscribed");
let raw = module.call_with("my_unsub", [sub_id]).await.unwrap();
let unsub_2: RpcError = serde_json::from_str(&raw).unwrap();
assert_eq!(
unsub_2,
RpcError {
jsonrpc: TwoPointZero,
error: v2::ErrorObject {
code: ErrorCode::ServerError(INVALID_SUBSCRIPTION_CODE),
message: INVALID_SUBSCRIPTION_MSG,
data: None
},
id: Id::Number(0)
}
)
}
}