crc: RpcError expose punctual error types, #6250

Author: fdefelici

stacks-node/src/burnchains/rpc/bitcoin_rpc_client/tests.rs

@@ -176,7 +176,7 @@ fn test_get_blockchain_info_fails_for_unknown_network() {
 
     assert!(matches!(
         error,
-        BitcoinRpcClientError::Rpc(RpcError::Decode(_))
+        BitcoinRpcClientError::Rpc(RpcError::DecodeJson(_))
     ));
 }
 
@@ -433,7 +433,7 @@ fn test_generate_to_address_fails_for_invalid_block_hash() {
         .expect_err("Should fail!");
     assert!(matches!(
         error,
-        BitcoinRpcClientError::Rpc(RpcError::Decode(_))
+        BitcoinRpcClientError::Rpc(RpcError::DecodeJson(_))
     ));
 }
 
@@ -586,7 +586,7 @@ fn test_generate_block_fails_for_invalid_block_hash() {
         .expect_err("Should fail!");
     assert!(matches!(
         error,
-        BitcoinRpcClientError::Rpc(RpcError::Decode(_))
+        BitcoinRpcClientError::Rpc(RpcError::DecodeJson(_))
     ));
 }
 
@@ -842,7 +842,7 @@ fn test_get_new_address_fails_for_invalid_address() {
         .expect_err("Should fail!");
     assert!(matches!(
         error,
-        BitcoinRpcClientError::Rpc(RpcError::Decode(_))
+        BitcoinRpcClientError::Rpc(RpcError::DecodeJson(_))
     ))
 }
 
@@ -919,7 +919,7 @@ fn test_send_to_address_fails_for_invalid_tx_id() {
         .expect_err("Should fail!");
     assert!(matches!(
         error,
-        BitcoinRpcClientError::Rpc(RpcError::Decode(_))
+        BitcoinRpcClientError::Rpc(RpcError::DecodeJson(_))
     ));
 }
 

stacks-node/src/burnchains/rpc/rpc_transport/mod.rs

@@ -58,43 +58,62 @@ struct JsonRpcResponse<T> {
     /// Result returned from the RPC method, if successful.
     result: Option<T>,
     /// Error object returned by the RPC server, if the call failed.
-    error: Option<Value>,
+    error: Option<JsonRpcError>,
+}
+
+/// Represents the JSON-RPC response error received from the endpoint
+#[derive(Deserialize, Debug, thiserror::Error)]
+#[error("JsonRpcError code {code}: {message}")]
+pub struct JsonRpcError {
+    /// error code
+    code: i32,
+    /// human-readable error message
+    message: String,
+    /// data can be any JSON value or omitted
+    data: Option<Value>,
 }
 
 /// Represents a JSON-RPC error encountered during a transport operation.
