Storing `with_interceptor` clients is substantially more challenging in 0.5

[Sushisource]

Feature Request

Crates

tonic

Motivation

Previously, I could simply store a FooServiceClient<tonic::transport::Channel> without the need for generics or boxing in a struct, and use that. I do so to encapsulate access to the client, partly for mocking reasons, partly for enforcing certain invariants or making the API simpler.

Now, the type is something like FooServiceClient<tonic::codegen::InterceptedService<Channel, impl Fn<(tonic::Request<()>,)>>> which is a much bigger pain to store.

Proposal

Kind of like #671, but not quite, it would be nice if there was simply a FooServiceClient trait with all the rpc methods on it that the generated structs would implement (using async_trait, maybe opt-in). This would make storage and mocking much simpler.

Alternatives

Beyond having a full client trait, a trait that combines all the bounds in the big where clause on generated clients would also make this much easier. I'm referencing:

    impl<T> FooServiceClient<T>
    where
        T: tonic::client::GrpcService<tonic::body::BoxBody>,
        T::ResponseBody: Body + Send + Sync + 'static,
        T::Error: Into<StdError>,
        <T::ResponseBody as Body>::Error: Into<StdError> + Send,

[davidpdrsn]

I think the right solution for this is #713. Then you can define interceptors as nameable types without using closures.

[Sushisource]

I think the right solution for this is #713. Then you can define interceptors as nameable types without using closures.

Yeah that looks like a great way to solve the problem.

[mattoni]

What's the correct way to use #713 to have a client implementation as a struct field?

[yotamofek]

What's the correct way to use #713 to have a client implementation as a struct field?

@mattoni you mean something like this?

struct MyInterceptor {}

impl Interceptor for MyInterceptor {
    fn call(&mut self, request: tonic::Request<()>) -> Result<tonic::Request<()>, Status> {
        todo();
    }
}

struct ClientWrapper<S: Service> {
    client: InterceptedService<S, MyInterceptor>,
}

[davidpdrsn]

This was fixed in #713 and released as part of tonic 0.5.1. I also added some examples here https://github.com/hyperium/tonic/blob/master/examples/src/interceptor/client.rs. Let me know if you have more questions!

[mattoni]

Awesome, thanks for the examples @davidpdrsn . One quick question, I noticed you're passing the struct in exactly here:

  let client: GreeterClient<InterceptedService<Channel, MyInterceptor>> =
      GreeterClient::with_interceptor(channel, MyInterceptor);

However, when attempting that I get the following:

I've rebuilt with the latest tonic-build: 0.5.1 and still get the same thing. The generated service signature is this:

and my AuthService is implemented as this:

How can I get it to follow your example?

Thanks!

[davidpdrsn]

Are you certain you've updated to tonic 0.5.1 (not tonic-build)? It looks like you're still on 0.5.0.

[mattoni]

Looks like my old build artifacts were still cached even though I had done a cargo clean? Anyway, working now. Cheers!

[mattoni]

Actually I spoke too soon. I switched to using the git dependency directly and it's still having the same issue in the generated code. The actual interceptor definition looks correct, but generated params are still wrong:

here are the deps I have listed. Did a cargo clean and a fresh cargo build and got the error.

[davidpdrsn]

No idea whats wrong. The code on GitHub looks correct.

[davidpdrsn]

Are you able to reproduce it in a smaller example? If so, can make a repo with the code, then I'll take a look.

[mattoni]

So I found the issue - I set tonic-build to the git repo version and it worked. looks like 0.5.1 generates the wrong code.