Support layers in generated server codes.

Hakuyume commented 1 year ago

Feature Request

Crates

tonic, tonic-build

Motivation

Servers generated by tonic-build have with_interceptor method, so that users can inject some layers before underlying service. However, async/await is unavailable in an interceptor.

Although the document and examples suggest to use Server::layer for such demand, there are some limitations.

Users cannot choose services to be wrapped.
Layers cannot access tonic::Request (http::Request is available instead) .

Proposal

Make tonic-build to generate layer method for each server.

Server::builder()
    .add_service(ServerA::new(service_a).layer(MyLayer))
    .add_service(ServerB::new(service_b))
    .serve()
    .await?

struct MyLayer {
   ..
}

impl Layer<S> for  MyLayer {
    type Service = MyService<S>;
    ..
}

struct MyService<S> {
   ..
}

impl<S, T, U> Service<tonic::Request<T>> for MyService<S>
where
    S: Service<tonic::Request<T>, Response = tonic::Response<U>, Error = tonic::Status>,
    ..
{
     type Response = tonic::Response<U>;
     type Error = tonic::Status;
     ..
}

Alternatives

A workaround is to conduct routing and type conversion (http::Request/Response <-> tonic::Request/Response) in layers of Server::layer. This is redundant since generated servers do routing and type conversion in them.

kyle-mccarthy commented 11 months ago

You can use tower's ServiceBuilder for this.

let service_a = ServiceBuilder::new()
    .layer(MyLayer)
    .service(
        MyService::new(...).start()
    );

Server::builder()
    .add_service(service_a)
    .serve()
    .await?

Hakuyume commented 11 months ago

You can use tower's ServiceBuilder for this.

No, ServiceBuilder cannot inject layers that handles tonic::Request and tonic::Response .

I tried with examples/src/helloworld/server.rs

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let addr = "[::1]:50051".parse().unwrap();
    let greeter = MyGreeter::default();

    println!("GreeterServer listening on {}", addr);

    let service = tower::ServiceBuilder::new()
        .layer(MyLayer {})
        .service(GreeterServer::new(greeter));

    Server::builder().add_service(service).serve(addr).await?;

    Ok(())
}

#[derive(Clone)]
struct MyLayer {}

impl<S> tower::Layer<S> for MyLayer {
    type Service = MyService<S>;

    fn layer(&self, inner: S) -> Self::Service {
        MyService { inner }
    }
}

#[derive(Clone)]
struct MyService<S> {
    inner: S,
}

impl<S, T, U> tower::Service<tonic::Request<T>> for MyService<S>
where
    S: tower::Service<tonic::Request<T>, Response = tonic::Response<U>>,
{
    type Response = tonic::Response<U>;
    type Error = S::Error;
    type Future = S::Future;

    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.inner.poll_ready(cx)
    }

    fn call(&mut self, request: tonic::Request<T>) -> Self::Future {
        self.inner.call(request)
    }
}

This code raises the following errors.

error[E0277]: the trait bound `MyService<GreeterServer<MyGreeter>>: Service<request::Request<tonic::transport::Body>>` is not satisfied
   --> examples/src/helloworld/server.rs:41:35
    |
41  |     Server::builder().add_service(service).serve(addr).await?;
    |                       ----------- ^^^^^^^ the trait `Service<request::Request<tonic::transport::Body>>` is not implemented for `MyService<GreeterServer<MyGreeter>>`
    |                       |
    |                       required by a bound introduced by this call
    |
    = help: the trait `Service<tonic::Request<T>>` is implemented for `MyService<S>`
note: required by a bound in `Server::<L>::add_service`
   --> /home/admin/github.com/hyperium/tonic/tonic/src/transport/server/mod.rs:364:12
    |
362 |     pub fn add_service<S>(&mut self, svc: S) -> Router<L>
    |            ----------- required by a bound in this associated function
363 |     where
364 |         S: Service<Request<Body>, Response = Response<BoxBody>, Error = Infallible>
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Server::<L>::add_service`

error[E0277]: the trait bound `MyService<GreeterServer<MyGreeter>>: NamedService` is not satisfied
   --> examples/src/helloworld/server.rs:41:35
    |
41  |     Server::builder().add_service(service).serve(addr).await?;
    |                       ----------- ^^^^^^^ the trait `NamedService` is not implemented for `MyService<GreeterServer<MyGreeter>>`
    |                       |
    |                       required by a bound introduced by this call
    |
    = help: the following other types implement trait `NamedService`:
              GreeterServer<T>
              tonic::service::interceptor::InterceptedService<S, F>
              Either<S, T>
note: required by a bound in `Server::<L>::add_service`
   --> /home/admin/github.com/hyperium/tonic/tonic/src/transport/server/mod.rs:365:15
    |
362 |     pub fn add_service<S>(&mut self, svc: S) -> Router<L>
    |            ----------- required by a bound in this associated function
...
365 |             + NamedService
    |               ^^^^^^^^^^^^ required by this bound in `Server::<L>::add_service`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `examples` (bin "helloworld-server") due to 2 previous errors

kyle-mccarthy commented 11 months ago

Why do you need it to use tonic::Request and tonic::Response? You should be using hyper::Request and hyper::Response. To resolve the error about the missing NamedService bound, you can implement the trait on your layer.

impl<S> Service<hyper::Request<Body>> for MyService<S>
where
    S: Service<hyper::Request<Body>, Response = hyper::Response<BoxBody>> + Clone + Send + 'static,
    S::Future: Send + 'static,
{
    type Response = S::Response;
    type Error = S::Error;
    type Future = BoxFuture<'static, Result<Self::Response, Self::Error>>;

    fn poll_ready(
        &mut self,
        cx: &mut std::task::Context<'_>,
    ) -> std::task::Poll<Result<(), Self::Error>> {
        self.inner.poll_ready(cx)
    }

    fn call(&mut self, mut request: hyper::Request<Body>) -> Self::Future {
        // This is necessary because tonic internally uses `tower::buffer::Buffer`.
        // See https://github.com/tower-rs/tower/issues/547#issuecomment-767629149
        // for details on why this is necessary
        let clone = self.inner.clone();
        let mut inner = std::mem::replace(&mut self.inner, clone);

        Box::pin(async move {
            Ok(inner.call(req).await?)
        })
    }
}

// Required by the server builder when converted into a [tonic::transport::server::Router]
impl<S> NamedService for MyService<S>
where
    S: NamedService,
{
    const NAME: &'static str = S::NAME;
}

There is an example here https://github.com/hyperium/tonic/blob/master/examples/src/tower/server.rs

Hakuyume commented 11 months ago

Why do you need it to use tonic::Request and tonic::Response?

In my case, I'd like to access remote_addr in my layer. tonic::Request also provides some useful fields.

You should be using hyper::Request and hyper::Response. To resolve the error about the missing NamedService bound, you can implement the trait on your layer.

I know I can make my code work as I mentioned in Alternatives above. I'm asking to add a new helper so that I can write it easily.

hyperium / tonic