-#[derive(Debug, Clone, Deserialize, Serialize)]
+#[derive(Debug, thiserror::Error)]
 pub enum RpcError {
-    /// Represents a network-level error, such as connection failures or timeouts.
-    Network(String),
-    /// Indicates that the request could not be encoded properly
-    Encode(String),
-    /// Indicates that the response could not be decoded properly.
-    Decode(String),
+    // Serde decoding error
+    #[error("JSON decoding error: {0}")]
+    DecodeJson(serde_json::Error),
+    // Serde encoding error
+    #[error("JSON encoding error: {0}")]
+    EncodeJson(serde_json::Error),
+    /// Indicates that the response doesn't contain a json payload
+    #[error("Invalid JSON payload error")]
+    InvalidJsonPayload,
+    // RPC Id mismatch between request and response
+    #[error("Id Mismatch! Request: {0}, Response: {1}")]
+    MismatchedId(String, String),
+    // Stacks common network error
+    #[error("Stacks Net error: {0}")]
+    NetworkStacksCommon(#[from] stacks_common::types::net::Error),
+    // Stacks network lib error
+    #[error("IO error: {0}")]
+    NetworkIO(#[from] io::Error),
+    // Stacks lib network error
+    #[error("Stacks Net error: {0}")]
+    NetworkStacksLib(#[from] stacks::net::Error),
     /// Represents an error returned by the RPC service itself.
-    Service(String),
+    #[error("Service JSON error: {0}")]
+    Service(JsonRpcError),
+    // URL missing host error
+    #[error("URL missing host error: {0}")]
+    UrlMissingHost(Url),
+    // URL missing port error
+    #[error("URL missing port error: {0}")]
+    UrlMissingPort(Url),
+    // URL parse error
+    #[error("URL error: {0}")]
+    UrlParse(#[from] url::ParseError),
 }
 
 /// Alias for results returned from RPC operations using `RpcTransport`.
 pub type RpcResult<T> = Result<T, RpcError>;
 
-impl From<url::ParseError> for RpcError {
-    fn from(e: url::ParseError) -> Self {
-        Self::Network(format!("Url Error: {e:?}"))
-    }
-}
-
-impl From<stacks_common::types::net::Error> for RpcError {
-    fn from(e: stacks_common::types::net::Error) -> Self {
-        Self::Network(format!("Net Error: {e:?}"))
-    }
-}
-
-impl From<io::Error> for RpcError {
-    fn from(e: io::Error) -> Self {
-        Self::Network(format!("IO Error: {e:?}"))
-    }
-}
-
 /// Represents supported authentication mechanisms for RPC requests.
 #[derive(Debug, Clone)]
 pub enum RpcAuth {
@@ -136,10 +155,10 @@ impl RpcTransport {
         let url_obj = Url::parse(&url)?;
         let host = url_obj
             .host_str()
-            .ok_or(RpcError::Network(format!("Missing host in url: {url}")))?;
+            .ok_or(RpcError::UrlMissingHost(url_obj.clone()))?;
         let port = url_obj
             .port_or_known_default()
-            .ok_or(RpcError::Network(format!("Missing port in url: {url}")))?;
+            .ok_or(RpcError::UrlMissingHost(url_obj.clone()))?;
 
         let peer: PeerHost = format!("{host}:{port}").parse()?;
         let path = url_obj.path().to_string();
@@ -177,20 +196,15 @@ impl RpcTransport {
             params: Value::Array(params),
         };
 
-        let json_payload = serde_json::to_value(payload)
-            .map_err(|e| RpcError::Encode(format!("Failed to encode request as JSON: {e:?}")))?;
+        let json_payload = serde_json::to_value(payload).map_err(RpcError::EncodeJson)?;
 
         let mut request = StacksHttpRequest::new_for_peer(
             self.peer.clone(),
             "POST".to_string(),
             self.path.clone(),
             HttpRequestContents::new().payload_json(json_payload),
-        )
-        .map_err(|e| {
-            RpcError::Encode(format!(
-                "Failed to encode infallible data as HTTP request {e:?}"
-            ))
-        })?;
+        )?;
+
         request.add_header("Connection".into(), "close".into());
 
         if let Some(auth_header) = self.auth_header() {
@@ -203,27 +217,24 @@ impl RpcTransport {
         let response = send_http_request(&host, port, request, self.timeout)?;
         let json_response = match response.destruct().1 {
             HttpResponsePayload::JSON(js) => Ok(js),
-            _ => Err(RpcError::Decode("Did not get a JSON response".to_string())),
+            _ => Err(RpcError::InvalidJsonPayload),
         }?;
 
-        let parsed_response: JsonRpcResponse<T> = serde_json::from_value(json_response)
-            .map_err(|e| RpcError::Decode(format!("Json Parse Error: {e:?}")))?;
+        let parsed_response: JsonRpcResponse<T> =
+            serde_json::from_value(json_response).map_err(RpcError::DecodeJson)?;
 
         if id != parsed_response.id {
-            return Err(RpcError::Decode(format!(
-                "Invalid response: mismatched 'id': expected '{}', got '{}'",
-                id, parsed_response.id
-            )));
+            return Err(RpcError::MismatchedId(id.to_string(), parsed_response.id));
         }
 
         if let Some(error) = parsed_response.error {
-            return Err(RpcError::Service(format!("{:#}", error)));
+            return Err(RpcError::Service(error));
         }
 
         if let Some(result) = parsed_response.result {
             Ok(result)
         } else {
-            Ok(serde_json::from_value(Value::Null).unwrap())
+            Ok(serde_json::from_value(Value::Null).map_err(RpcError::DecodeJson)?)
         }
     }
 

stacks-node/src/burnchains/rpc/rpc_transport/tests.rs

@@ -110,13 +110,19 @@ fn test_send_with_string_result_with_basic_auth_ok() {
 }
 
 #[test]
-fn test_send_fails_with_network_error() {
-    let transport = RpcTransport::new("http://127.0.0.1:65535".to_string(), RpcAuth::None, None)
+fn test_send_fails_due_to_unreachable_endpoint() {
+    let unreachable_endpoint = "http://127.0.0.1:65535".to_string();
+    let transport = RpcTransport::new(unreachable_endpoint, RpcAuth::None, None)
         .expect("Should be created properly!");
 
     let result: RpcResult<Value> = transport.send("client_id", "dummy_method", vec![]);
     assert!(result.is_err());
-    assert!(matches!(result.unwrap_err(), RpcError::Network(_)));
+
+    let err = result.unwrap_err();
+    assert!(
+        matches!(err, RpcError::NetworkIO(_)),
+        "Expected NetworkIO error, got: {err:?}"
+    );
 }
 
 #[test]
@@ -133,10 +139,11 @@ fn test_send_fails_with_http_500() {
 
     assert!(result.is_err());
     match result {
-        Err(RpcError::Network(msg)) => {
-            assert!(msg.contains("500"))
+        Err(RpcError::NetworkIO(e)) => {
+            let msg = e.to_string();
+            assert!(msg.contains("500"), "Should contain error 500!");
         }
-        _ => panic!("Expected error 500"),
+        other => panic!("Expected NetworkIO error, got: {other:?}"),
     }
 }
 
@@ -155,10 +162,14 @@ fn test_send_fails_with_invalid_json() {
 
     assert!(result.is_err());
     match result {
-        Err(RpcError::Network(msg)) => {
-            assert!(msg.contains("invalid message"))
+        Err(RpcError::NetworkIO(e)) => {
+            let msg = e.to_string();
+            assert!(
+                msg.contains("invalid message"),
+                "Should contain 'invalid message'!"
+            )
         }
-        _ => panic!("Expected network error"),
+        other => panic!("Expected NetworkIO error, got: {other:?}"),
     }
 }
 
@@ -184,7 +195,7 @@ fn test_send_ok_if_missing_both_result_and_error() {
 #[test]
 fn test_send_fails_with_invalid_id() {
     let response_body = json!({
-        "id": "wrong_client_id",
+        "id": "res_client_id_wrong",
         "result": true,
     });
 
@@ -197,14 +208,14 @@ fn test_send_fails_with_invalid_id() {
         .create();
 
     let transport = utils::rpc_no_auth(&server);
-    let result: RpcResult<Value> = transport.send("client_id", "dummy", vec![]);
+    let result: RpcResult<Value> = transport.send("req_client_id", "dummy", vec![]);
 
     match result {
-        Err(RpcError::Decode(msg)) => assert_eq!(
-            "Invalid response: mismatched 'id': expected 'client_id', got 'wrong_client_id'",
-            msg
-        ),
-        _ => panic!("Expected missing result/error error"),
+        Err(RpcError::MismatchedId(req_id, res_id)) => {
+            assert_eq!("req_client_id", req_id);
+            assert_eq!("res_client_id_wrong", res_id);
+        }
+        other => panic!("Expected MismatchedId, got {other:?}"),
     }
 }
 
@@ -231,11 +242,11 @@ fn test_send_fails_with_service_error() {
     let result: RpcResult<Value> = transport.send("client_id", "unknown_method", vec![]);
 
     match result {
-        Err(RpcError::Service(msg)) => assert_eq!(
-            "{\n  \"code\": -32601,\n  \"message\": \"Method not found\"\n}",
-            msg
-        ),
-        _ => panic!("Expected service error"),
+        Err(RpcError::Service(err)) => {
+            assert_eq!(-32601, err.code);
+            assert_eq!("Method not found", err.message);
+        }
+        other => panic!("Expected Service error, got {other:?}"),
     }
 }
 
@@ -275,9 +286,10 @@ fn test_send_fails_due_to_timeout() {
 
     assert!(result.is_err());
     match result.unwrap_err() {
-        RpcError::Network(msg) => {
-            assert!(msg.contains("Timed out"));
+        RpcError::NetworkIO(e) => {
+            let msg = e.to_string();
+            assert!(msg.contains("Timed out"), "Should contain 'Timed out'!");
         }
-        err => panic!("Expected network error, got: {:?}", err),
+        other => panic!("Expected NetworkIO error, got: {other:?}"),
     }
 }

stacks-node/src/tests/bitcoin_rpc_integrations.rs

@@ -97,7 +97,7 @@ fn test_rpc_call_fails_when_bitcond_with_auth_but_rpc_no_auth() {
     let err = client.get_blockchain_info().expect_err("Should fail!");
 
     assert!(
-        matches!(err, BitcoinRpcClientError::Rpc(RpcError::Network(_))),
+        matches!(err, BitcoinRpcClientError::Rpc(RpcError::NetworkIO(_))),
         "Expected RpcError::Network, got: {err:?}"
     );
 }
@@ -123,7 +123,7 @@ fn test_rpc_call_fails_when_bitcond_no_auth_and_rpc_no_auth() {
     let err = client.get_blockchain_info().expect_err("Should fail!");
 
     assert!(
-        matches!(err, BitcoinRpcClientError::Rpc(RpcError::Network(_))),
+        matches!(err, BitcoinRpcClientError::Rpc(RpcError::NetworkIO(_))),
         "Expected RpcError::Network, got: {err:?}"
     );
 }
@@ -232,8 +232,8 @@ fn test_wallet_creation_fails_if_already_exists() {
         .expect_err("mywallet1 creation should fail now!");
 
     match &err {
-        BitcoinRpcClientError::Rpc(RpcError::Network(msg)) => {
-            assert!(msg.contains("500"), "Bitcoind v25 returns HTTP 500)");
+        BitcoinRpcClientError::Rpc(RpcError::NetworkIO(_)) => {
+            assert!(true, "Bitcoind v25 returns HTTP 500)");
         }
         BitcoinRpcClientError::Rpc(RpcError::Service(_)) => {
             assert!(true, "Bitcoind v26+ returns HTTP 200");