[Feature Request] Create a wrapper around the client which implements the service trait

[Rustleman]

Currently I’m using the RPC client similar to this example where I create a wrapper around the client:

#[async_trait]
#[export_trait]
pub trait Arith {
    #[export_method]
    async fn add(&self, args: (i32, i32)) -> anyhow::Result<i32>;

    #[export_method]
    async fn subtract(&self, args: (i32, i32)) -> anyhow::Result<i32>;
}

pub struct ArithClientWrapper {
    pub client: toy_rpc::Client<Connected>,
}

#[async_trait]
impl Arith for ArithClientWrapper {
    async fn add(&self, args: (i32, i32)) -> anyhow::Result<i32> {
        Ok(self.client.arith().add(args).await?)
    }

    async fn subtract(&self, args: (i32, i32)) -> anyhow::Result<i32> {
        Ok(self.client.arith().subtract(args).await?)
    }
}

In this case it converts the result of the call into a anyhow::Result. This way I can use the wrapper for a function which requires an implementation of the Arith trait as follows:

async fn foo(arith: impl Arith) {
    arith.add((1, 2)).await
}

let wrapper = ArithClientWrapper { client };

foo(wrapper).await

What I would like to see is a feature which can automatically generate such a wrapper or something similar which implements the trait of the RPC service.

[minghuaw]

Something similar is already generated by the macro. When you call client.arith() it actually creates a client wrapper ArithClient, which has a reference to the client. You can imagine something like

// This definition is generated by the proc_macro
// pub struct ArithClient<'a> {
//     inner: &'a Client
// }
// 
// The `Arith` impl is also generated for `ArithClient` by the macro
// impl<'a> Arith for ArithClient<'a> {
//     // ...
// }

let arith: ArithClient = client.arith();

But this does hold a reference to the client, so I am not sure if the lifetime param will be a constraint for you.

[minghuaw]

If you need a wrapper that holds ownership of the client instance, that should be a relative easy change in the proc_macro and probably won't break anything.

[Rustleman]

If you need a wrapper that holds ownership of the client instance, that should be a relative easy change in the proc_macro and probably won't break anything.

