tonic icon indicating copy to clipboard operation
tonic copied to clipboard

Improve ergonomics when passing Clients as function arguments

Open daniel5151 opened this issue 6 years ago • 10 comments
trafficstars

Feature Request

Motivation

At the moment, any function that takes a Client as a parameter requires specifying some pretty scary-looking type bounds:

async fn foo<T>(mut client: MyClient<T>)
where
    T: tonic::client::GrpcService<tonic::body::BoxBody>,
    T::ResponseBody: tonic::codegen::Body + tonic::codegen::HttpBody + Send + 'static,
    T::Error: Into<tonic::codegen::StdError>,
    <T::ResponseBody as tonic::codegen::HttpBody>::Error: Into<tonic::codegen::StdError> + Send,
    <T::ResponseBody as tonic::codegen::HttpBody>::Data: Into<bytes::Bytes> + Send,
{ ... }

It would be nice to somehow simplify these sorts of declarations.

Proposal

I'm not entirely sure how to work around this issue!

A heavyweight solution might be to write a proc macro that automagically includes these type bounds on a function. e.g:

#[tonic::clientbounds(T)]
async fn foo<T>(mut client: MyClient<T>) { ... }

Alternatives

There are probably other, simpler solutions, but I can't think of any great ones off the top of my head. That said, I don't have a thorough understanding of tonic internals, so maybe there is an easy fix.

daniel5151 avatar Oct 31 '19 21:10 daniel5151

An easy workaround is to use a concrete type instead of the type parameter. Currently, I believe the only concrete type that can be used to construct a client is a Channel, so you could do:

async fn foo(mut client: MyClient<Channel>) {...}

Here's an example: https://github.com/hyperium/tonic/blob/master/tonic-examples/src/routeguide/client.rs#L17

alce avatar Oct 31 '19 22:10 alce

Huh, that totally solves my particular issue. Not sure why I didn't think of that earlier! Thanks! :smile:

That said, there are times when it might be useful to write transport-independent functions. For example, a API might require first calling client.update_foo(), and if that fails, falling back to client.create_foo(). It would be nice to easily write a helper function update_or_create_foo<C>(client: FooClient<C>) that orchestrates those gRPC calls.

daniel5151 avatar Oct 31 '19 23:10 daniel5151

I agree, you raise a good point. I believe there are plans (and maybe even some initial work) to provide different transports, in which case we may need to find a different solution.

alce avatar Oct 31 '19 23:10 alce

@daniel5151 yeah, so I tried to get https://docs.rs/tonic/0.1.0-alpha.5/tonic/client/trait.GrpcService.html to do as much of that work as possible but it seemed that rust was not happen. Sounds like this would be a good thing to revisit.

LucioFranco avatar Nov 02 '19 17:11 LucioFranco

So, what @alce said (about Channel being the only thing you can create a client with) is not really true anymore and with Interceptors this quickly becomes unwieldy since you can get types like: Client<tonic::codegen::InterceptedService<HttpStatusInterceptor, [closure@src/registry.rs:83:9: 126:10]>>. Even returning this from a function is a no-go :)

abbec avatar Nov 23 '21 20:11 abbec

Interceptors can be made using named types by implementing the Interceptor trait. There are a few examples here. It's not the most ergonomic thing ever but at least its possible and can be cleaned up by defining your own type alias.

davidpdrsn avatar Nov 23 '21 20:11 davidpdrsn

Sure, but that will only get rid of the closure part so it is (as you said) still not really "ergonomic" :)

abbec avatar Nov 23 '21 20:11 abbec

In 2023 I don't think much has changed in terms of making naming client types any easier. Is there any interest in having the generated client implement the generated service trait?

If so, I'm happy to contribute.

domesticsimian avatar Sep 12 '23 16:09 domesticsimian

I'm running into this while trying to use Tower middleware. Using a service builder means that I now have a pretty deeply nested type instead of a channel. The following example spits outs a Timeout<InterceptedService<InterceptedService<Channel, {[email protected]:140:44}>, ...>>>.

    let channel = ServiceBuilder::new()
        .timeout(Duration::from_secs(30))
        .layer(tonic::service::interceptor(my_cool_interceptor(
            ctx.clone(),
        )))
        .layer(tonic::service::interceptor(|req| Ok(req)))
        .service(channel);

Some (generated?) documentation about the bounds to include when passing a client to a fn would be awesome.

blinsay avatar Feb 13 '24 21:02 blinsay

Just chiming in, I'm also hitting this issue in the context of trying to store the tonic client inside of my own struct.

pub struct SchemaClient<T> {
	inner: SchemaServiceClient<T>
}

I then use a tower ServiceBuilder to setup the channel. I want to be able to configure some layers to be added/removed based on on some configs, but changing the layers seems to change my T which makes it really hard to store one of these things.

a10y avatar Mar 13 '24 14:03 a10y