Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to use tonic-web (0.12.3) while multiplexing with axum (0.7.7) without using the deprecated Router::into_router()? #1964

Open
repnop opened this issue Sep 30, 2024 · 5 comments

Comments

@repnop
Copy link

repnop commented Sep 30, 2024

Bug Report

Version

axum = { version = "0.7.7", features = ["macros"] }
hyper = { version = "1.4.1", features = ["http1", "http2", "server"] }
tonic = "0.12.3"
tonic-web = "0.12.3"
tower = { version = "0.5.1", features = ["steer"] }
tower-http = { version = "0.6.1", features = ["cors", "trace"] }

Platform

N/A

Crates

tonic = "0.12.3"
tonic-web = "0.12.3"

Description

I'm working on upgrading some of my web services which use both gRPC (via tonic) and regular REST endpoints (via axum) and cannot find a way to implement the GrpcWebLayer without using the deprecated tonic::transport::server::Router::into_router() method because of mismatched body types.

Compiles:

let grpc = tonic::transport::Server::builder()
    .accept_http1(true)
    .layer(GrpcWebLayer::new())
    .add_service(NodeMapServer::new(NodeMapServerImpl::new(databases)))
    .into_router();

let service =
    tower::steer::Steer::new([rest, grpc], |req: &hyper::Request<_>, _services: &[_]| {
        if req
            .headers()
            .get(hyper::header::CONTENT_TYPE)
            .map(|content_type| content_type.as_bytes())
            .filter(|content_type| content_type.starts_with(b"application/grpc"))
            .is_some()
        {
            // route to the gRPC service (second service element) when the
            // header is set
            1
        } else {
            // otherwise route to the REST service
            0
        }
    });

let service = tower::ServiceBuilder::new()
    .layer(
        tower_http::cors::CorsLayer::new()
            .allow_origin(tower_http::cors::Any)
            .allow_headers(tower_http::cors::Any)
            .expose_headers(tower_http::cors::Any),
    )
    .service(service);

axum::serve(listener, tower::make::Shared::new(service))
    .with_graceful_shutdown(async move {
        let _ = shutdown_rx.changed().await;
    })
    .await?;

Does not compile:

let grpc = tonic::service::Routes::new(NodeMapServer::new(NodeMapServerImpl::new(databases)))
        .prepare()
        .into_axum_router();

let service =
    tower::steer::Steer::new([rest, grpc], |req: &hyper::Request<_>, _services: &[_]| {
        if req
            .headers()
            .get(hyper::header::CONTENT_TYPE)
            .map(|content_type| content_type.as_bytes())
            .filter(|content_type| content_type.starts_with(b"application/grpc"))
            .is_some()
        {
            // route to the gRPC service (second service element) when the
            // header is set
            1
        } else {
            // otherwise route to the REST service
            0
        }
    });

let service = tower::ServiceBuilder::new()
    .layer(
        tower_http::cors::CorsLayer::new()
            .allow_origin(tower_http::cors::Any)
            .allow_headers(tower_http::cors::Any)
            .expose_headers(tower_http::cors::Any),
    )
    // is this even the right place to do this?
    .layer(GrpcWebLayer::new())
    .service(service);

axum::serve(listener, tower::make::Shared::new(service))
    .with_graceful_shutdown(async move {
        let _ = shutdown_rx.changed().await;
    })
    .await?;
error[E0271]: type mismatch resolving `<Steer<Router, {closure@main.rs:197:48}, Request<UnsyncBoxBody<Bytes, Status>>> as Service<Request<UnsyncBoxBody<Bytes, Status>>>>::Response == Response<UnsyncBoxBody<Bytes, Status>>`
   --> src/main.rs:222:10
    |
222 |         .service(service);
    |          ^^^^^^^ expected `Response<Body>`, found `Response<UnsyncBoxBody<Bytes, Status>>`
    |
    = note: expected struct `hyper::Response<AxumBody>`
               found struct `hyper::Response<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>`
    = note: required for `GrpcWebLayer` to implement `tower::Layer<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>`
    = note: 1 redundant requirement hidden
    = note: required for `Stack<GrpcWebLayer, Stack<CorsLayer, tower::layer::util::Identity>>` to implement `tower::Layer<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>`

error[E0271]: type mismatch resolving `<Steer<Router, {closure@main.rs:197:48}, Request<UnsyncBoxBody<Bytes, Status>>> as Service<Request<UnsyncBoxBody<Bytes, Status>>>>::Response == Response<UnsyncBoxBody<Bytes, Status>>`
   --> src/main.rs:222:18
    |
