From 7d475758d9b20eab568926df423eeb41b8be9d72 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Sat, 9 Apr 2022 11:51:36 +0200 Subject: [PATCH 1/3] update jsonrpsee --- Cargo.lock | 28 ++-- Cargo.toml | 3 + client/beefy/rpc/src/lib.rs | 28 ++-- client/consensus/manual-seal/src/error.rs | 7 +- client/finality-grandpa/rpc/src/error.rs | 4 +- client/finality-grandpa/rpc/src/lib.rs | 27 ++-- client/rpc-api/src/author/error.rs | 158 +++++++++++---------- client/rpc-api/src/author/mod.rs | 2 +- client/rpc-api/src/chain/error.rs | 10 +- client/rpc-api/src/chain/mod.rs | 12 +- client/rpc-api/src/dev/error.rs | 22 ++- client/rpc-api/src/offchain/error.rs | 12 +- client/rpc-api/src/state/error.rs | 19 ++- client/rpc-api/src/state/mod.rs | 4 +- client/rpc-api/src/system/error.rs | 13 +- client/rpc/src/author/mod.rs | 24 ++-- client/rpc/src/author/tests.rs | 26 ++-- client/rpc/src/chain/chain_full.rs | 22 +-- client/rpc/src/chain/mod.rs | 20 +-- client/rpc/src/chain/tests.rs | 4 +- client/rpc/src/dev/tests.rs | 2 +- client/rpc/src/state/mod.rs | 28 ++-- client/rpc/src/state/state_full.rs | 93 +++++++----- client/rpc/src/state/tests.rs | 14 +- client/rpc/src/system/tests.rs | 16 ++- frame/contracts/rpc/src/lib.rs | 24 ++-- frame/merkle-mountain-range/rpc/Cargo.toml | 1 - frame/merkle-mountain-range/rpc/src/lib.rs | 41 +++--- utils/frame/rpc/system/src/lib.rs | 54 +++---- 29 files changed, 379 insertions(+), 339 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7d859ba067b3..521ad9b9a3582 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3159,8 +3159,7 @@ dependencies = [ [[package]] name = "jsonrpsee" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91dc760c341fa81173f9a434931aaf32baad5552b0230cc6c93e8fb7eaad4c19" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "jsonrpsee-core", "jsonrpsee-http-server", @@ -3173,8 +3172,7 @@ dependencies = [ [[package]] name = "jsonrpsee-client-transport" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "765f7a36d5087f74e3b3b47805c2188fef8eb54afcb587b078d9f8ebfe9c7220" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "futures", "http", @@ -3194,8 +3192,7 @@ dependencies = [ [[package]] name = "jsonrpsee-core" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82ef77ecd20c2254d54f5da8c0738eacca61e6b6511268a8f2753e3148c6c706" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "anyhow", "arrayvec 0.7.1", @@ -3219,8 +3216,7 @@ dependencies = [ [[package]] name = "jsonrpsee-http-server" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d35477aab03691360d21a77dd475f384474bc138c2051aafa766fe4aed50ac50" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "futures-channel", "futures-util", @@ -3238,8 +3234,7 @@ dependencies = [ [[package]] name = "jsonrpsee-proc-macros" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7291c72805bc7d413b457e50d8ef3e87aa554da65ecbbc278abb7dfc283e7f0" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -3250,8 +3245,7 @@ dependencies = [ [[package]] name = "jsonrpsee-types" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38b6aa52f322cbf20c762407629b8300f39bcc0cf0619840d9252a2f65fd2dd9" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "anyhow", "beef", @@ -3264,8 +3258,7 @@ dependencies = [ [[package]] name = "jsonrpsee-ws-client" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd66d18bab78d956df24dd0d2e41e4c00afbb818fda94a98264bdd12ce8506ac" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "jsonrpsee-client-transport", "jsonrpsee-core", @@ -3275,8 +3268,7 @@ dependencies = [ [[package]] name = "jsonrpsee-ws-server" version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a382e22db11cd9a1f04f5a4cc5446f155a3cd20cd1778fc65f30a76aff524120" +source = "git+https://github.com/paritytech/jsonrpsee?branch=na-jsonrpsee-pubsub-should-not-respond-to-bad-params#3da7b0fa6cfb97cfe55829b19397e73aca7587c5" dependencies = [ "futures-channel", "futures-util", @@ -10943,9 +10935,9 @@ version = "1.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4ee73e6e4924fe940354b8d4d98cad5231175d615cd855b758adc658c0aac6a0" dependencies = [ - "cfg-if 1.0.0", + "cfg-if 0.1.10", "digest 0.10.3", - "rand 0.8.4", + "rand 0.6.5", "static_assertions", ] diff --git a/Cargo.toml b/Cargo.toml index 13657dd1234a5..caa48bfc04290 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -294,3 +294,6 @@ inherits = "release" lto = "fat" # https://doc.rust-lang.org/rustc/codegen-options/index.html#codegen-units codegen-units = 1 + +[patch.crates-io] +jsonrpsee = { git = "https://github.com/paritytech/jsonrpsee", branch = "na-jsonrpsee-pubsub-should-not-respond-to-bad-params" } diff --git a/client/beefy/rpc/src/lib.rs b/client/beefy/rpc/src/lib.rs index 0c29f0e60952c..e8e847006c605 100644 --- a/client/beefy/rpc/src/lib.rs +++ b/client/beefy/rpc/src/lib.rs @@ -33,7 +33,7 @@ use futures::{ use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, - SubscriptionSink, + PendingSubscription, }; use log::warn; @@ -61,7 +61,7 @@ pub trait BeefyApi { unsubscribe = "beefy_unsubscribeJustifications", item = Notification, )] - fn subscribe_justifications(&self) -> RpcResult<()>; + fn subscribe_justifications(&self); /// Returns hash of the latest BEEFY finalized block as seen by this client. /// @@ -109,17 +109,20 @@ impl BeefyApiServer where Block: BlockT, { - fn subscribe_justifications(&self, sink: SubscriptionSink) -> RpcResult<()> { + fn subscribe_justifications(&self, pending: PendingSubscription) { let stream = self .signed_commitment_stream .subscribe() .map(|sc| notification::EncodedSignedCommitment::new::(sc)); - let fut = sink.pipe_from_stream(stream).map(|_| ()).boxed(); + let fut = async move { + if let Some(mut sink) = pending.accept() { + sink.pipe_from_stream(stream).await; + } + } + .boxed(); - self.executor - .spawn_obj(fut.into()) - .map_err(|e| JsonRpseeError::to_call_error(e)) + let _ = self.executor.spawn_obj(fut.into()); } async fn latest_finalized(&self) -> RpcResult { @@ -207,7 +210,7 @@ mod tests { if response != not_ready { assert_eq!(response, expected); // Success - return + return; } std::thread::sleep(std::time::Duration::from_millis(50)) } @@ -236,14 +239,11 @@ mod tests { ); let (response, _) = rpc.raw_json_request(&unsub_req).await.unwrap(); - assert_eq!(response, r#"{"jsonrpc":"2.0","result":"Unsubscribed","id":1}"#); + assert_eq!(response, r#"{"jsonrpc":"2.0","result":true,"id":1}"#); // Unsubscribe again and fail let (response, _) = rpc.raw_json_request(&unsub_req).await.unwrap(); - let expected = format!( - r#"{{"jsonrpc":"2.0","error":{{"code":-32002,"message":"Server error","data":"Invalid subscription ID={}"}},"id":1}}"#, - ser_id - ); + let expected = r#"{"jsonrpc":"2.0","result":false,"id":1}"#; assert_eq!(response, expected); } @@ -264,7 +264,7 @@ mod tests { ) .await .unwrap(); - let expected = r#"{"jsonrpc":"2.0","error":{"code":-32002,"message":"Server error","data":"Invalid subscription ID=\"FOO\""},"id":1}"#; + let expected = r#"{"jsonrpc":"2.0","result":false,"id":1}"#; assert_eq!(response, expected); } diff --git a/client/consensus/manual-seal/src/error.rs b/client/consensus/manual-seal/src/error.rs index f0193e205d7b4..a056c541c3cef 100644 --- a/client/consensus/manual-seal/src/error.rs +++ b/client/consensus/manual-seal/src/error.rs @@ -20,7 +20,10 @@ //! This is suitable for a testing environment. use futures::channel::{mpsc::SendError, oneshot}; -use jsonrpsee::{core::Error as JsonRpseeError, types::error::CallError}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; use sc_consensus::ImportResult; use sp_blockchain::Error as BlockchainError; use sp_consensus::Error as ConsensusError; @@ -105,6 +108,6 @@ impl Error { impl From for JsonRpseeError { fn from(err: Error) -> Self { - CallError::Custom { code: err.to_code(), message: err.to_string(), data: None }.into() + CallError::Custom(ErrorObject::owned(err.to_code(), err.to_string(), None::<()>)).into() } } diff --git a/client/finality-grandpa/rpc/src/error.rs b/client/finality-grandpa/rpc/src/error.rs index 6636d8e549d8e..6ddeebfb1274f 100644 --- a/client/finality-grandpa/rpc/src/error.rs +++ b/client/finality-grandpa/rpc/src/error.rs @@ -16,7 +16,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use jsonrpsee::types::error::CallError; +use jsonrpsee::types::error::{CallError, ErrorObject}; #[derive(Debug, thiserror::Error)] /// Top-level error type for the RPC handler @@ -62,7 +62,7 @@ impl From for CallError { fn from(error: Error) -> Self { let message = error.to_string(); let code = ErrorCode::from(error); - Self::Custom { code: code as i32, message, data: None } + Self::Custom(ErrorObject::owned(code as i32, message, None::<()>)) } } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 8735a40e288f1..c858d4e528d82 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -26,7 +26,7 @@ use std::sync::Arc; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, - SubscriptionSink, + PendingSubscription, }; mod error; @@ -57,7 +57,7 @@ pub trait GrandpaApi { unsubscribe = "grandpa_unsubscribeJustifications", item = Notification )] - fn subscribe_justifications(&self) -> RpcResult<()>; + fn subscribe_justifications(&self); /// Prove finality for the given block number by returning the Justification for the last block /// in the set and all the intermediary headers to link them together. @@ -103,17 +103,21 @@ where .map_err(|e| JsonRpseeError::to_call_error(e)) } - fn subscribe_justifications(&self, sink: SubscriptionSink) -> RpcResult<()> { + fn subscribe_justifications(&self, pending: PendingSubscription) { let stream = self.justification_stream.subscribe().map( |x: sc_finality_grandpa::GrandpaJustification| { JustificationNotification::from(x) }, ); - let fut = sink.pipe_from_stream(stream).map(|_| ()).boxed(); - self.executor - .spawn_obj(fut.into()) - .map_err(|e| JsonRpseeError::to_call_error(e)) + let fut = async move { + if let Some(mut sink) = pending.accept() { + sink.pipe_from_stream(stream).await; + } + } + .boxed(); + + let _ = self.executor.spawn_obj(fut.into()); } async fn prove_finality( @@ -322,14 +326,11 @@ mod tests { ); let (response, _) = rpc.raw_json_request(&unsub_req).await.unwrap(); - assert_eq!(response, r#"{"jsonrpc":"2.0","result":"Unsubscribed","id":1}"#); + assert_eq!(response, r#"{"jsonrpc":"2.0","result":true,"id":1}"#); // Unsubscribe again and fail let (response, _) = rpc.raw_json_request(&unsub_req).await.unwrap(); - let expected = format!( - r#"{{"jsonrpc":"2.0","error":{{"code":-32002,"message":"Server error","data":"Invalid subscription ID={}"}},"id":1}}"#, - ser_id - ); + let expected = r#"{"jsonrpc":"2.0","result":false,"id":1}"#; assert_eq!(response, expected); } @@ -350,7 +351,7 @@ mod tests { ) .await .unwrap(); - let expected = r#"{"jsonrpc":"2.0","error":{"code":-32002,"message":"Server error","data":"Invalid subscription ID=\"FOO\""},"id":1}"#; + let expected = r#"{"jsonrpc":"2.0","result":false,"id":1}"#; assert_eq!(response, expected); } diff --git a/client/rpc-api/src/author/error.rs b/client/rpc-api/src/author/error.rs index 9d27a61ba121d..b411541aee0d9 100644 --- a/client/rpc-api/src/author/error.rs +++ b/client/rpc-api/src/author/error.rs @@ -19,8 +19,8 @@ //! Authoring RPC module errors. use jsonrpsee::{ - core::{to_json_raw_value, Error as JsonRpseeError, JsonRawValue}, - types::error::CallError, + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, }; use sp_runtime::transaction_validity::InvalidTransaction; @@ -92,87 +92,95 @@ impl From for JsonRpseeError { use sc_transaction_pool_api::error::Error as PoolError; match e { - Error::BadFormat(e) => CallError::Custom { - code: BAD_FORMAT, - message: format!("Extrinsic has invalid format: {}", e).into(), - data: None, - }, - Error::Verification(e) => CallError::Custom { - code: VERIFICATION_ERROR, - message: format!("Verification Error: {}", e).into(), - data: JsonRawValue::from_string(format!("\"{:?}\"", e)).ok(), - }, - Error::Pool(PoolError::InvalidTransaction(InvalidTransaction::Custom(e))) => CallError::Custom { - code: POOL_INVALID_TX, - message: "Invalid Transaction".into(), - data: JsonRawValue::from_string(format!("\"Custom error: {}\"", e)).ok(), + Error::BadFormat(e) => CallError::Custom(ErrorObject::owned( + BAD_FORMAT, + format!("Extrinsic has invalid format: {}", e), + None::<()>, + )), + Error::Verification(e) => CallError::Custom(ErrorObject::owned( + VERIFICATION_ERROR, + format!("Verification Error: {}", e), + Some(format!("{:?}", e)), + )), + Error::Pool(PoolError::InvalidTransaction(InvalidTransaction::Custom(e))) => { + CallError::Custom(ErrorObject::owned( + POOL_INVALID_TX, + "Invalid Transaction", + Some(format!("Custom error: {}", e)), + )) }, Error::Pool(PoolError::InvalidTransaction(e)) => { - CallError::Custom { - code: POOL_INVALID_TX, - message: "Invalid Transaction".into(), - data: to_json_raw_value(&e).ok(), - } - }, - Error::Pool(PoolError::UnknownTransaction(e)) => CallError::Custom { - code: POOL_UNKNOWN_VALIDITY, - message: "Unknown Transaction Validity".into(), - data: to_json_raw_value(&e).ok(), - }, - Error::Pool(PoolError::TemporarilyBanned) => CallError::Custom { - code: (POOL_TEMPORARILY_BANNED), - message: "Transaction is temporarily banned".into(), - data: None, - }, - Error::Pool(PoolError::AlreadyImported(hash)) => CallError::Custom { - code: (POOL_ALREADY_IMPORTED), - message: "Transaction Already Imported".into(), - data: JsonRawValue::from_string(format!("\"{:?}\"", hash)).ok(), - }, - Error::Pool(PoolError::TooLowPriority { old, new }) => CallError::Custom { - code: (POOL_TOO_LOW_PRIORITY), - message: format!("Priority is too low: ({} vs {})", old, new), - data: to_json_raw_value(&"The transaction has too low priority to replace another transaction already in the pool.").ok(), - }, - Error::Pool(PoolError::CycleDetected) => CallError::Custom { - code: (POOL_CYCLE_DETECTED), - message: "Cycle Detected".into(), - data: None, - }, - Error::Pool(PoolError::ImmediatelyDropped) => CallError::Custom { - code: (POOL_IMMEDIATELY_DROPPED), - message: "Immediately Dropped".into(), - data: to_json_raw_value(&"The transaction couldn't enter the pool because of the limit").ok(), - }, - Error::Pool(PoolError::Unactionable) => CallError::Custom { - code: (POOL_UNACTIONABLE), - message: "Unactionable".into(), - data: to_json_raw_value( - &"The transaction is unactionable since it is not propagable and \ - the local node does not author blocks" - ).ok(), - }, - Error::Pool(PoolError::NoTagsProvided) => CallError::Custom { - code: (POOL_NO_TAGS), - message: "No tags provided".into(), - data: to_json_raw_value( - &"Transaction does not provide any tags, so the pool can't identify it" - ).ok(), + let msg: &str = e.into(); + CallError::Custom(ErrorObject::owned( + POOL_INVALID_TX, + "Invalid Transaction", + Some(msg), + )) }, - Error::Pool(PoolError::InvalidBlockId(_)) => CallError::Custom { - code: (POOL_INVALID_BLOCK_ID), - message: "The provided block ID is not valid".into(), - data: None, + Error::Pool(PoolError::UnknownTransaction(e)) => { + CallError::Custom(ErrorObject::owned( + POOL_UNKNOWN_VALIDITY, + "Unknown Transaction Validity", + Some(format!("{:?}", e)), + )) }, - Error::Pool(PoolError::RejectedFutureTransaction) => CallError::Custom { - code: (POOL_FUTURE_TX), - message: "The pool is not accepting future transactions".into(), - data: None, + Error::Pool(PoolError::TemporarilyBanned) => + CallError::Custom(ErrorObject::owned( + POOL_TEMPORARILY_BANNED, + "Transaction is temporarily banned", + None::<()>, + )), + Error::Pool(PoolError::AlreadyImported(hash)) => + CallError::Custom(ErrorObject::owned( + POOL_ALREADY_IMPORTED, + "Transaction Already Imported", + Some(format!("{:?}", hash)), + )), + Error::Pool(PoolError::TooLowPriority { old, new }) => CallError::Custom(ErrorObject::owned( + POOL_TOO_LOW_PRIORITY, + format!("Priority is too low: ({} vs {})", old, new), + Some("The transaction has too low priority to replace another transaction already in the pool.") + )), + Error::Pool(PoolError::CycleDetected) => + CallError::Custom(ErrorObject::owned( + POOL_CYCLE_DETECTED, + "Cycle Detected", + None::<()> + )), + Error::Pool(PoolError::ImmediatelyDropped) => CallError::Custom(ErrorObject::owned( + POOL_IMMEDIATELY_DROPPED, + "Immediately Dropped", + Some("The transaction couldn't enter the pool because of the limit"), + )), + Error::Pool(PoolError::Unactionable) => CallError::Custom(ErrorObject::owned( + POOL_UNACTIONABLE, + "Unactionable", + Some("The transaction is unactionable since it is not propagable and \ + the local node does not author blocks") + )), + Error::Pool(PoolError::NoTagsProvided) => CallError::Custom(ErrorObject::owned( + POOL_NO_TAGS, + "No tags provided", + Some("Transaction does not provide any tags, so the pool can't identify it") + )), + Error::Pool(PoolError::InvalidBlockId(_)) => + CallError::Custom(ErrorObject::owned( + POOL_INVALID_BLOCK_ID, + "The provided block ID is not valid", + None::<()> + )), + Error::Pool(PoolError::RejectedFutureTransaction) => { + CallError::Custom(ErrorObject::owned( + POOL_FUTURE_TX, + "The pool is not accepting future transactions", + None::<()>, + )) }, Error::UnsafeRpcCalled(e) => e.into(), Error::Client(e) => CallError::Failed(anyhow::anyhow!(e)), Error::BadKeyType => CallError::InvalidParams(e.into()), Error::InvalidSessionKeys | Error::KeyStoreUnavailable => CallError::Failed(e.into()), - }.into() + } + .into() } } diff --git a/client/rpc-api/src/author/mod.rs b/client/rpc-api/src/author/mod.rs index 7ff498aca388f..feba7640e3b9f 100644 --- a/client/rpc-api/src/author/mod.rs +++ b/client/rpc-api/src/author/mod.rs @@ -74,5 +74,5 @@ pub trait AuthorApi { unsubscribe = "author_unwatchExtrinsic", item = TransactionStatus, )] - fn watch_extrinsic(&self, bytes: Bytes) -> RpcResult<()>; + fn watch_extrinsic(&self, bytes: Bytes); } diff --git a/client/rpc-api/src/chain/error.rs b/client/rpc-api/src/chain/error.rs index fd87e0e465881..30a7ad4e34c5b 100644 --- a/client/rpc-api/src/chain/error.rs +++ b/client/rpc-api/src/chain/error.rs @@ -18,7 +18,10 @@ //! Error helpers for Chain RPC module. -use jsonrpsee::{core::Error as JsonRpseeError, types::error::CallError}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; /// Chain RPC Result type. pub type Result = std::result::Result; @@ -39,8 +42,9 @@ const BASE_ERROR: i32 = 3000; impl From for JsonRpseeError { fn from(e: Error) -> Self { match e { - Error::Other(message) => - CallError::Custom { code: BASE_ERROR + 1, message, data: None }.into(), + Error::Other(message) => { + CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, message, None::<()>)).into() + }, e => e.into(), } } diff --git a/client/rpc-api/src/chain/mod.rs b/client/rpc-api/src/chain/mod.rs index dbd6a6eadb1db..f5f9524264e34 100644 --- a/client/rpc-api/src/chain/mod.rs +++ b/client/rpc-api/src/chain/mod.rs @@ -52,23 +52,25 @@ pub trait ChainApi { unsubscribe = "chain_unsubscribeAllHeads", item = Header )] - fn subscribe_all_heads(&self) -> RpcResult<()>; + fn subscribe_all_heads(&self); /// New head subscription. #[subscription( name = "chain_subscribeNewHeads" => "chain_newHead", aliases = ["subscribe_newHead", "chain_subscribeNewHead"], - unsubscribe = "chain_unsubscribeNewHead", + unsubscribe = "chain_unsubscribeNewHeads", + unsubscribe_aliases = ["unsubscribe_newHead", "chain_unsubscribeNewHead"], item = Header )] - fn subscribe_new_heads(&self) -> RpcResult<()>; + fn subscribe_new_heads(&self); /// Finalized head subscription. #[subscription( name = "chain_subscribeFinalizedHeads" => "chain_finalizedHead", aliases = ["chain_subscribeFinalisedHeads"], - unsubscribe = "chain_unsubscribeFinalisedHeads", + unsubscribe = "chain_unsubscribeFinalizedHeads", + unsubscribe_aliases = ["chain_unsubscribeFinalisedHeads"], item = Header )] - fn subscribe_finalized_heads(&self) -> RpcResult<()>; + fn subscribe_finalized_heads(&self); } diff --git a/client/rpc-api/src/dev/error.rs b/client/rpc-api/src/dev/error.rs index 81339575e8449..1d040bfc23934 100644 --- a/client/rpc-api/src/dev/error.rs +++ b/client/rpc-api/src/dev/error.rs @@ -18,7 +18,10 @@ //! Error helpers for Dev RPC module. -use jsonrpsee::{core::Error as JsonRpseeError, types::error::CallError}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; /// Dev RPC errors. #[derive(Debug, thiserror::Error)] @@ -42,13 +45,18 @@ const BASE_ERROR: i32 = 6000; impl From for JsonRpseeError { fn from(e: Error) -> Self { + let msg = e.to_string(); + match e { - Error::BlockQueryError(_) => - CallError::Custom { code: BASE_ERROR + 1, message: e.to_string(), data: None }, - Error::BlockExecutionFailed => - CallError::Custom { code: BASE_ERROR + 3, message: e.to_string(), data: None }, - Error::WitnessCompactionFailed => - CallError::Custom { code: BASE_ERROR + 4, message: e.to_string(), data: None }, + Error::BlockQueryError(_) => { + CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, msg, None::<()>)) + }, + Error::BlockExecutionFailed => { + CallError::Custom(ErrorObject::owned(BASE_ERROR + 3, msg, None::<()>)) + }, + Error::WitnessCompactionFailed => { + CallError::Custom(ErrorObject::owned(BASE_ERROR + 4, msg, None::<()>)) + }, Error::UnsafeRpcCalled(e) => e.into(), } .into() diff --git a/client/rpc-api/src/offchain/error.rs b/client/rpc-api/src/offchain/error.rs index 2be9721706739..7a7b7425bfd09 100644 --- a/client/rpc-api/src/offchain/error.rs +++ b/client/rpc-api/src/offchain/error.rs @@ -18,7 +18,7 @@ //! Offchain RPC errors. -use jsonrpsee::types::error::CallError; +use jsonrpsee::types::error::{CallError, ErrorObject}; /// Offchain RPC Result type. pub type Result = std::result::Result; @@ -40,11 +40,11 @@ const BASE_ERROR: i32 = 5000; impl From for CallError { fn from(e: Error) -> Self { match e { - Error::UnavailableStorageKind => Self::Custom { - code: BASE_ERROR + 1, - message: "This storage kind is not available yet".into(), - data: None, - }, + Error::UnavailableStorageKind => Self::Custom(ErrorObject::owned( + BASE_ERROR + 1, + "This storage kind is not available yet", + None::<()>, + )), Error::UnsafeRpcCalled(e) => e.into(), } } diff --git a/client/rpc-api/src/state/error.rs b/client/rpc-api/src/state/error.rs index 8970f305a3e76..423bf1147d749 100644 --- a/client/rpc-api/src/state/error.rs +++ b/client/rpc-api/src/state/error.rs @@ -18,7 +18,10 @@ //! State RPC errors. -use jsonrpsee::{core::Error as JsonRpseeError, types::error::CallError}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; /// State RPC Result type. pub type Result = std::result::Result; @@ -57,12 +60,14 @@ const BASE_ERROR: i32 = 4000; impl From for JsonRpseeError { fn from(e: Error) -> Self { match e { - Error::InvalidBlockRange { .. } => - CallError::Custom { code: BASE_ERROR + 1, message: e.to_string(), data: None } - .into(), - Error::InvalidCount { .. } => - CallError::Custom { code: BASE_ERROR + 2, message: e.to_string(), data: None } - .into(), + Error::InvalidBlockRange { .. } => { + CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, e.to_string(), None::<()>)) + .into() + }, + Error::InvalidCount { .. } => { + CallError::Custom(ErrorObject::owned(BASE_ERROR + 2, e.to_string(), None::<()>)) + .into() + }, e => e.into(), } } diff --git a/client/rpc-api/src/state/mod.rs b/client/rpc-api/src/state/mod.rs index 87b268effa4cc..fba023e830262 100644 --- a/client/rpc-api/src/state/mod.rs +++ b/client/rpc-api/src/state/mod.rs @@ -123,7 +123,7 @@ pub trait StateApi { unsubscribe_aliases = ["chain_unsubscribeRuntimeVersion"], item = RuntimeVersion, )] - fn subscribe_runtime_version(&self) -> RpcResult<()>; + fn subscribe_runtime_version(&self); /// New storage subscription #[subscription( @@ -131,7 +131,7 @@ pub trait StateApi { unsubscribe = "state_unsubscribeStorage", item = StorageChangeSet, )] - fn subscribe_storage(&self, keys: Option>) -> RpcResult<()>; + fn subscribe_storage(&self, keys: Option>); /// The `traceBlock` RPC provides a way to trace the re-execution of a single /// block, collecting Spans and Events from both the client and the relevant WASM runtime. diff --git a/client/rpc-api/src/system/error.rs b/client/rpc-api/src/system/error.rs index 3a16558cfe82e..8bf14607652c4 100644 --- a/client/rpc-api/src/system/error.rs +++ b/client/rpc-api/src/system/error.rs @@ -19,7 +19,7 @@ //! System RPC module errors. use crate::system::helpers::Health; -use jsonrpsee::{core::to_json_raw_value, types::error::CallError}; +use jsonrpsee::types::error::{CallError, ErrorObject}; /// System RPC Result type. pub type Result = std::result::Result; @@ -45,13 +45,12 @@ const MALFORMATTED_PEER_ARG_ERROR: i32 = BASE_ERROR + 2; impl From for CallError { fn from(e: Error) -> Self { match e { - Error::NotHealthy(ref h) => Self::Custom { - code: NOT_HEALTHY_ERROR, - message: e.to_string(), - data: to_json_raw_value(&h).ok(), + Error::NotHealthy(ref h) => { + Self::Custom(ErrorObject::owned(NOT_HEALTHY_ERROR, e.to_string(), Some(h))) + }, + Error::MalformattedPeerArg(e) => { + Self::Custom(ErrorObject::owned(MALFORMATTED_PEER_ARG_ERROR + 2, e, None::<()>)) }, - Error::MalformattedPeerArg(e) => - Self::Custom { code: MALFORMATTED_PEER_ARG_ERROR + 2, message: e, data: None }, } } } diff --git a/client/rpc/src/author/mod.rs b/client/rpc/src/author/mod.rs index 6f935d9581ecc..6972ed5d73fed 100644 --- a/client/rpc/src/author/mod.rs +++ b/client/rpc/src/author/mod.rs @@ -29,7 +29,7 @@ use codec::{Decode, Encode}; use futures::{task::Spawn, FutureExt}; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, - SubscriptionSink, + PendingSubscription, }; use sc_rpc_api::DenyUnsafe; use sc_transaction_pool_api::{ @@ -175,14 +175,15 @@ where .collect()) } - fn watch_extrinsic(&self, mut sink: SubscriptionSink, xt: Bytes) -> RpcResult<()> { + fn watch_extrinsic(&self, pending: PendingSubscription, xt: Bytes) { let best_block_hash = self.client.info().best_hash; let dxt = match TransactionFor::

::decode(&mut &xt[..]) { Ok(dxt) => dxt, Err(e) => { log::debug!("[author_watchExtrinsic] failed to decode extrinsic: {:?}", e); - let _ = sink.close_with_custom_message(&e.to_string()); - return Err(JsonRpseeError::to_call_error(e)) + let err = JsonRpseeError::to_call_error(e); + let _ = pending.reject(err); + return; }, }; @@ -194,8 +195,15 @@ where { Ok(stream) => stream, Err(err) => { - let _ = sink.close_with_custom_message(&err.to_string()); - return + let _ = pending.reject(JsonRpseeError::to_call_error(err)); + return; + }, + }; + + let mut sink = match pending.accept() { + Some(sink) => sink, + _ => { + return; }, }; @@ -203,8 +211,6 @@ where } .boxed(); - self.executor - .spawn_obj(fut.into()) - .map_err(|e| JsonRpseeError::to_call_error(e)) + let _ = self.executor.spawn_obj(fut.into()); } } diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index f016fb57be1fc..32f845b790f7d 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -22,10 +22,7 @@ use crate::testing::timeout_secs; use assert_matches::assert_matches; use codec::Encode; use jsonrpsee::{ - core::{ - error::{SubscriptionClosed, SubscriptionClosedReason}, - Error as RpcError, - }, + core::{Error as RpcError}, types::{error::CallError, EmptyParams}, RpcModule, }; @@ -107,7 +104,7 @@ async fn author_submit_transaction_should_not_cause_error() { assert_matches!( api.call::<_, H256>("author_submitExtrinsic", [xt]).await, - Err(RpcError::Call(CallError::Custom { message, ..})) if message.contains("Already imported") + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Already imported") ); } @@ -156,19 +153,14 @@ async fn author_should_return_watch_validation_error() { const METHOD: &'static str = "author_submitAndWatchExtrinsic"; let api = TestSetup::into_rpc(); - let mut sub = api + let failed_sub = api .subscribe(METHOD, [to_hex(&uxt(AccountKeyring::Alice, 179).encode(), true)]) - .await - .unwrap(); + .await; - let (pool_error, _) = timeout_secs(10, sub.next::()) - .await - .unwrap() - .unwrap() - .unwrap(); - assert_matches!(pool_error.close_reason(), SubscriptionClosedReason::Server(reason) => { - assert_eq!(reason, "Transaction pool error") - }); + assert_matches!( + failed_sub, + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Transaction pool error") + ); } #[tokio::test] @@ -287,7 +279,7 @@ async fn author_has_session_keys() { assert_matches!( api.call::<_, bool>("author_hasSessionKeys", vec![Bytes::from(vec![1, 2, 3])]).await, - Err(RpcError::Call(CallError::Custom { message, ..})) if message.as_str() == "Session keys are not encoded correctly" + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Session keys are not encoded correctly") ); } diff --git a/client/rpc/src/chain/chain_full.rs b/client/rpc/src/chain/chain_full.rs index 24c85113a27b6..cee3aa379a0ab 100644 --- a/client/rpc/src/chain/chain_full.rs +++ b/client/rpc/src/chain/chain_full.rs @@ -27,7 +27,7 @@ use futures::{ stream::{self, Stream, StreamExt}, task::Spawn, }; -use jsonrpsee::ws_server::SubscriptionSink; +use jsonrpsee::PendingSubscription; use sc_client_api::{BlockBackend, BlockchainEvents}; use sp_blockchain::HeaderBackend; use sp_runtime::{ @@ -71,7 +71,7 @@ where self.client.block(&BlockId::Hash(self.unwrap_or_best(hash))).map_err(client_err) } - fn subscribe_all_heads(&self, sink: SubscriptionSink) -> Result<(), Error> { + fn subscribe_all_heads(&self, sink: PendingSubscription) { subscribe_headers( &self.client, &self.executor, @@ -85,7 +85,7 @@ where ) } - fn subscribe_new_heads(&self, sink: SubscriptionSink) -> Result<(), Error> { + fn subscribe_new_heads(&self, sink: PendingSubscription) { subscribe_headers( &self.client, &self.executor, @@ -100,7 +100,7 @@ where ) } - fn subscribe_finalized_heads(&self, sink: SubscriptionSink) -> Result<(), Error> { + fn subscribe_finalized_heads(&self, sink: PendingSubscription) { subscribe_headers( &self.client, &self.executor, @@ -119,11 +119,10 @@ where fn subscribe_headers( client: &Arc, executor: &SubscriptionTaskExecutor, - sink: SubscriptionSink, + pending: PendingSubscription, best_block_hash: G, stream: F, -) -> Result<(), Error> -where +) where Block: BlockT + 'static, Block::Header: Unpin, Client: HeaderBackend + 'static, @@ -147,7 +146,12 @@ where // we set up the stream and chain it to the stream. Consuming code would need to handle // duplicates at the beginning of the stream though. let stream = stream::iter(maybe_header).chain(stream()); - let fut = sink.pipe_from_stream(stream).map(|_| ()).boxed(); - executor.spawn_obj(fut.into()).map_err(|e| Error::Client(Box::new(e))) + let fut = async move { + if let Some(mut sink) = pending.accept() { + sink.pipe_from_stream(stream).await; + } + } + .boxed(); + let _ = executor.spawn_obj(fut.into()); } diff --git a/client/rpc/src/chain/mod.rs b/client/rpc/src/chain/mod.rs index 3efed92c8fa94..275af6958ba11 100644 --- a/client/rpc/src/chain/mod.rs +++ b/client/rpc/src/chain/mod.rs @@ -29,7 +29,7 @@ use crate::SubscriptionTaskExecutor; use jsonrpsee::{ core::{async_trait, RpcResult}, - SubscriptionSink, + PendingSubscription, }; use sc_client_api::BlockchainEvents; use sp_rpc::{list::ListOrValue, number::NumberOrHex}; @@ -99,13 +99,13 @@ where } /// All new head subscription - fn subscribe_all_heads(&self, sink: SubscriptionSink) -> Result<(), Error>; + fn subscribe_all_heads(&self, sink: PendingSubscription); /// New best head subscription - fn subscribe_new_heads(&self, sink: SubscriptionSink) -> Result<(), Error>; + fn subscribe_new_heads(&self, sink: PendingSubscription); /// Finalized head subscription - fn subscribe_finalized_heads(&self, sink: SubscriptionSink) -> Result<(), Error>; + fn subscribe_finalized_heads(&self, sink: PendingSubscription); } /// Create new state API that works on full node. @@ -165,16 +165,16 @@ where self.backend.finalized_head().map_err(Into::into) } - fn subscribe_all_heads(&self, sink: SubscriptionSink) -> RpcResult<()> { - self.backend.subscribe_all_heads(sink).map_err(Into::into) + fn subscribe_all_heads(&self, sink: PendingSubscription) { + self.backend.subscribe_all_heads(sink) } - fn subscribe_new_heads(&self, sink: SubscriptionSink) -> RpcResult<()> { - self.backend.subscribe_new_heads(sink).map_err(Into::into) + fn subscribe_new_heads(&self, sink: PendingSubscription) { + self.backend.subscribe_new_heads(sink) } - fn subscribe_finalized_heads(&self, sink: SubscriptionSink) -> RpcResult<()> { - self.backend.subscribe_finalized_heads(sink).map_err(Into::into) + fn subscribe_finalized_heads(&self, sink: PendingSubscription) { + self.backend.subscribe_finalized_heads(sink) } } diff --git a/client/rpc/src/chain/tests.rs b/client/rpc/src/chain/tests.rs index a0e2831896f04..6ba4983024777 100644 --- a/client/rpc/src/chain/tests.rs +++ b/client/rpc/src/chain/tests.rs @@ -19,7 +19,7 @@ use super::*; use crate::testing::timeout_secs; use assert_matches::assert_matches; -use jsonrpsee::{core::error::SubscriptionClosed, types::EmptyParams}; +use jsonrpsee::types::EmptyParams; use sc_block_builder::BlockBuilderProvider; use sp_consensus::BlockOrigin; use sp_rpc::list::ListOrValue; @@ -241,5 +241,5 @@ async fn test_head_subscription(method: &str) { assert_matches!(timeout_secs(10, sub.next::