Yes the ownership is one issue, but more importantly my point was that the Airth trait is not implemented by the ArithClient. This is the result of cargo expand:

    pub struct ArithClient<'c> {
        client: &'c toy_rpc::client::Client<toy_rpc::client::Connected>,
        service_name: &'c str,
    }
    impl<'c> ArithClient<'c> {
        pub fn add<A>(&'c self, args: A) -> toy_rpc::client::Call<i32>
        where
            A: std::borrow::Borrow<(i32, i32)> + Send + Sync + toy_rpc::serde::Serialize + 'static,
        {
            self.client.call("Arith.add", args)
        }
        pub fn subtract<A>(&'c self, args: A) -> toy_rpc::client::Call<i32>
        where
            A: std::borrow::Borrow<(i32, i32)> + Send + Sync + toy_rpc::serde::Serialize + 'static,
        {
            self.client.call("Arith.subtract", args)
        }
    }

It generates a new struct and methods for add and subtract which return a Call. But what I what would like is another struct which implements Arith:

impl Arith for ArithClientWrapper

For this to work the Call needs to be converted into the original Result type like I did in the example. This is probably not desirable for everyone, so my Idea was to keep the ArithClient as it is where every method returns the Call and make another struct ArithClientWrapper which attempts to implement the trait. Though this might only work for some result types (like anyhow::Result or Result<_, String> or maybe the Error can be serialized on one side and deserialized on the other side if it's not a String).

[minghuaw]

I totally forgot that the ArithClient methods returns a Call. I think what I can do is to generate From impl for a new type (say ArithClientWrapper), and the usage will probably look like

// This will take ownership of `client` 
// and will impl `Arith` trait by simply `.awaiting` on the call.
let wrapper: ArithClientWrapper = client.into::<ArithClientWrapper>();

This will probably be gated behind a new feature flag. I haven't thought about the name of the new feature. Do you have any name for the new feature flag name? @jadedragon-mrm

[Rustleman]

I think what I can do is to generate From impl for a new type (say ArithClientWrapper)

Yes that should be fine.

This will probably be gated behind a new feature flag. I haven't thought about the name of the new feature. Do you have any name for the new feature flag name?

How about client_trait_impl ?

[minghuaw]

I think there is probably a better design. Instead of having wrapper type, the client itself can implement the trait, so the usage will be something like

let result = Arith::add(&client, (3i32, 4)).await;

[minghuaw]

I think there is probably a better design. Instead of having wrapper type, the client itself can implement the trait, so the usage will be something like

let result = Arith::add(&client, (3i32, 4)).await;

Actually there could be some problem with errors . The Result definition are different. .awaiting on the Call will give a result that has toy_rpc::Error as the Err value because there are other source of errors than the execution itself. Although one work around could be mapping the toy_rpc::Error to a String with map_err, the control on error handling is kind of lost.

I will need to make a few experiments before settling on one solution I guess.

[Rustleman]

Actually there could be some problem with errors . The Result definition are different. .awaiting on the Call will give a result that has toy_rpc::Error as the Err value because there are other source of errors than the execution itself. Although one work around could be mapping the toy_rpc::Error to a String with map_err, the control on error handling is kind of lost.

Yes I think mapping the toy_rpc::Error to String or anyhow::Error is fine.

[minghuaw]

Yes I think mapping the toy_rpc::Error to String or anyhow::Error is fine.

I don't quite like the idea of simply mapping the error to String. I will need to think about some solutions

[minghuaw]

@jadedragon-mrm What about this idea. The RPC method return type definition will change from Result<T, impl ToString> to something like Result<T, Into<toy_rpc::Error>>, and I will provide blanket impl for any type that implements std::error::Error to convert to toy_rpc::Error. toy_rpc::Error also implements std::error::Error, and hence you can use it with anyhow::Result. Please let me know if this sounds like a good idea to you.

[Rustleman]

The RPC method return type definition will change from Result<T, impl ToString> to something like Result<T, Into<toy_rpc::Error>>, and I will provide blanket impl for any type that implements std::error::Error to convert to toy_rpc::Error. toy_rpc::Error also implements std::error::Error, and hence you can use it with anyhow::Result. Please let me know if this sounds like a good idea to you.

If it works as intended, that would solve the problem too I think. toy_rpc::Error could have a Remote or Forwarded error type to hold the original error.

[minghuaw]

I have uploaded my initial attempt in a pre-alpha branch (as well as a tag). In short, there is no additional feature flag but an argument for the export_trait macro on the service declaration (an example here), which looks like this

#[async_trait]
#[export_trait(impl_for_client)]
pub trait Arith {
    // This example uses `toy_rpc::Error` type, other error types have not been tested
    #[export_method]
    async fn add(&self, args: (i32, i32)) -> Result<i32, Error>; 

    #[export_method]
    async fn subtract(&self, args: (i32, i32)) -> Result<i32, Error>;
}

The exact line of client usage can be found here, which looks like below

let reply = Arith::add(&client, (3i32, 4i32)).await;
println!("{:?}", reply);

PubSub will likely take another one or two days. Please let me know if this looks reasonable to you.

[Rustleman]

let reply = Arith::add(&client, (3i32, 4i32)).await;
println!("{:?}", reply);

I think this is fine. Will the implicit error conversation also work when I call reply? ?

[minghuaw]

The reply will be of type Result<i32, toy_rpc::Error> in this case, and because toy_rpc::Error implements std::error::Error trait, you should be able to simply use ? with anyhow::Result. I have implemented From trait for all basic types (string, u8, etc) to convert to toy_rpc::Error, and I will probably also impl TryFrom for all the basic types as well. So you will be able to simply use ? in the service implementation