[cln-grpc] gRPC call doesn't return the actual RpcError structure

[louneskmt]

trafficstars

Issue and Steps to Reproduce

I am using Rust and tonic in order to make gRPC calls to a Core Lightning node (v0.11.2), using the proto files of this version.

On error, I get the following string instead of a proper body (calling here the Pay method with an invalid bolt11):

Error calling method Pay: RpcError { code: Some(-32602), message: "Invalid bolt11: Bad bech32 string" }

While I should receive:

{
    "code": -32602,
    "message": "Invalid bolt11: Bad bech32 string"
}

This looks like an error formatted to a debug string, raised and returned.

getinfo output

Running in Polar.

Toggle output

{
   "id": "0294277c047bead8c306a3eca034bea30386df5d9a20a0919674144f01c1aa4e57",
   "alias": "bob",
   "color": "029427",
   "num_peers": 2,
   "num_pending_channels": 0,
   "num_active_channels": 2,
   "num_inactive_channels": 0,
   "address": [],
   "binding": [
      {
         "type": "ipv4",
         "address": "172.21.0.3",
         "port": 19846
      }
   ],
   "version": "v0.11.2",
   "blockheight": 126,
   "network": "regtest",
   "msatoshi_fees_collected": 0,
   "fees_collected_msat": "0msat",
   "lightning-dir": "/home/clightning/.lightning/regtest",
   "our_features": {
      "init": "080269a2",
      "node": "800000080269a2",
      "channel": "",
      "invoice": "02000000024100"
   }
}

[cdecker]

Thanks for reporting this. I was not aware that grpc even allows for richer errors being returned. From what I can see tonic will convert any std::io::Error into its string representation by default, hence why we defaulted to that as well. The only facility to add more context and rich error I can find are the with_details_and_metadata, with_metadata and [with_details](with_metadata methods, none of them allowing us to pass an unserialized protobuf message. If desired we could add the numeric error code in the metadata, but for now we don't have plans to give errors a schema, making them serializable as protobuf messages.

[louneskmt]

Maybe we could implement a conversion from RpcError to a tonic::Status, embedding the RPC error code and message into metadata (using a nested error in order to comply with the orphan rule)? It could look like this:

#[derive(Clone, Serialize, Deserialize, Debug)]
pub struct GrpcError(pub RpcError);

impl From<GrpcError> for tonic::Status {
    fn from(err: GrpcError) -> Self {
        let rpc_err = err.0;

        let mut metadata = MetadataMap::new();
        if let Some(code) = rpc_err.code {
            metadata.insert("code", code);
        }
        metadata.insert("message", rpc_err.message);

        tonic::Status::with_metadata(tonic::Code::Unknown, rpc_err.to_string(), metadata)
    }
}

Then change the following https://github.com/ElementsProject/lightning/blob/82f703552455549f7470e6c9fab136c5a95b2225/cln-grpc/src/server.rs#L40-L44 to

let result = rpc.call(Request::Getinfo(req))
    .await
    .map_err(|e| GrpcError(e))?;

for each call.

I haven't tested this, just thinking.

[louneskmt]

For now, I'm parsing the error message with regular expressions, but this isn't really ideal haha

impl From<tonic::Status> for Error {
    fn from(status: tonic::Status) -> Self {
        let status_message = status.message().to_string();

        let rpc_error_regex =
            regex::Regex::new(r"RpcError").expect("Hardcoded regex should be valid.");
        if rpc_error_regex.is_match(&status_message) {
            let rpc_error_message_regex = regex::Regex::new(r#"message: "(?P<msg>.*)""#)
                .expect("Hardcoded regex should be valid.");
            let rpc_error_message = rpc_error_message_regex
                .captures(&status_message)
                .and_then(|cap| cap.name("msg").map(|msg| msg.as_str()));

            if let Some(message) = rpc_error_message {
                return Error::ApiError(message.to_string());
            }
        }

        Error::ApiError(status_message)
    }
}