error: RpcError with custom client error (#694)

* error: `RpcError` with custom client error

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* error: Add `SubscriptionDropped` and panic on param errors

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* Update subxt/src/rpc/rpc_client_t.rs

Co-authored-by: James Wilson <james@jsdw.me>

* Apply rustfmt

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: James Wilson <james@jsdw.me>

subxt/src/client/online_client.rs

@@ -74,7 +74,7 @@ impl<T: Config> OnlineClient<T> {
     pub async fn from_url(url: impl AsRef<str>) -> Result<OnlineClient<T>, Error> {
         let client = jsonrpsee_helpers::ws_client(url.as_ref())
             .await
-            .map_err(|e| crate::error::RpcError(e.to_string()))?;
+            .map_err(|e| crate::error::RpcError::ClientError(Box::new(e)))?;
         OnlineClient::from_rpc_client(Arc::new(client)).await
     }
 }

subxt/src/error.rs

@@ -102,10 +102,17 @@ impl From<DispatchError> for Error {
 }
 
 /// An RPC error. Since we are generic over the RPC client that is used,
-/// the error is any custom string.
+/// the error is boxed and could be casted.
 #[derive(Debug, thiserror::Error)]
-#[error("RPC error: {0}")]
-pub struct RpcError(pub String);
+#[error("RPC error")]
+pub enum RpcError {
+    // Dev note: We need the error to be safely sent between threads
+    // for `subscribe_to_block_headers_filling_in_gaps` and friends.
+    /// Error related to the RPC client.
+    ClientError(Box<dyn std::error::Error + Send + 'static>),
+    /// The RPC subscription dropped.
+    SubscriptionDropped,
+}
 
 /// This is our attempt to decode a runtime DispatchError. We either
 /// successfully decode it into a [`ModuleError`], or we fail and keep

subxt/src/rpc/jsonrpsee_impl.rs

@@ -32,10 +32,10 @@ impl RpcClientT for Client {
         params: Option<Box<RawValue>>,
     ) -> RpcFuture<'a, Box<RawValue>> {
         Box::pin(async move {
-            let params = prep_params_for_jsonrpsee(params)?;
+            let params = prep_params_for_jsonrpsee(params);
             let res = ClientT::request(self, method, Some(params))
                 .await
-                .map_err(|e| RpcError(e.to_string()))?;
+                .map_err(|e| RpcError::ClientError(Box::new(e)))?;
             Ok(res)
         })
     }
@@ -47,7 +47,7 @@ impl RpcClientT for Client {
         unsub: &'a str,
     ) -> RpcFuture<'a, RpcSubscription> {
         Box::pin(async move {
-            let params = prep_params_for_jsonrpsee(params)?;
+            let params = prep_params_for_jsonrpsee(params);
             let sub = SubscriptionClientT::subscribe::<Box<RawValue>>(
                 self,
                 sub,
@@ -55,8 +55,8 @@ impl RpcClientT for Client {
                 unsub,
             )
             .await
-            .map_err(|e| RpcError(e.to_string()))?
-            .map_err(|e| RpcError(e.to_string()))
+            .map_err(|e| RpcError::ClientError(Box::new(e)))?
+            .map_err(|e| RpcError::ClientError(Box::new(e)))
             .boxed();
             Ok(sub)
         })
@@ -65,22 +65,18 @@ impl RpcClientT for Client {
 
 // This is ugly; we have to encode to Value's to be compat with the jsonrpc interface.
 // Remove and simplify this once something like https://github.com/paritytech/jsonrpsee/issues/862 is in:
-fn prep_params_for_jsonrpsee(
-    params: Option<Box<RawValue>>,
-) -> Result<ParamsSer<'static>, RpcError> {
+fn prep_params_for_jsonrpsee(params: Option<Box<RawValue>>) -> ParamsSer<'static> {
     let params = match params {
         Some(params) => params,
         // No params? avoid any work and bail early.
-        None => return Ok(ParamsSer::Array(Vec::new())),
+        None => return ParamsSer::Array(Vec::new()),
     };
     let val = serde_json::to_value(&params).expect("RawValue guarantees valid JSON");
     let arr = match val {
-        Value::Array(arr) => Ok(arr),
+        Value::Array(arr) => arr,
         _ => {
-            Err(RpcError(format!(
-                "RPC Params are expected to be an array but got {params}"
-            )))
+            panic!("RPC Params are expected to be an array but got {params}");
         }
-    }?;
-    Ok(ParamsSer::Array(arr))
+    };
+    ParamsSer::Array(arr)
 }

subxt/src/rpc/rpc_client_t.rs

@@ -20,6 +20,11 @@ pub use serde_json::value::RawValue;
 /// the caller. This is the case because we want the methods to be object-safe (which prohibits
 /// generics), and want to avoid any unnecessary allocations in serializing/deserializing
 /// parameters.
+///
+/// # Panics
+///
+/// Implementations are free to panic if the `RawValue`'s passed to `request_raw` or
+/// `subscribe_raw` are not JSON arrays. Internally, we ensure that this is always the case.
 pub trait RpcClientT: Send + Sync + 'static {
     /// Make a raw request for which we expect a single response back from. Implementations
     /// should expect that the params will either be `None`, or be an already-serialized

subxt/src/tx/tx_progress.rs

@@ -107,7 +107,7 @@ where
                 _ => continue,
             }
         }
-        Err(RpcError("RPC subscription dropped".to_string()).into())
+        Err(RpcError::SubscriptionDropped.into())
     }
 
     /// Wait for the transaction to be finalized, and return a [`TxInBlock`]
@@ -133,7 +133,7 @@ where
                 _ => continue,
             }
         }
-        Err(RpcError("RPC subscription dropped".to_string()).into())
+        Err(RpcError::SubscriptionDropped.into())
     }
 
     /// Wait for the transaction to be finalized, and for the transaction events to indicate