()).await, Ok(Some(_))); sub.close(); - assert_matches!(timeout_secs(10, sub.next::()).await, Ok(Some(_))) + assert_matches!(timeout_secs(10, sub.next::
()).await, Ok(None)); } diff --git a/client/rpc/src/dev/tests.rs b/client/rpc/src/dev/tests.rs index 4dae4ca2b43e4..b7a0de8f5ae0b 100644 --- a/client/rpc/src/dev/tests.rs +++ b/client/rpc/src/dev/tests.rs @@ -64,6 +64,6 @@ async fn deny_unsafe_works() { assert_matches!( api.call::<_, Option>("dev_getBlockStats", [client.info().best_hash]) .await, - Err(JsonRpseeError::Call(CallError::Custom { message, .. })) if message.as_str() == "RPC call is unsafe to be called externally" + Err(JsonRpseeError::Call(CallError::Custom(err))) if err.message().contains("RPC call is unsafe to be called externally") ); } diff --git a/client/rpc/src/state/mod.rs b/client/rpc/src/state/mod.rs index 21e1ae326ff24..01f7ebebbfa4b 100644 --- a/client/rpc/src/state/mod.rs +++ b/client/rpc/src/state/mod.rs @@ -29,7 +29,7 @@ use crate::SubscriptionTaskExecutor; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, - ws_server::SubscriptionSink, + ws_server::PendingSubscription, }; use sc_rpc_api::{state::ReadProof, DenyUnsafe}; @@ -156,14 +156,10 @@ where ) -> Result; /// New runtime version subscription - fn subscribe_runtime_version(&self, sink: SubscriptionSink) -> Result<(), Error>; + fn subscribe_runtime_version(&self, sink: PendingSubscription); /// New storage subscription - fn subscribe_storage( - &self, - sink: SubscriptionSink, - keys: Option>, - ) -> Result<(), Error>; + fn subscribe_storage(&self, sink: PendingSubscription, keys: Option>); } /// Create new state API that works on full node. @@ -259,7 +255,7 @@ where return Err(JsonRpseeError::to_call_error(Error::InvalidCount { value: count, max: STORAGE_KEYS_PAGED_MAX_COUNT, - })) + })); } self.backend .storage_keys_paged(block, prefix, count, start_key) @@ -365,20 +361,12 @@ where .map_err(|e| JsonRpseeError::to_call_error(e)) } - fn subscribe_runtime_version(&self, sink: SubscriptionSink) -> RpcResult<()> { - self.backend - .subscribe_runtime_version(sink) - .map_err(|e| JsonRpseeError::to_call_error(e)) + fn subscribe_runtime_version(&self, sink: PendingSubscription) { + self.backend.subscribe_runtime_version(sink) } - fn subscribe_storage( - &self, - sink: SubscriptionSink, - keys: Option>, - ) -> RpcResult<()> { - self.backend - .subscribe_storage(sink, keys) - .map_err(|e| JsonRpseeError::to_call_error(e)) + fn subscribe_storage(&self, sink: PendingSubscription, keys: Option>) { + self.backend.subscribe_storage(sink, keys) } } diff --git a/client/rpc/src/state/state_full.rs b/client/rpc/src/state/state_full.rs index 8775dcbe2b97c..39c6752fc2279 100644 --- a/client/rpc/src/state/state_full.rs +++ b/client/rpc/src/state/state_full.rs @@ -28,7 +28,7 @@ use super::{ use crate::SubscriptionTaskExecutor; use futures::{future, stream, task::Spawn, FutureExt, StreamExt}; -use jsonrpsee::SubscriptionSink; +use jsonrpsee::{core::Error as JsonRpseeError, PendingSubscription}; use sc_client_api::{ Backend, BlockBackend, BlockchainEvents, CallExecutor, ExecutorProvider, ProofProvider, StorageProvider, @@ -105,7 +105,7 @@ where &from_meta, &to_meta, "from number > to number".to_owned(), - )) + )); } // check if we can get from `to` to `from` by going through parent_hashes. @@ -126,7 +126,7 @@ where &from_meta, &to_meta, "from and to are on different forks".to_owned(), - )) + )); } hashes.reverse(); hashes @@ -360,18 +360,21 @@ where .map_err(client_err) } - fn subscribe_runtime_version(&self, mut sink: SubscriptionSink) -> std::result::Result<(), Error> { + fn subscribe_runtime_version(&self, pending: PendingSubscription) { let client = self.client.clone(); - let initial = self - .block_or_best(None) - .and_then(|block| { + let res: std::result::Result = + self.block_or_best(None).and_then(|block| { self.client.runtime_version_at(&BlockId::Hash(block)).map_err(Into::into) - }) - .map_err(|e| { - sink.close_with_custom_message(&e.to_string()); - Error::Client(Box::new(e)) - })?; + }); + + let initial = match res { + Ok(i) => i, + Err(e) => { + let _ = pending.reject(JsonRpseeError::to_call_error(e)); + return; + }, + }; let mut previous_version = initial.clone(); @@ -394,23 +397,25 @@ where }); let stream = futures::stream::once(future::ready(initial)).chain(version_stream); - let fut = sink.pipe_from_stream(stream).map(|_| ()).boxed(); - self.executor.spawn_obj(fut.into()).map_err(|e| Error::Client(Box::new(e))) + let fut = async move { + if let Some(mut sink) = pending.accept() { + sink.pipe_from_stream(stream).await; + } + } + .boxed(); + + let _ = self.executor.spawn_obj(fut.into()); } - fn subscribe_storage( - &self, - mut sink: SubscriptionSink, - keys: Option>, - ) -> std::result::Result<(), Error> { - let stream = self - .client - .storage_changes_notification_stream(keys.as_deref(), None) - .map_err(|blockchain_err| { - sink.close_with_custom_message(&blockchain_err.to_string()); - Error::Client(Box::new(blockchain_err)) - })?; + fn subscribe_storage(&self, pending: PendingSubscription, keys: Option>) { + let stream = match self.client.storage_changes_notification_stream(keys.as_deref(), None) { + Ok(stream) => stream, + Err(blockchain_err) => { + let _ = pending.reject(JsonRpseeError::to_call_error(blockchain_err)); + return; + }, + }; // initial values let initial = stream::iter(keys.map(|keys| { @@ -439,8 +444,13 @@ where .chain(storage_stream) .filter(|storage| future::ready(!storage.changes.is_empty())); - let fut = sink.pipe_from_stream(stream).map(|_| ()).boxed(); - self.executor.spawn_obj(fut.into()).map_err(|e| Error::Client(Box::new(e))) + let fut = async move { + if let Some(mut sink) = pending.accept() { + sink.pipe_from_stream(stream).await; + } + } + .boxed(); + let _ = self.executor.spawn_obj(fut.into()); } async fn trace_block( @@ -491,8 +501,9 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => - ChildInfo::new_default(storage_key), + Some((ChildType::ParentKeyId, storage_key)) => { + ChildInfo::new_default(storage_key) + }, None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client @@ -516,8 +527,9 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => - ChildInfo::new_default(storage_key), + Some((ChildType::ParentKeyId, storage_key)) => { + ChildInfo::new_default(storage_key) + }, None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage_keys(&BlockId::Hash(block), &child_info, &prefix) @@ -536,8 +548,9 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => - ChildInfo::new_default(storage_key), + Some((ChildType::ParentKeyId, storage_key)) => { + ChildInfo::new_default(storage_key) + }, None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage_keys_iter( @@ -560,8 +573,9 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => - ChildInfo::new_default(storage_key), + Some((ChildType::ParentKeyId, storage_key)) => { + ChildInfo::new_default(storage_key) + }, None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage(&BlockId::Hash(block), &child_info, &key) @@ -580,7 +594,7 @@ where { Arc::new(ChildInfo::new_default(storage_key)) } else { - return Err(client_err(sp_blockchain::Error::InvalidChildStorageKey)) + return Err(client_err(sp_blockchain::Error::InvalidChildStorageKey)); }; let block = self.block_or_best(block).map_err(client_err)?; let client = self.client.clone(); @@ -604,8 +618,9 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => - ChildInfo::new_default(storage_key), + Some((ChildType::ParentKeyId, storage_key)) => { + ChildInfo::new_default(storage_key) + }, None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage_hash(&BlockId::Hash(block), &child_info, &key) diff --git a/client/rpc/src/state/tests.rs b/client/rpc/src/state/tests.rs index 086c9674c3ef4..37866f3160ddc 100644 --- a/client/rpc/src/state/tests.rs +++ b/client/rpc/src/state/tests.rs @@ -22,7 +22,7 @@ use crate::testing::timeout_secs; use assert_matches::assert_matches; use futures::executor; use jsonrpsee::{ - core::{error::SubscriptionClosed, Error as RpcError}, + core::Error as RpcError, types::{error::CallError as RpcCallError, EmptyParams}, }; use sc_block_builder::BlockBuilderProvider; @@ -257,8 +257,8 @@ async fn should_notify_about_storage_changes() { // We should get a message back on our subscription about the storage change: // NOTE: previous versions of the subscription code used to return an empty value for the // "initial" storage change here - assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(_)); - assert_matches!(timeout_secs(1, sub.next::()).await, Ok(_)); + assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(Some(_))); + assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(None)); } #[tokio::test] @@ -292,11 +292,11 @@ async fn should_send_initial_storage_changes_and_notifications() { sub }; - assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(_)); - assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(_)); + assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(Some(_))); + assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(Some(_))); // No more messages to follow - assert_matches!(timeout_secs(1, sub.next::()).await, Ok(_)); + assert_matches!(timeout_secs(1, sub.next::>()).await, Ok(None)); } #[tokio::test] @@ -533,7 +533,7 @@ async fn should_notify_on_runtime_version_initially() { assert_matches!(timeout_secs(10, sub.next::()).await, Ok(Some(_))); sub.close(); - assert_matches!(timeout_secs(10, sub.next::()).await, Ok(_)); + assert_matches!(timeout_secs(10, sub.next::()).await, Ok(None)); } #[test] diff --git a/client/rpc/src/system/tests.rs b/client/rpc/src/system/tests.rs index 3ccb85d1ac748..5c630c3eecc00 100644 --- a/client/rpc/src/system/tests.rs +++ b/client/rpc/src/system/tests.rs @@ -101,15 +101,17 @@ fn api>>(sync: T) -> RpcModule> { Request::NetworkAddReservedPeer(peer, sender) => { let _ = match sc_network::config::parse_str_addr(&peer) { Ok(_) => sender.send(Ok(())), - Err(s) => - sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))), + Err(s) => { + sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))) + }, }; }, Request::NetworkRemoveReservedPeer(peer, sender) => { let _ = match peer.parse::() { Ok(_) => sender.send(Ok(())), - Err(s) => - sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))), + Err(s) => { + sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))) + }, }; }, Request::NetworkReservedPeers(sender) => { @@ -315,7 +317,7 @@ async fn system_network_add_reserved() { let bad_peer_id = ["/ip4/198.51.100.19/tcp/30333"]; assert_matches!( api(None).call::<_, ()>("system_addReservedPeer", bad_peer_id).await, - Err(RpcError::Call(CallError::Custom { message, .. })) if message.as_str() == "Peer id is missing from the address" + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Peer id is missing from the address") ); } @@ -331,7 +333,7 @@ async fn system_network_remove_reserved() { assert_matches!( api(None).call::<_, String>("system_removeReservedPeer", bad_peer_id).await, - Err(RpcError::Call(CallError::Custom { message, .. })) if message.as_str() == "base-58 decode error: provided string contained invalid character '/' at byte 0" + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("base-58 decode error: provided string contained invalid character '/' at byte 0") ); } #[tokio::test] @@ -371,7 +373,7 @@ fn test_add_reset_log_filter() { }; futures::executor::block_on(fut).expect("`system_resetLogFilter` failed"); } else if line.contains("exit") { - return + return; } log::trace!(target: "test_before_add", "{}", EXPECTED_WITH_TRACE); log::debug!(target: "test_before_add", "{}", EXPECTED_BEFORE_ADD); diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index d79e8d718095e..e7387d54ce835 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -24,9 +24,9 @@ use std::{marker::PhantomData, sync::Arc}; use anyhow::anyhow; use codec::Codec; use jsonrpsee::{ - core::{async_trait, to_json_raw_value, Error as JsonRpseeError, RpcResult}, + core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, - types::error::CallError, + types::error::{CallError, ErrorObject}, }; use pallet_contracts_primitives::{ Code, CodeUploadResult, ContractExecResult, ContractInstantiateResult, @@ -69,11 +69,11 @@ impl From for JsonRpseeError { fn from(e: ContractAccessError) -> Self { use pallet_contracts_primitives::ContractAccessError::*; match e.0 { - DoesntExist => CallError::Custom { - code: CONTRACT_DOESNT_EXIST, - message: "The specified contract doesn't exist.".into(), - data: None, - } + DoesntExist => CallError::Custom(ErrorObject::owned( + CONTRACT_DOESNT_EXIST, + "The specified contract doesn't exist.", + None::<()>, + )) .into(), } } @@ -310,11 +310,11 @@ where /// Converts a runtime trap into an RPC error. fn runtime_error_into_rpc_err(err: impl std::fmt::Debug) -> JsonRpseeError { - CallError::Custom { - code: RUNTIME_ERROR, - message: "Runtime error".into(), - data: to_json_raw_value(&format!("{:?}", err)).ok(), - } + CallError::Custom(ErrorObject::owned( + RUNTIME_ERROR, + "Runtime error", + Some(format!("{:?}", err)), + )) .into() } diff --git a/frame/merkle-mountain-range/rpc/Cargo.toml b/frame/merkle-mountain-range/rpc/Cargo.toml index 10b237e86fb17..949d1804a48d0 100644 --- a/frame/merkle-mountain-range/rpc/Cargo.toml +++ b/frame/merkle-mountain-range/rpc/Cargo.toml @@ -16,7 +16,6 @@ targets = ["x86_64-unknown-linux-gnu"] codec = { package = "parity-scale-codec", version = "3.0.0" } jsonrpsee = { version = "0.10.1", features = ["server", "macros"] } serde = { version = "1.0.136", features = ["derive"] } -serde_json = "1" sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } sp-core = { version = "6.0.0", path = "../../../primitives/core" } diff --git a/frame/merkle-mountain-range/rpc/src/lib.rs b/frame/merkle-mountain-range/rpc/src/lib.rs index c8ed11e7709bd..5d86aa3aa5dbc 100644 --- a/frame/merkle-mountain-range/rpc/src/lib.rs +++ b/frame/merkle-mountain-range/rpc/src/lib.rs @@ -23,10 +23,13 @@ use std::{marker::PhantomData, sync::Arc}; use codec::{Codec, Encode}; -use jsonrpsee::{core::async_trait, proc_macros::rpc, types::error::CallError}; +use jsonrpsee::{ + core::async_trait, + proc_macros::rpc, + types::error::{CallError, ErrorObject}, +}; use pallet_mmr_primitives::{Error as MmrError, Proof}; use serde::{Deserialize, Serialize}; -use serde_json::value::to_raw_value; use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; @@ -129,29 +132,29 @@ where /// Converts a mmr-specific error into a [`CallError`]. fn mmr_error_into_rpc_error(err: MmrError) -> CallError { - let data = to_raw_value(&format!("{:?}", err)).ok(); + let data = format!("{:?}", err); match err { - MmrError::LeafNotFound => CallError::Custom { - code: LEAF_NOT_FOUND_ERROR, - message: "Leaf was not found".into(), - data, - }, - MmrError::GenerateProof => CallError::Custom { - code: GENERATE_PROOF_ERROR, - message: "Error while generating the proof".into(), - data, - }, - _ => CallError::Custom { code: MMR_ERROR, message: "Unexpected MMR error".into(), data }, + MmrError::LeafNotFound => CallError::Custom(ErrorObject::owned( + LEAF_NOT_FOUND_ERROR, + "Leaf was not found", + Some(data), + )), + MmrError::GenerateProof => CallError::Custom(ErrorObject::owned( + GENERATE_PROOF_ERROR, + "Error while generating the proof", + Some(data), + )), + _ => CallError::Custom(ErrorObject::owned(MMR_ERROR, "Unexpected MMR error", Some(data))), } } /// Converts a runtime trap into a [`CallError`]. fn runtime_error_into_rpc_error(err: impl std::fmt::Debug) -> CallError { - CallError::Custom { - code: RUNTIME_ERROR, - message: "Runtime trapped".into(), - data: to_raw_value(&format!("{:?}", err)).ok(), - } + CallError::Custom(ErrorObject::owned( + RUNTIME_ERROR, + "Runtime trapped", + Some(format!("{:?}", err)), + )) } #[cfg(test)] diff --git a/utils/frame/rpc/system/src/lib.rs b/utils/frame/rpc/system/src/lib.rs index 3c39add2f07cf..271b0fb072282 100644 --- a/utils/frame/rpc/system/src/lib.rs +++ b/utils/frame/rpc/system/src/lib.rs @@ -23,7 +23,7 @@ use codec::{self, Codec, Decode, Encode}; use jsonrpsee::{ core::{async_trait, RpcResult}, proc_macros::rpc, - types::error::CallError, + types::error::{CallError, ErrorObject}, }; use sc_rpc_api::DenyUnsafe; @@ -120,43 +120,49 @@ where self.client.info().best_hash)); let uxt: ::Extrinsic = - Decode::decode(&mut &*extrinsic).map_err(|e| CallError::Custom { - code: Error::DecodeError.into(), - message: "Unable to dry run extrinsic.".into(), - data: serde_json::value::to_raw_value(&e.to_string()).ok(), + Decode::decode(&mut &*extrinsic).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::DecodeError.into(), + "Unable to dry run extrinsic", + Some(e.to_string()), + )) })?; let api_version = api .api_version::>(&at) - .map_err(|e| CallError::Custom { - code: Error::RuntimeError.into(), - message: "Unable to dry run extrinsic.".into(), - data: serde_json::value::to_raw_value(&e.to_string()).ok(), + .map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to dry run extrinsic.", + Some(e.to_string()), + )) })? - .ok_or_else(|| CallError::Custom { - code: Error::RuntimeError.into(), - message: "Unable to dry run extrinsic.".into(), - data: serde_json::value::to_raw_value(&format!( - "Could not find `BlockBuilder` api for block `{:?}`.", - at + .ok_or_else(|| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to dry run extrinsic.", + Some(format!("Could not find `BlockBuilder` api for block `{:?}`.", at)), )) - .ok(), })?; let result = if api_version < 6 { #[allow(deprecated)] api.apply_extrinsic_before_version_6(&at, uxt) .map(legacy::byte_sized_error::convert_to_latest) - .map_err(|e| CallError::Custom { - code: Error::RuntimeError.into(), - message: "Unable to dry run extrinsic.".into(), - data: serde_json::value::to_raw_value(&e.to_string()).ok(), + .map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to dry run extrinsic.", + Some(e.to_string()), + )) })? } else { - api.apply_extrinsic(&at, uxt).map_err(|e| CallError::Custom { - code: Error::RuntimeError.into(), - message: "Unable to dry run extrinsic.".into(), - data: serde_json::value::to_raw_value(&e.to_string()).ok(), + api.apply_extrinsic(&at, uxt).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to dry run extrinsic.", + Some(e.to_string()), + )) })? }; From a504c14ef686458c93f77293e17902ebe08a9600 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 20 Apr 2022 18:37:09 +0200 Subject: [PATCH 2/3] fix error responses --- client/beefy/rpc/src/lib.rs | 2 +- client/rpc-api/src/chain/error.rs | 5 +- client/rpc-api/src/dev/error.rs | 15 +++--- client/rpc-api/src/state/error.rs | 10 ++-- client/rpc-api/src/system/error.rs | 10 ++-- client/rpc/src/author/mod.rs | 35 ++++++------ client/rpc/src/author/tests.rs | 2 +- client/rpc/src/state/mod.rs | 2 +- client/rpc/src/state/state_full.rs | 54 +++++++++---------- client/rpc/src/system/tests.rs | 12 ++--- frame/contracts/rpc/src/lib.rs | 4 +- frame/transaction-payment/rpc/src/lib.rs | 2 +- .../rpc/state-trie-migration-rpc/src/lib.rs | 11 ++-- 13 files changed, 75 insertions(+), 89 deletions(-) diff --git a/client/beefy/rpc/src/lib.rs b/client/beefy/rpc/src/lib.rs index 1d90e24ac0b8c..ca5eb825b78ad 100644 --- a/client/beefy/rpc/src/lib.rs +++ b/client/beefy/rpc/src/lib.rs @@ -208,7 +208,7 @@ mod tests { if response != not_ready { assert_eq!(response, expected); // Success - return; + return } std::thread::sleep(std::time::Duration::from_millis(50)) } diff --git a/client/rpc-api/src/chain/error.rs b/client/rpc-api/src/chain/error.rs index 30a7ad4e34c5b..670e221cf1cde 100644 --- a/client/rpc-api/src/chain/error.rs +++ b/client/rpc-api/src/chain/error.rs @@ -42,9 +42,8 @@ const BASE_ERROR: i32 = 3000; impl From for JsonRpseeError { fn from(e: Error) -> Self { match e { - Error::Other(message) => { - CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, message, None::<()>)).into() - }, + Error::Other(message) => + CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, message, None::<()>)).into(), e => e.into(), } } diff --git a/client/rpc-api/src/dev/error.rs b/client/rpc-api/src/dev/error.rs index 1d040bfc23934..fe74dea256376 100644 --- a/client/rpc-api/src/dev/error.rs +++ b/client/rpc-api/src/dev/error.rs @@ -48,15 +48,12 @@ impl From for JsonRpseeError { let msg = e.to_string(); match e { - Error::BlockQueryError(_) => { - CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, msg, None::<()>)) - }, - Error::BlockExecutionFailed => { - CallError::Custom(ErrorObject::owned(BASE_ERROR + 3, msg, None::<()>)) - }, - Error::WitnessCompactionFailed => { - CallError::Custom(ErrorObject::owned(BASE_ERROR + 4, msg, None::<()>)) - }, + Error::BlockQueryError(_) => + CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, msg, None::<()>)), + Error::BlockExecutionFailed => + CallError::Custom(ErrorObject::owned(BASE_ERROR + 3, msg, None::<()>)), + Error::WitnessCompactionFailed => + CallError::Custom(ErrorObject::owned(BASE_ERROR + 4, msg, None::<()>)), Error::UnsafeRpcCalled(e) => e.into(), } .into() diff --git a/client/rpc-api/src/state/error.rs b/client/rpc-api/src/state/error.rs index 423bf1147d749..e720db5c57d02 100644 --- a/client/rpc-api/src/state/error.rs +++ b/client/rpc-api/src/state/error.rs @@ -60,14 +60,12 @@ const BASE_ERROR: i32 = 4000; impl From for JsonRpseeError { fn from(e: Error) -> Self { match e { - Error::InvalidBlockRange { .. } => { + Error::InvalidBlockRange { .. } => CallError::Custom(ErrorObject::owned(BASE_ERROR + 1, e.to_string(), None::<()>)) - .into() - }, - Error::InvalidCount { .. } => { + .into(), + Error::InvalidCount { .. } => CallError::Custom(ErrorObject::owned(BASE_ERROR + 2, e.to_string(), None::<()>)) - .into() - }, + .into(), e => e.into(), } } diff --git a/client/rpc-api/src/system/error.rs b/client/rpc-api/src/system/error.rs index 8bf14607652c4..2fae3587e8418 100644 --- a/client/rpc-api/src/system/error.rs +++ b/client/rpc-api/src/system/error.rs @@ -45,12 +45,10 @@ const MALFORMATTED_PEER_ARG_ERROR: i32 = BASE_ERROR + 2; impl From for CallError { fn from(e: Error) -> Self { match e { - Error::NotHealthy(ref h) => { - Self::Custom(ErrorObject::owned(NOT_HEALTHY_ERROR, e.to_string(), Some(h))) - }, - Error::MalformattedPeerArg(e) => { - Self::Custom(ErrorObject::owned(MALFORMATTED_PEER_ARG_ERROR + 2, e, None::<()>)) - }, + Error::NotHealthy(ref h) => + Self::Custom(ErrorObject::owned(NOT_HEALTHY_ERROR, e.to_string(), Some(h))), + Error::MalformattedPeerArg(e) => + Self::Custom(ErrorObject::owned(MALFORMATTED_PEER_ARG_ERROR + 2, e, None::<()>)), } } } diff --git a/client/rpc/src/author/mod.rs b/client/rpc/src/author/mod.rs index be44cabd43487..5db388cac69e3 100644 --- a/client/rpc/src/author/mod.rs +++ b/client/rpc/src/author/mod.rs @@ -26,7 +26,7 @@ use std::sync::Arc; use crate::SubscriptionTaskExecutor; use codec::{Decode, Encode}; -use futures::FutureExt; +use futures::{FutureExt, TryFutureExt}; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, PendingSubscription, @@ -177,37 +177,38 @@ where fn watch_extrinsic(&self, pending: PendingSubscription, xt: Bytes) { let best_block_hash = self.client.info().best_hash; - let dxt = match TransactionFor::

