Client always generates content-type: application/grpc & server not recognizing a combined content-type header

jetaggart commented 2 years ago

Bug Report

Version

0.7.2

Platform

wasm-unknown-unknown (browser)

Crates

tonic-web (tonic-web-wasm-client)

Description

I'm using tonic on the backend. The created client mergers the content-type together creating a header that tonic-web does not expect.

For example, tonic-web expects one of these 4 content-types:

https://github.com/hyperium/tonic/blob/8084f4ea26cccf9bd2d96d2a81eaea490aaf603b/tonic-web/src/service.rs#L29-L32

However, the header application/grpc gets combined with the headers added by this client application/grpc-web+proto. This results in a 400:

400 Bad Request
panicked at 'called `Result::unwrap()` on an `Err` value: Status { code: Unknown, message: "missing content-type header in grpc response", source: Some(MissingContentTypeHeader) }'

The header that is generated by this client looks like this:

content-type: application/grpc-web+proto, application/grpc

If I modify these lines: https://github.com/devashishdxt/tonic-web-wasm-client/blob/main/src/client.rs#L62-L64 to be this:

    for (header_name, header_value) in req.headers().iter() {
        if header_name.as_str() != "content-type" {
            builder = builder.header(header_name.as_str(), header_value.to_str()?);
        }
    }

it all works. I'm not sure where the application/grpc is getting attached, I think it's on the tonic client side. Not sure exactly what the resolution here is either.

I'm not sure why the client is attaching that header and what the correct fix here is. Should the server accept combined content-type headers? Should the client not attach headers by default, especially if the transport feature is disabled?

Here is the linked issue to the wasm client: https://github.com/devashishdxt/tonic-web-wasm-client/issues/3

jetaggart commented 2 years ago

This is fixed in the related library, however, I think it's slightly incorrect considering the header is application/grpc, which is a lie, vs. application/grpc+web. Can the tonic client to either 1) allow overriding the headers for grpc-web 2) the server supports a concatenated list, for example: application/gprc, application/grpc-web. Fwiw, other gRPC servers (i.e. go) support concatenated lists

LucioFranco commented 2 years ago

So historically, grpc-go only worked with application/grpc and if you used +proto etc things just broke. I found this out the hard way when working on the gcp example. For this reason, tonic itself only sets that simpler content type. That said, if you use tower layers you should be able to override it pretty easily via ServiceBuilder::map_request/map_response and just edit the http header to your liking.

hyperium / tonic