cln-rpc: Test that we forward errors correctly

So far we were papering over the actual error with a generic string
that isn't very useful. Now we decode the error and forward it through
the grpc layer as well (though still as a string).

cln-rpc/examples/getinfo.rs

@@ -1,4 +1,4 @@
-use anyhow::Context;
+use anyhow::{anyhow, Context};
 use cln_rpc::{model::GetinfoRequest, ClnRpc, Request};
 use std::env::args;
 use std::path::Path;
@@ -13,7 +13,10 @@ async fn main() -> Result<(), anyhow::Error> {
     let p = Path::new(&rpc_path);
 
     let mut rpc = ClnRpc::new(p).await?;
-    let response = rpc.call(Request::Getinfo(GetinfoRequest {})).await?;
+    let response = rpc
+        .call(Request::Getinfo(GetinfoRequest {}))
+        .await
+        .map_err(|e| anyhow!("Error calling getinfo: {:?}", e))?;
     println!("{}", serde_json::to_string_pretty(&response)?);
     Ok(())
 }

cln-rpc/src/lib.rs

@@ -1,7 +1,7 @@
 use crate::codec::JsonCodec;
 use crate::codec::JsonRpc;
-use anyhow::{Context, Result};
 pub use anyhow::Error;
+use anyhow::Result;
 use futures_util::sink::SinkExt;
 use futures_util::StreamExt;
 use log::{debug, trace};
@@ -21,6 +21,7 @@ pub mod primitives;
 pub use crate::{
     model::{Request, Response},
     notifications::Notification,
+    primitives::RpcError,
 };
 
 ///
@@ -54,30 +55,54 @@ impl ClnRpc {
         })
     }
 
-    pub async fn call(&mut self, req: Request) -> Result<Response, Error> {
+    pub async fn call(&mut self, req: Request) -> Result<Response, RpcError> {
         trace!("Sending request {:?}", req);
 
         // Wrap the raw request in a well-formed JSON-RPC outer dict.
         let id = self.next_id.fetch_add(1, Ordering::SeqCst);
         let req: JsonRpc<Notification, Request> = JsonRpc::Request(id, req);
-        let req = serde_json::to_value(req)?;
+        let req = serde_json::to_value(req).map_err(|e| RpcError {
+            code: None,
+            message: format!("Error parsing request: {}", e),
+        })?;
         let req2 = req.clone();
-        self.write.send(req).await?;
+        self.write.send(req).await.map_err(|e| RpcError {
+            code: None,
+            message: format!("Error passing request to lightningd: {}", e),
+        })?;
 
         let mut response = self
             .read
             .next()
             .await
-            .context("no response from lightningd")?
-            .context("reading response from socket")?;
+            .ok_or_else(|| RpcError {
+                code: None,
+                message: "no response from lightningd".to_string(),
+            })?
+            .map_err(|_| RpcError {
+                code: None,
+                message: "reading response from socket".to_string(),
+            })?;
         trace!("Read response {:?}", response);
 
         // Annotate the response with the method from the request, so
         // serde_json knows which variant of [`Request`] should be
         // used.
         response["method"] = req2["method"].clone();
-	log::warn!("XXX {:?}", response);
-        serde_json::from_value(response).context("converting response into enum")
+        if let Some(_) = response.get("result") {
+            serde_json::from_value(response).map_err(|e| RpcError {
+                code: None,
+                message: format!("Malformed response from lightningd: {}", e),
+            })
+        } else if let Some(e) = response.get("error") {
+            let e: RpcError = serde_json::from_value(e.clone()).unwrap();
+            Err(e)
+        } else {
+            Err(RpcError {
+                code: None,
+                message: format!("Malformed response from lightningd: {}", response),
+            })
+        }
     }
 }
 

cln-rpc/src/primitives.rs

@@ -642,3 +642,11 @@ pub struct Routehint {
 pub struct RoutehintList {
     pub hints: Vec<Routehint>,
 }
+
+/// An error returned by the lightningd RPC consisting of a code and a
+/// message
+#[derive(Clone, Serialize, Deserialize, Debug)]
+pub struct RpcError {
+    pub code: Option<i32>,
+    pub message: String,
+}

tests/test_cln_rs.py

@@ -3,7 +3,7 @@ from pathlib import Path
 from pyln.testing.utils import env, TEST_NETWORK, wait_for
 from ephemeral_port_reserve import reserve
 import grpc
-from primitives_pb2 import AmountOrAny
+from primitives_pb2 import AmountOrAny, Amount
 import pytest
 import subprocess
 
@@ -113,6 +113,15 @@ def test_grpc_connect(node_factory):
     ))
     print(inv)
 
+    # Test a failing RPC call, so we know that errors are returned correctly.
+    with pytest.raises(Exception, match=r'Duplicate label'):
+        # This request creates a label collision
+        stub.Invoice(nodepb.InvoiceRequest(
+            msatoshi=AmountOrAny(amount=Amount(msat=12345)),
+            description="hello",
+            label="lbl1",
+        ))
+
 
 def test_grpc_generate_certificate(node_factory):
     """Test whether we correctly generate the certificates.