::decode(&mut &xt[..]) { + let dxt = match TransactionFor::

::decode(&mut &xt[..]).map_err(|e| Error::from(e)) { Ok(dxt) => dxt, Err(e) => { - log::debug!("[author_watchExtrinsic] failed to decode extrinsic: {:?}", e); - let err = JsonRpseeError::to_call_error(e); - pending.reject(err); - return; + pending.reject(JsonRpseeError::from(e)); + return }, }; - let pool = self.pool.clone(); + let submit = self + .pool + .submit_and_watch(&generic::BlockId::hash(best_block_hash), TX_SOURCE, dxt) + .map_err(|e| { + e.into_pool_error() + .map(error::Error::from) + .unwrap_or_else(|e| error::Error::Verification(Box::new(e)).into()) + }); + let fut = async move { - let stream = match pool - .submit_and_watch(&generic::BlockId::hash(best_block_hash), TX_SOURCE, dxt) - .await - { + let stream = match submit.await { Ok(stream) => stream, Err(err) => { - let _ = pending.reject(JsonRpseeError::to_call_error(err)); - return; + pending.reject(JsonRpseeError::from(err)); + return }, }; let mut sink = match pending.accept() { Some(sink) => sink, - _ => { - return; - }, + _ => return, }; - let _ = sink.pipe_from_stream(stream).await; + sink.pipe_from_stream(stream).await; } .boxed(); diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index e5ddc676244df..799e2d5ef42bb 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -22,7 +22,7 @@ use crate::testing::{test_executor, timeout_secs}; use assert_matches::assert_matches; use codec::Encode; use jsonrpsee::{ - core::{Error as RpcError}, + core::Error as RpcError, types::{error::CallError, EmptyParams}, RpcModule, }; diff --git a/client/rpc/src/state/mod.rs b/client/rpc/src/state/mod.rs index 01f7ebebbfa4b..1fdbe0433b476 100644 --- a/client/rpc/src/state/mod.rs +++ b/client/rpc/src/state/mod.rs @@ -255,7 +255,7 @@ where return Err(JsonRpseeError::to_call_error(Error::InvalidCount { value: count, max: STORAGE_KEYS_PAGED_MAX_COUNT, - })); + })) } self.backend .storage_keys_paged(block, prefix, count, start_key) diff --git a/client/rpc/src/state/state_full.rs b/client/rpc/src/state/state_full.rs index 3756e36fb5955..bd0fe9fff9b96 100644 --- a/client/rpc/src/state/state_full.rs +++ b/client/rpc/src/state/state_full.rs @@ -28,7 +28,7 @@ use super::{ use crate::SubscriptionTaskExecutor; use futures::{future, stream, FutureExt, StreamExt}; -use jsonrpsee::{PendingSubscription, core::Error as JsonRpseeError}; +use jsonrpsee::{core::Error as JsonRpseeError, PendingSubscription}; use sc_client_api::{ Backend, BlockBackend, BlockchainEvents, CallExecutor, ExecutorProvider, ProofProvider, StorageProvider, @@ -105,7 +105,7 @@ where &from_meta, &to_meta, "from number > to number".to_owned(), - )); + )) } // check if we can get from `to` to `from` by going through parent_hashes. @@ -126,7 +126,7 @@ where &from_meta, &to_meta, "from and to are on different forks".to_owned(), - )); + )) } hashes.reverse(); hashes @@ -363,16 +363,17 @@ where fn subscribe_runtime_version(&self, pending: PendingSubscription) { let client = self.client.clone(); - let res: std::result::Result = - self.block_or_best(None).and_then(|block| { + let initial = match self + .block_or_best(None) + .and_then(|block| { self.client.runtime_version_at(&BlockId::Hash(block)).map_err(Into::into) - }); - - let initial = match res { - Ok(i) => i, + }) + .map_err(|e| Error::Client(Box::new(e))) + { + Ok(initial) => initial, Err(e) => { - let _ = pending.reject(JsonRpseeError::to_call_error(e)); - return; + pending.reject(JsonRpseeError::from(e)); + return }, }; @@ -413,8 +414,8 @@ where let stream = match self.client.storage_changes_notification_stream(keys.as_deref(), None) { Ok(stream) => stream, Err(blockchain_err) => { - let _ = pending.reject(JsonRpseeError::to_call_error(blockchain_err)); - return; + pending.reject(JsonRpseeError::from(Error::Client(Box::new(blockchain_err)))); + return }, }; @@ -504,9 +505,8 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => { - ChildInfo::new_default(storage_key) - }, + Some((ChildType::ParentKeyId, storage_key)) => + ChildInfo::new_default(storage_key), None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client @@ -530,9 +530,8 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => { - ChildInfo::new_default(storage_key) - }, + Some((ChildType::ParentKeyId, storage_key)) => + ChildInfo::new_default(storage_key), None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage_keys(&BlockId::Hash(block), &child_info, &prefix) @@ -551,9 +550,8 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => { - ChildInfo::new_default(storage_key) - }, + Some((ChildType::ParentKeyId, storage_key)) => + ChildInfo::new_default(storage_key), None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage_keys_iter( @@ -576,9 +574,8 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => { - ChildInfo::new_default(storage_key) - }, + Some((ChildType::ParentKeyId, storage_key)) => + ChildInfo::new_default(storage_key), None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage(&BlockId::Hash(block), &child_info, &key) @@ -597,7 +594,7 @@ where { Arc::new(ChildInfo::new_default(storage_key)) } else { - return Err(client_err(sp_blockchain::Error::InvalidChildStorageKey)); + return Err(client_err(sp_blockchain::Error::InvalidChildStorageKey)) }; let block = self.block_or_best(block).map_err(client_err)?; let client = self.client.clone(); @@ -621,9 +618,8 @@ where self.block_or_best(block) .and_then(|block| { let child_info = match ChildType::from_prefixed_key(&storage_key) { - Some((ChildType::ParentKeyId, storage_key)) => { - ChildInfo::new_default(storage_key) - }, + Some((ChildType::ParentKeyId, storage_key)) => + ChildInfo::new_default(storage_key), None => return Err(sp_blockchain::Error::InvalidChildStorageKey), }; self.client.child_storage_hash(&BlockId::Hash(block), &child_info, &key) diff --git a/client/rpc/src/system/tests.rs b/client/rpc/src/system/tests.rs index 5c630c3eecc00..77acdf8418ccc 100644 --- a/client/rpc/src/system/tests.rs +++ b/client/rpc/src/system/tests.rs @@ -101,17 +101,15 @@ fn api>>(sync: T) -> RpcModule> { Request::NetworkAddReservedPeer(peer, sender) => { let _ = match sc_network::config::parse_str_addr(&peer) { Ok(_) => sender.send(Ok(())), - Err(s) => { - sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))) - }, + Err(s) => + sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))), }; }, Request::NetworkRemoveReservedPeer(peer, sender) => { let _ = match peer.parse::() { Ok(_) => sender.send(Ok(())), - Err(s) => { - sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))) - }, + Err(s) => + sender.send(Err(error::Error::MalformattedPeerArg(s.to_string()))), }; }, Request::NetworkReservedPeers(sender) => { @@ -373,7 +371,7 @@ fn test_add_reset_log_filter() { }; futures::executor::block_on(fut).expect("`system_resetLogFilter` failed"); } else if line.contains("exit") { - return; + return } log::trace!(target: "test_before_add", "{}", EXPECTED_WITH_TRACE); log::debug!(target: "test_before_add", "{}", EXPECTED_BEFORE_ADD); diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index 2fa25bb30b361..599e80676cb19 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -25,7 +25,7 @@ use codec::Codec; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, - types::error::{CallError, ErrorObject, ErrorCode}, + types::error::{CallError, ErrorCode, ErrorObject}, }; use pallet_contracts_primitives::{ Code, CodeUploadResult, ContractExecResult, ContractInstantiateResult, @@ -322,7 +322,7 @@ fn decode_hex>(from: H, name: &str) -> JsonRpseeError::Call(CallError::Custom(ErrorObject::owned( ErrorCode::InvalidParams.code(), format!("{:?} does not fit into the {} type", from, name), - None::<()> + None::<()>, ))) }) } diff --git a/frame/transaction-payment/rpc/src/lib.rs b/frame/transaction-payment/rpc/src/lib.rs index 2f66f3f8fc993..62beb23233873 100644 --- a/frame/transaction-payment/rpc/src/lib.rs +++ b/frame/transaction-payment/rpc/src/lib.rs @@ -23,7 +23,7 @@ use codec::{Codec, Decode}; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, - types::error::{CallError, ErrorObject, ErrorCode}, + types::error::{CallError, ErrorCode, ErrorObject}, }; use pallet_transaction_payment_rpc_runtime_api::{FeeDetails, InclusionFee, RuntimeDispatchInfo}; use sp_api::ProvideRuntimeApi; diff --git a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs index 07151c382fbad..8f85dc6e432e2 100644 --- a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs +++ b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs @@ -49,15 +49,14 @@ fn count_migrate<'a, H: Hasher>( for node in iter_node { let node = node.map_err(|e| format!("TrieDB node iterator error: {}", e))?; match node.2.node_plan() { - NodePlan::Leaf { value, .. } | NodePlan::NibbledBranch { value: Some(value), .. } => { + NodePlan::Leaf { value, .. } | NodePlan::NibbledBranch { value: Some(value), .. } => if let ValuePlan::Inline(range) = value { - if (range.end - range.start) as u32 - >= sp_core::storage::TRIE_VALUE_NODE_THRESHOLD + if (range.end - range.start) as u32 >= + sp_core::storage::TRIE_VALUE_NODE_THRESHOLD { nb += 1; } - } - }, + }, _ => (), } } @@ -74,7 +73,7 @@ where let trie_backend = if let Some(backend) = backend.as_trie_backend() { backend } else { - return Err("No access to trie from backend.".to_string()); + return Err("No access to trie from backend.".to_string()) }; let essence = trie_backend.essence(); let (nb_to_migrate, trie) = count_migrate(essence, &essence.root())?; From 2ab97f79956355a77b293d66dccc3ad5b25dd692 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 20 Apr 2022 21:41:24 +0200 Subject: [PATCH 3/3] revert error codes --- client/beefy/rpc/src/lib.rs | 38 +++++++++-- client/consensus/babe/rpc/src/lib.rs | 9 ++- client/finality-grandpa/rpc/src/error.rs | 13 +++- client/finality-grandpa/rpc/src/lib.rs | 14 ++-- client/rpc-api/src/offchain/error.rs | 12 ++-- client/rpc-api/src/policy.rs | 16 ++++- client/rpc-api/src/state/error.rs | 2 +- client/rpc-api/src/system/error.rs | 17 +++-- client/rpc/src/author/mod.rs | 9 +-- client/rpc/src/author/tests.rs | 4 +- client/rpc/src/offchain/mod.rs | 6 +- client/rpc/src/offchain/tests.rs | 8 +-- client/rpc/src/state/mod.rs | 84 ++++++------------------ client/rpc/src/state/tests.rs | 42 +++++++----- client/rpc/src/system/mod.rs | 4 +- client/sync-state-rpc/src/lib.rs | 10 ++- frame/transaction-payment/rpc/src/lib.rs | 55 +++++++++++++--- utils/frame/rpc/system/src/lib.rs | 15 +++-- 18 files changed, 218 insertions(+), 140 deletions(-) diff --git a/client/beefy/rpc/src/lib.rs b/client/beefy/rpc/src/lib.rs index ca5eb825b78ad..e4c8c76419ccb 100644 --- a/client/beefy/rpc/src/lib.rs +++ b/client/beefy/rpc/src/lib.rs @@ -30,6 +30,7 @@ use futures::{task::SpawnError, FutureExt, StreamExt}; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, + types::{error::CallError, ErrorObject}, PendingSubscription, }; use log::warn; @@ -49,7 +50,36 @@ pub enum Error { RpcTaskFailure(#[from] SpawnError), } -/// Provides RPC methods for interacting with BEEFY. +/// The error codes returned by jsonrpc. +pub enum ErrorCode { + /// Returned when BEEFY RPC endpoint is not ready. + NotReady = 1, + /// Returned on BEEFY RPC background task failure. + TaskFailure = 2, +} + +impl From for ErrorCode { + fn from(error: Error) -> Self { + match error { + Error::EndpointNotReady => ErrorCode::NotReady, + Error::RpcTaskFailure(_) => ErrorCode::TaskFailure, + } + } +} + +impl From for JsonRpseeError { + fn from(error: Error) -> Self { + let message = error.to_string(); + let code = ErrorCode::from(error); + JsonRpseeError::Call(CallError::Custom(ErrorObject::owned( + code as i32, + message, + None::<()>, + ))) + } +} + +// Provides RPC methods for interacting with BEEFY. #[rpc(client, server)] pub trait BeefyApi { /// Returns the block most recently finalized by BEEFY, alongside side its justification. @@ -129,7 +159,7 @@ where .as_ref() .cloned() .ok_or(Error::EndpointNotReady) - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(Into::into) } } @@ -172,7 +202,7 @@ mod tests { async fn uninitialized_rpc_handler() { let (rpc, _) = setup_io_handler(); let request = r#"{"jsonrpc":"2.0","method":"beefy_getFinalizedHead","params":[],"id":1}"#; - let expected_response = r#"{"jsonrpc":"2.0","error":{"code":-32000,"message":"BEEFY RPC endpoint not ready"},"id":1}"#.to_string(); + let expected_response = r#"{"jsonrpc":"2.0","error":{"code":1,"message":"BEEFY RPC endpoint not ready"},"id":1}"#.to_string(); let (result, _) = rpc.raw_json_request(&request).await.unwrap(); assert_eq!(expected_response, result,); @@ -197,7 +227,7 @@ mod tests { .to_string(); let not_ready = "{\ \"jsonrpc\":\"2.0\",\ - \"error\":{\"code\":-32000,\"message\":\"BEEFY RPC endpoint not ready\"},\ + \"error\":{\"code\":1,\"message\":\"BEEFY RPC endpoint not ready\"},\ \"id\":1\ }" .to_string(); diff --git a/client/consensus/babe/rpc/src/lib.rs b/client/consensus/babe/rpc/src/lib.rs index faa205c6b1712..f08568c2e42fd 100644 --- a/client/consensus/babe/rpc/src/lib.rs +++ b/client/consensus/babe/rpc/src/lib.rs @@ -22,6 +22,7 @@ use futures::TryFutureExt; use jsonrpsee::{ core::{async_trait, Error as JsonRpseeError, RpcResult}, proc_macros::rpc, + types::{error::CallError, ErrorObject}, }; use sc_consensus_babe::{authorship, Config, Epoch}; @@ -172,7 +173,11 @@ pub enum Error { impl From for JsonRpseeError { fn from(error: Error) -> Self { - JsonRpseeError::to_call_error(error) + JsonRpseeError::Call(CallError::Custom(ErrorObject::owned( + 1234, + error.to_string(), + None::<()>, + ))) } } @@ -267,7 +272,7 @@ mod tests { let request = r#"{"jsonrpc":"2.0","method":"babe_epochAuthorship","params":[],"id":1}"#; let (response, _) = api.raw_json_request(request).await.unwrap(); - let expected = r#"{"jsonrpc":"2.0","error":{"code":-32000,"message":"RPC call is unsafe to be called externally"},"id":1}"#; + let expected = r#"{"jsonrpc":"2.0","error":{"code":-32601,"message":"RPC call is unsafe to be called externally"},"id":1}"#; assert_eq!(&response, expected); } diff --git a/client/finality-grandpa/rpc/src/error.rs b/client/finality-grandpa/rpc/src/error.rs index 6ddeebfb1274f..197c0b8a72102 100644 --- a/client/finality-grandpa/rpc/src/error.rs +++ b/client/finality-grandpa/rpc/src/error.rs @@ -16,7 +16,10 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use jsonrpsee::types::error::{CallError, ErrorObject}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; #[derive(Debug, thiserror::Error)] /// Top-level error type for the RPC handler @@ -58,11 +61,15 @@ impl From for ErrorCode { } } -impl From for CallError { +impl From for JsonRpseeError { fn from(error: Error) -> Self { let message = error.to_string(); let code = ErrorCode::from(error); - Self::Custom(ErrorObject::owned(code as i32, message, None::<()>)) + JsonRpseeError::Call(CallError::Custom(ErrorObject::owned( + code as i32, + message, + None::<()>, + ))) } } diff --git a/client/finality-grandpa/rpc/src/lib.rs b/client/finality-grandpa/rpc/src/lib.rs index 2b8ece10f0f3c..82962d716d589 100644 --- a/client/finality-grandpa/rpc/src/lib.rs +++ b/client/finality-grandpa/rpc/src/lib.rs @@ -24,7 +24,7 @@ use log::warn; use std::sync::Arc; use jsonrpsee::{ - core::{async_trait, Error as JsonRpseeError, RpcResult}, + core::{async_trait, RpcResult}, proc_macros::rpc, PendingSubscription, }; @@ -99,8 +99,7 @@ where ProofProvider: RpcFinalityProofProvider + Send + Sync + 'static, { async fn round_state(&self) -> RpcResult { - ReportedRoundStates::from(&self.authority_set, &self.voter_state) - .map_err(|e| JsonRpseeError::to_call_error(e)) + ReportedRoundStates::from(&self.authority_set, &self.voter_state).map_err(Into::into) } fn subscribe_justifications(&self, pending: PendingSubscription) { @@ -127,8 +126,11 @@ where ) -> RpcResult> { self.finality_proof_provider .rpc_prove_finality(block) - .map_err(|finality_err| error::Error::ProveFinalityFailed(finality_err)) - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(|e| { + warn!("Error proving finality: {}", e); + error::Error::ProveFinalityFailed(e) + }) + .map_err(Into::into) } } @@ -281,7 +283,7 @@ mod tests { #[tokio::test] async fn uninitialized_rpc_handler() { let (rpc, _) = setup_io_handler(EmptyVoterState); - let expected_response = r#"{"jsonrpc":"2.0","error":{"code":-32000,"message":"GRANDPA RPC endpoint not ready"},"id":0}"#.to_string(); + let expected_response = r#"{"jsonrpc":"2.0","error":{"code":1,"message":"GRANDPA RPC endpoint not ready"},"id":0}"#.to_string(); let request = r#"{"jsonrpc":"2.0","method":"grandpa_roundState","params":[],"id":0}"#; let (result, _) = rpc.raw_json_request(&request).await.unwrap(); diff --git a/client/rpc-api/src/offchain/error.rs b/client/rpc-api/src/offchain/error.rs index 7a7b7425bfd09..be72e05fc4460 100644 --- a/client/rpc-api/src/offchain/error.rs +++ b/client/rpc-api/src/offchain/error.rs @@ -18,7 +18,10 @@ //! Offchain RPC errors. -use jsonrpsee::types::error::{CallError, ErrorObject}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; /// Offchain RPC Result type. pub type Result = std::result::Result; @@ -37,14 +40,15 @@ pub enum Error { /// Base error code for all offchain errors. const BASE_ERROR: i32 = 5000; -impl From for CallError { +impl From for JsonRpseeError { fn from(e: Error) -> Self { match e { - Error::UnavailableStorageKind => Self::Custom(ErrorObject::owned( + Error::UnavailableStorageKind => CallError::Custom(ErrorObject::owned( BASE_ERROR + 1, "This storage kind is not available yet", None::<()>, - )), + )) + .into(), Error::UnsafeRpcCalled(e) => e.into(), } } diff --git a/client/rpc-api/src/policy.rs b/client/rpc-api/src/policy.rs index 095cc82dd198e..69ca8958520a6 100644 --- a/client/rpc-api/src/policy.rs +++ b/client/rpc-api/src/policy.rs @@ -21,7 +21,13 @@ //! Contains a `DenyUnsafe` type that can be used to deny potentially unsafe //! RPC when accessed externally. -use jsonrpsee::{core::Error as JsonRpseeError, types::error::CallError}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::{ + error::{CallError, ErrorCode}, + ErrorObject, + }, +}; /// Signifies whether a potentially unsafe RPC should be denied. #[derive(Clone, Copy, Debug)] @@ -57,12 +63,16 @@ impl std::error::Error for UnsafeRpcError {} impl From for CallError { fn from(e: UnsafeRpcError) -> CallError { - CallError::from_std_error(e) + CallError::Custom(ErrorObject::owned( + ErrorCode::MethodNotFound.code(), + e.to_string(), + None::<()>, + )) } } impl From for JsonRpseeError { fn from(e: UnsafeRpcError) -> JsonRpseeError { - JsonRpseeError::to_call_error(e) + JsonRpseeError::Call(e.into()) } } diff --git a/client/rpc-api/src/state/error.rs b/client/rpc-api/src/state/error.rs index e720db5c57d02..b1df64b4789ab 100644 --- a/client/rpc-api/src/state/error.rs +++ b/client/rpc-api/src/state/error.rs @@ -66,7 +66,7 @@ impl From for JsonRpseeError { Error::InvalidCount { .. } => CallError::Custom(ErrorObject::owned(BASE_ERROR + 2, e.to_string(), None::<()>)) .into(), - e => e.into(), + e => Self::to_call_error(e), } } } diff --git a/client/rpc-api/src/system/error.rs b/client/rpc-api/src/system/error.rs index 2fae3587e8418..777f8c6c6df0b 100644 --- a/client/rpc-api/src/system/error.rs +++ b/client/rpc-api/src/system/error.rs @@ -19,7 +19,10 @@ //! System RPC module errors. use crate::system::helpers::Health; -use jsonrpsee::types::error::{CallError, ErrorObject}; +use jsonrpsee::{ + core::Error as JsonRpseeError, + types::error::{CallError, ErrorObject}, +}; /// System RPC Result type. pub type Result = std::result::Result; @@ -42,13 +45,17 @@ const NOT_HEALTHY_ERROR: i32 = BASE_ERROR + 1; // Peer argument is malformatted. const MALFORMATTED_PEER_ARG_ERROR: i32 = BASE_ERROR + 2; -impl From for CallError { +impl From for JsonRpseeError { fn from(e: Error) -> Self { match e { Error::NotHealthy(ref h) => - Self::Custom(ErrorObject::owned(NOT_HEALTHY_ERROR, e.to_string(), Some(h))), - Error::MalformattedPeerArg(e) => - Self::Custom(ErrorObject::owned(MALFORMATTED_PEER_ARG_ERROR + 2, e, None::<()>)), + CallError::Custom(ErrorObject::owned(NOT_HEALTHY_ERROR, e.to_string(), Some(h))), + Error::MalformattedPeerArg(e) => CallError::Custom(ErrorObject::owned( + MALFORMATTED_PEER_ARG_ERROR + 2, + e, + None::<()>, + )), } + .into() } } diff --git a/client/rpc/src/author/mod.rs b/client/rpc/src/author/mod.rs index 5db388cac69e3..8d330355d1dbe 100644 --- a/client/rpc/src/author/mod.rs +++ b/client/rpc/src/author/mod.rs @@ -93,7 +93,7 @@ where async fn submit_extrinsic(&self, ext: Bytes) -> RpcResult> { let xt = match Decode::decode(&mut &ext[..]) { Ok(xt) => xt, - Err(err) => return Err(JsonRpseeError::to_call_error(err)), + Err(err) => return Err(Error::Client(Box::new(err)).into()), }; let best_block_hash = self.client.info().best_hash; self.pool @@ -101,8 +101,9 @@ where .await .map_err(|e| { e.into_pool_error() - .map(|e| JsonRpseeError::to_call_error(e)) - .unwrap_or_else(|e| JsonRpseeError::to_call_error(e)) + .map(|e| Error::Pool(e)) + .unwrap_or_else(|e| Error::Verification(Box::new(e))) + .into() }) } @@ -134,7 +135,7 @@ where .client .runtime_api() .decode_session_keys(&generic::BlockId::Hash(best_block_hash), session_keys.to_vec()) - .map_err(|e| JsonRpseeError::to_call_error(e))? + .map_err(|e| Error::Client(Box::new(e)))? .ok_or_else(|| Error::InvalidSessionKeys)?; Ok(SyncCryptoStore::has_keys(&*self.keystore, &keys)) diff --git a/client/rpc/src/author/tests.rs b/client/rpc/src/author/tests.rs index 799e2d5ef42bb..f969812e5b14c 100644 --- a/client/rpc/src/author/tests.rs +++ b/client/rpc/src/author/tests.rs @@ -104,7 +104,7 @@ async fn author_submit_transaction_should_not_cause_error() { assert_matches!( api.call::<_, H256>("author_submitExtrinsic", [xt]).await, - Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Already imported") + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Already Imported") && err.code() == 1013 ); } @@ -159,7 +159,7 @@ async fn author_should_return_watch_validation_error() { assert_matches!( failed_sub, - Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Transaction pool error") + Err(RpcError::Call(CallError::Custom(err))) if err.message().contains("Invalid Transaction") && err.code() == 1010 ); } diff --git a/client/rpc/src/offchain/mod.rs b/client/rpc/src/offchain/mod.rs index 48f1f33693e5b..b66b78274a64e 100644 --- a/client/rpc/src/offchain/mod.rs +++ b/client/rpc/src/offchain/mod.rs @@ -55,8 +55,7 @@ impl OffchainApiServer for Offchain { let prefix = match kind { StorageKind::PERSISTENT => sp_offchain::STORAGE_PREFIX, - StorageKind::LOCAL => - return Err(JsonRpseeError::to_call_error(Error::UnavailableStorageKind)), + StorageKind::LOCAL => return Err(JsonRpseeError::from(Error::UnavailableStorageKind)), }; self.storage.write().set(prefix, &*key, &*value); Ok(()) @@ -67,8 +66,7 @@ impl OffchainApiServer for Offchain { let prefix = match kind { StorageKind::PERSISTENT => sp_offchain::STORAGE_PREFIX, - StorageKind::LOCAL => - return Err(JsonRpseeError::to_call_error(Error::UnavailableStorageKind)), + StorageKind::LOCAL => return Err(JsonRpseeError::from(Error::UnavailableStorageKind)), }; Ok(self.storage.read().get(prefix, &*key).map(Into::into)) diff --git a/client/rpc/src/offchain/tests.rs b/client/rpc/src/offchain/tests.rs index 21f643daba805..28a7b6115b657 100644 --- a/client/rpc/src/offchain/tests.rs +++ b/client/rpc/src/offchain/tests.rs @@ -47,14 +47,14 @@ fn offchain_calls_considered_unsafe() { assert_matches!( offchain.set_local_storage(StorageKind::PERSISTENT, key.clone(), value.clone()), - Err(JsonRpseeError::Call(CallError::Failed(err))) => { - assert_eq!(err.to_string(), "RPC call is unsafe to be called externally") + Err(JsonRpseeError::Call(CallError::Custom(err))) => { + assert_eq!(err.message(), "RPC call is unsafe to be called externally") } ); assert_matches!( offchain.get_local_storage(StorageKind::PERSISTENT, key), - Err(JsonRpseeError::Call(CallError::Failed(err))) => { - assert_eq!(err.to_string(), "RPC call is unsafe to be called externally") + Err(JsonRpseeError::Call(CallError::Custom(err))) => { + assert_eq!(err.message(), "RPC call is unsafe to be called externally") } ); } diff --git a/client/rpc/src/state/mod.rs b/client/rpc/src/state/mod.rs index 1fdbe0433b476..ffb60536d3a6d 100644 --- a/client/rpc/src/state/mod.rs +++ b/client/rpc/src/state/mod.rs @@ -215,10 +215,7 @@ where data: Bytes, block: Option, ) -> RpcResult { - self.backend - .call(block, method, data) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.call(block, method, data).await.map_err(Into::into) } async fn storage_keys( @@ -226,10 +223,7 @@ where key_prefix: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage_keys(block, key_prefix) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_keys(block, key_prefix).await.map_err(Into::into) } async fn storage_pairs( @@ -238,10 +232,7 @@ where block: Option, ) -> RpcResult> { self.deny_unsafe.check_if_safe()?; - self.backend - .storage_pairs(block, key_prefix) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_pairs(block, key_prefix).await.map_err(Into::into) } async fn storage_keys_paged( @@ -252,7 +243,7 @@ where block: Option, ) -> RpcResult> { if count > STORAGE_KEYS_PAGED_MAX_COUNT { - return Err(JsonRpseeError::to_call_error(Error::InvalidCount { + return Err(JsonRpseeError::from(Error::InvalidCount { value: count, max: STORAGE_KEYS_PAGED_MAX_COUNT, })) @@ -260,7 +251,7 @@ where self.backend .storage_keys_paged(block, prefix, count, start_key) .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(Into::into) } async fn storage( @@ -268,10 +259,7 @@ where key: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage(block, key) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage(block, key).await.map_err(Into::into) } async fn storage_hash( @@ -279,10 +267,7 @@ where key: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage_hash(block, key) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_hash(block, key).await.map_err(Into::into) } async fn storage_size( @@ -290,21 +275,15 @@ where key: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage_size(block, key) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_size(block, key).await.map_err(Into::into) } async fn metadata(&self, block: Option) -> RpcResult { - self.backend.metadata(block).await.map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.metadata(block).await.map_err(Into::into) } async fn runtime_version(&self, at: Option) -> RpcResult { - self.backend - .runtime_version(at) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.runtime_version(at).await.map_err(Into::into) } async fn query_storage( @@ -314,10 +293,7 @@ where to: Option, ) -> RpcResult>> { self.deny_unsafe.check_if_safe()?; - self.backend - .query_storage(from, to, keys) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.query_storage(from, to, keys).await.map_err(Into::into) } async fn query_storage_at( @@ -325,10 +301,7 @@ where keys: Vec, at: Option, ) -> RpcResult>> { - self.backend - .query_storage_at(keys, at) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.query_storage_at(keys, at).await.map_err(Into::into) } async fn read_proof( @@ -336,10 +309,7 @@ where keys: Vec, block: Option, ) -> RpcResult> { - self.backend - .read_proof(block, keys) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.read_proof(block, keys).await.map_err(Into::into) } /// Re-execute the given block with the tracing targets given in `targets` @@ -358,7 +328,7 @@ where self.backend .trace_block(block, targets, storage_keys, methods) .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(Into::into) } fn subscribe_runtime_version(&self, sink: PendingSubscription) { @@ -459,7 +429,7 @@ where self.backend .storage_keys(block, storage_key, key_prefix) .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(Into::into) } async fn storage_keys_paged( @@ -473,7 +443,7 @@ where self.backend .storage_keys_paged(block, storage_key, prefix, count, start_key) .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(Into::into) } async fn storage( @@ -482,10 +452,7 @@ where key: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage(block, storage_key, key) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage(block, storage_key, key).await.map_err(Into::into) } async fn storage_entries( @@ -494,10 +461,7 @@ where keys: Vec, block: Option, ) -> RpcResult>> { - self.backend - .storage_entries(block, storage_key, keys) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_entries(block, storage_key, keys).await.map_err(Into::into) } async fn storage_hash( @@ -506,10 +470,7 @@ where key: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage_hash(block, storage_key, key) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_hash(block, storage_key, key).await.map_err(Into::into) } async fn storage_size( @@ -518,10 +479,7 @@ where key: StorageKey, block: Option, ) -> RpcResult> { - self.backend - .storage_size(block, storage_key, key) - .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + self.backend.storage_size(block, storage_key, key).await.map_err(Into::into) } async fn read_child_proof( @@ -533,7 +491,7 @@ where self.backend .read_child_proof(block, child_storage_key, keys) .await - .map_err(|e| JsonRpseeError::to_call_error(e)) + .map_err(Into::into) } } diff --git a/client/rpc/src/state/tests.rs b/client/rpc/src/state/tests.rs index 6593d7679326a..1b0c131296cd9 100644 --- a/client/rpc/src/state/tests.rs +++ b/client/rpc/src/state/tests.rs @@ -23,7 +23,7 @@ use assert_matches::assert_matches; use futures::executor; use jsonrpsee::{ core::Error as RpcError, - types::{error::CallError as RpcCallError, EmptyParams}, + types::{error::CallError as RpcCallError, EmptyParams, ErrorObject}, }; use sc_block_builder::BlockBuilderProvider; use sc_rpc_api::DenyUnsafe; @@ -372,14 +372,16 @@ async fn should_query_storage() { assert_eq!( result.await.map_err(|e| e.to_string()), - Err(RpcError::Call(RpcCallError::Failed( + Err(RpcError::Call(RpcCallError::Custom(ErrorObject::owned( + 4001, Error::InvalidBlockRange { from: format!("1 ({:?})", block1_hash), to: format!("0 ({:?})", genesis_hash), details: "from number > to number".to_owned(), } - .into() - ))) + .to_string(), + None::<()>, + )))) .map_err(|e| e.to_string()) ); @@ -391,7 +393,8 @@ async fn should_query_storage() { assert_eq!( result.await.map_err(|e| e.to_string()), - Err(RpcError::Call(RpcCallError::Failed( + Err(RpcError::Call(RpcCallError::Custom(ErrorObject::owned( + 4001, Error::InvalidBlockRange { from: format!("{:?}", genesis_hash), to: format!("{:?}", Some(random_hash1)), @@ -400,8 +403,9 @@ async fn should_query_storage() { random_hash1 ), } - .into() - ))) + .to_string(), + None::<()>, + )))) .map_err(|e| e.to_string()) ); @@ -410,7 +414,8 @@ async fn should_query_storage() { assert_eq!( result.await.map_err(|e| e.to_string()), - Err(RpcError::Call(RpcCallError::Failed( + Err(RpcError::Call(RpcCallError::Custom(ErrorObject::owned( + 4001, Error::InvalidBlockRange { from: format!("{:?}", random_hash1), to: format!("{:?}", Some(genesis_hash)), @@ -419,8 +424,9 @@ async fn should_query_storage() { random_hash1 ), } - .into() - ))) + .to_string(), + None::<()>, + )))) .map_err(|e| e.to_string()), ); @@ -429,7 +435,8 @@ async fn should_query_storage() { assert_eq!( result.await.map_err(|e| e.to_string()), - Err(RpcError::Call(RpcCallError::Failed( + Err(RpcError::Call(RpcCallError::Custom(ErrorObject::owned( + 4001, Error::InvalidBlockRange { from: format!("{:?}", random_hash1), to: format!("{:?}", Some(block2_hash)), // Best block hash. @@ -438,8 +445,9 @@ async fn should_query_storage() { random_hash1 ), } - .into() - ))) + .to_string(), + None::<()>, + )))) .map_err(|e| e.to_string()), ); @@ -448,7 +456,8 @@ async fn should_query_storage() { assert_eq!( result.await.map_err(|e| e.to_string()), - Err(RpcError::Call(RpcCallError::Failed( + Err(RpcError::Call(RpcCallError::Custom(ErrorObject::owned( + 4001, Error::InvalidBlockRange { from: format!("{:?}", random_hash1), // First hash not found. to: format!("{:?}", Some(random_hash2)), @@ -457,8 +466,9 @@ async fn should_query_storage() { random_hash1 ), } - .into() - ))) + .to_string(), + None::<()> + )))) .map_err(|e| e.to_string()), ); diff --git a/client/rpc/src/system/mod.rs b/client/rpc/src/system/mod.rs index d9684f995f966..ea24524cd2ea9 100644 --- a/client/rpc/src/system/mod.rs +++ b/client/rpc/src/system/mod.rs @@ -144,7 +144,7 @@ impl SystemApiServer::Number> let _ = self.send_back.unbounded_send(Request::NetworkAddReservedPeer(peer, tx)); match rx.await { Ok(Ok(())) => Ok(()), - Ok(Err(e)) => Err(JsonRpseeError::to_call_error(e)), + Ok(Err(e)) => Err(JsonRpseeError::from(e)), Err(e) => Err(JsonRpseeError::to_call_error(e)), } } @@ -155,7 +155,7 @@ impl SystemApiServer::Number> let _ = self.send_back.unbounded_send(Request::NetworkRemoveReservedPeer(peer, tx)); match rx.await { Ok(Ok(())) => Ok(()), - Ok(Err(e)) => Err(JsonRpseeError::to_call_error(e)), + Ok(Err(e)) => Err(JsonRpseeError::from(e)), Err(e) => Err(JsonRpseeError::to_call_error(e)), } } diff --git a/client/sync-state-rpc/src/lib.rs b/client/sync-state-rpc/src/lib.rs index d0662a6c15c69..0ba39066d2870 100644 --- a/client/sync-state-rpc/src/lib.rs +++ b/client/sync-state-rpc/src/lib.rs @@ -44,6 +44,7 @@ use jsonrpsee::{ core::{Error as JsonRpseeError, RpcResult}, proc_macros::rpc, + types::{error::CallError, ErrorObject}, }; use sc_client_api::StorageData; use sp_blockchain::HeaderBackend; @@ -80,7 +81,11 @@ pub enum Error { impl From> for JsonRpseeError { fn from(error: Error) -> Self { - JsonRpseeError::to_call_error(error) + let message = match error { + Error::JsonRpc(s) => s, + _ => error.to_string(), + }; + CallError::Custom(ErrorObject::owned(1, message, None::<()>)).into() } } @@ -181,8 +186,7 @@ where Backend: HeaderBackend + sc_client_api::AuxStore + 'static, { fn system_gen_sync_spec(&self, raw: bool) -> RpcResult { - let current_sync_state = - self.build_sync_state().map_err(|e| JsonRpseeError::to_call_error(e))?; + let current_sync_state = self.build_sync_state()?; let mut chain_spec = self.chain_spec.cloned_box(); let extension = sc_chain_spec::get_extension_mut::( diff --git a/frame/transaction-payment/rpc/src/lib.rs b/frame/transaction-payment/rpc/src/lib.rs index 62beb23233873..b0be19fdb22a9 100644 --- a/frame/transaction-payment/rpc/src/lib.rs +++ b/frame/transaction-payment/rpc/src/lib.rs @@ -64,6 +64,23 @@ impl TransactionPaymentRpc { } } +/// Error type of this RPC api. +pub enum Error { + /// The transaction was not decodable. + DecodeError, + /// The call to runtime failed. + RuntimeError, +} + +impl From for i32 { + fn from(e: Error) -> i32 { + match e { + Error::RuntimeError => 1, + Error::DecodeError => 2, + } + } +} + #[async_trait] impl TransactionPaymentApiServer<::Hash, RuntimeDispatchInfo> @@ -84,10 +101,21 @@ where let encoded_len = encoded_xt.len() as u32; - let uxt: Block::Extrinsic = Decode::decode(&mut &*encoded_xt) - .map_err(|codec_err| JsonRpseeError::to_call_error(codec_err))?; - api.query_info(&at, uxt, encoded_len) - .map_err(|api_err| JsonRpseeError::to_call_error(api_err)) + let uxt: Block::Extrinsic = Decode::decode(&mut &*encoded_xt).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::DecodeError.into(), + "Unable to query dispatch info.", + Some(format!("{:?}", e)), + )) + })?; + api.query_info(&at, uxt, encoded_len).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to query dispatch info.", + Some(e.to_string()), + )) + .into() + }) } fn query_fee_details( @@ -100,11 +128,20 @@ where let encoded_len = encoded_xt.len() as u32; - let uxt: Block::Extrinsic = Decode::decode(&mut &*encoded_xt) - .map_err(|codec_err| JsonRpseeError::to_call_error(codec_err))?; - let fee_details = api - .query_fee_details(&at, uxt, encoded_len) - .map_err(|api_err| JsonRpseeError::to_call_error(api_err))?; + let uxt: Block::Extrinsic = Decode::decode(&mut &*encoded_xt).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::DecodeError.into(), + "Unable to query fee details.", + Some(format!("{:?}", e)), + )) + })?; + let fee_details = api.query_fee_details(&at, uxt, encoded_len).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to query fee details.", + Some(e.to_string()), + )) + })?; let try_into_rpc_balance = |value: Balance| { value.try_into().map_err(|_| { diff --git a/utils/frame/rpc/system/src/lib.rs b/utils/frame/rpc/system/src/lib.rs index 271b0fb072282..b044035c8120e 100644 --- a/utils/frame/rpc/system/src/lib.rs +++ b/utils/frame/rpc/system/src/lib.rs @@ -102,9 +102,14 @@ where let api = self.client.runtime_api(); let best = self.client.info().best_hash; let at = BlockId::hash(best); - let nonce = api - .account_nonce(&at, account.clone()) - .map_err(|api_err| CallError::from_std_error(api_err))?; + + let nonce = api.account_nonce(&at, account.clone()).map_err(|e| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Unable to query nonce.", + Some(e.to_string()), + )) + })?; Ok(adjust_nonce(&*self.pool, account, nonce)) } @@ -269,8 +274,8 @@ mod tests { // when let res = accounts.dry_run(vec![].into(), None).await; - assert_matches!(res, Err(JsonRpseeError::Call(CallError::Failed(e))) => { - assert_eq!(e.to_string(), "RPC call is unsafe to be called externally"); + assert_matches!(res, Err(JsonRpseeError::Call(CallError::Custom(e))) => { + assert!(e.message().contains("RPC call is unsafe to be called externally")); }); }