222 |         .service(service);
    |          ------- ^^^^^^^ expected `Response<UnsyncBoxBody<Bytes, Status>>`, found `Response<Body>`
    |          |
    |          required by a bound introduced by this call
    |
    = note: expected struct `hyper::Response<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>`
               found struct `hyper::Response<AxumBody>`
    = note: required for `GrpcWebLayer` to implement `tower::Layer<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>`
    = note: 1 redundant requirement hidden
    = note: required for `Stack<GrpcWebLayer, Stack<CorsLayer, tower::layer::util::Identity>>` to implement `tower::Layer<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>`

error[E0277]: the trait bound `GrpcWebService<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>: tower::Service<hyper::Request<AxumBody>>` is not satisfied
   --> src/main.rs:241:5
    |
241 |     axum::serve(listener, tower::make::Shared::new(service))
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `tower::Service<hyper::Request<AxumBody>>` is not implemented for `GrpcWebService<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>`, which is required by `Cors<GrpcWebService<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>>: tower::Service<hyper::Request<AxumBody>>`
    |
    = help: the trait `tower::Service<hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>` is implemented for `GrpcWebService<S>`
    = note: required for `Cors<GrpcWebService<Steer<AxumRouter, {closure@src/main.rs:197:48: 197:90}, hyper::Request<http_body_util::combinators::box_body::UnsyncBoxBody<axum::body::Bytes, tonic::Status>>>>>` to implement `tower::Service<hyper::Request<AxumBody>>`

am I missing something obvious? I'd like to not use the deprecated method if possible, but it doesn't seem possible to do so otherwise. is there a workaround available at this point or do I need for tonic-web to be changed to be generic over the body type? (#1361)

@repnop
Copy link
Author

repnop commented Oct 2, 2024

okay so it looks like the following works, albeit a little awkward:

tonic::service::Routes::new(GrpcWebLayer::new().layer(NodeMapServer::new(NodeMapServerImpl::new(databases))))
    .prepare()
    .into_axum_router()
    // layering etc

in case anyone hits this issue as well. it would still be nice to use GrpcWebLayer in the same way its provided in examples with the intended usage, but this workaround seems to work for the time being. not sure if anyone would want me to rename or close this issue, since its basically a duplicate of #1361 more or less?

@mbfm
Copy link

mbfm commented Nov 29, 2024

I've run into a similar problem, where I'm not sure, if it is possible to make my code work without using the deprecated method.

@tottoto Could you maybe take a quick look?
You deprecated the method in #1775, suggesting that an alternative should be trivially available via Routes.

But there doesn't seem to be a way to convert from tonic::transport::server::Router to Routes, as far as I can tell.

I'm guessing, the appropriate solution would be an implementation of the From trait, like you added one for axum::Router in #1863.

The implementation would probably look like this:

impl From<tonic::transport::server::Router> for Routes {
    fn from(router: tonic::transport::server::Router) -> Self {
        router.routes
    }
}

@tottoto
Copy link
Collaborator

tottoto commented Nov 30, 2024

There isn't need to get Routes via Router. It can be built directly.

@mbfm
Copy link

mbfm commented Dec 2, 2024

Ah, thanks, that makes more sense now.

For anyone else stumbling upon this, I was able to change our code from this structure:

let axum_router = tonic::transport::Server::builder()
    .layer(async_interceptor(move |request| {
        //...
    }))
    .accept_http1(true) //gRPC-Web uses HTTP1
    .add_service(service1)
    .add_service(service2)
    .into_router();

To this:

let mut routes_builder = Routes::builder();

routes_builder
    .add_service(service1)
    .add_service(service2);

let axum_router = routes_builder
    .routes()
    .into_axum_router()
    .layer(async_interceptor(move |request| {
        //...
    }));

I don't know where one would specify the .accept_http1(true) for gRPC-Web, but in our application, we don't actually need it, since we're using TLS, as described here: https://docs.rs/tonic-web/latest/tonic_web/#enabling-tonic-services
I'm guessing, it never actually did anything to begin with, since this information probably got dropped when converting to a router.

@stevehayles
Copy link

stevehayles commented Dec 12, 2024

In a situation where you might need the .accept_http1(true) (for instance in an gRPC-Web application) I think the original code can be changed from

let axum_router = tonic::transport::Server::builder()
    .layer(async_interceptor(move |request| {
        //...
    }))
    .accept_http1(true) //gRPC-Web uses HTTP1
    .add_service(service1)
    .add_service(service2)
    .into_router();

to

let axum_router = tonic::transport::Server::builder()
    .layer(async_interceptor(move |request| {
        //...
    }))
    .accept_http1(true)  //gRPC-Web uses HTTP1
    .add_service(service1)
    .add_service(service2)
    .into_service()
    .into_axum_router();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants