gRPC Http-Json transcoding throws 500 when supported serialization format does not include JSON.

[Dogacel]

When I enable transcoding it Http and use it such as,

.enableHttpJsonTranscoding(true)
.supportedSerializationFormats(GrpcSerializationFormats.PROTO)

it returns

Status: 500
Description: Internal Server Error

With little to no information. Taking a closer look, I realized in the debugger my content type was updated as application/grpc+json without asking me. Even though I used Content-type: application/json in my curl request. Such as

curl 'localhost:9027/sampleEndpoint' -H "Content-type: application/json; charset=utf-8"

Thus underlying issue became:

 A gRPC response must have the grpc-status header. response: DefaultAggregatedHttpResponse{headers=[:status=415, content-type=text/plain; charset=utf-8, content-length=39], content={39B, hex=4d697373696e67206f7220696e76616c}}

The solution was to add

.supportedSerializationFormats(GrpcSerializationFormats.JSON, GrpcSerializationFormats.PROTO)

And it was really tough for us to debug this. I think the response should be something more obvious , if not possible at least we should warn users during startup that transcoding won't work.

I still don't fully understand how my content-type header changed magically under the hood.

[jrhee17]

And it was really tough for us to debug this. I think the response should be something more obvious , if not possible at least we should warn users during startup that transcoding won't work.

I think adding validation in GrpcServiceBuilder is a good idea 👍 We can probably throw an ISE at GrpcServiceBuilder#build.

[Dogacel]

And it was really tough for us to debug this. I think the response should be something more obvious , if not possible at least we should warn users during startup that transcoding won't work.

I think adding validation in GrpcServiceBuilder is a good idea 👍 We can probably throw an ISE at GrpcServiceBuilder#build.

I think ISE makes sense overall but it can be a breaking change for some people. I am not sure what people would think if their service starts to throw exceptions during startup after a version upgrade 😓 That's why I suggested a warning and maybe we can completely disable the transcoding if JSON is not enabled so it just throws like 404 and stuff.

What are your thoughts?

[jrhee17]

I am not sure what people would think if their service starts to throw exceptions during startup after a version upgrade 😓

Although it can be a somewhat breaking change, I don't think this is very hard to migrate and throwing an exception is clearer. I still prefer throwing an exception, but any other thoughts are welcome. @trustin @minwoox @ikhoon

[ikhoon]

Agreed. Since it is a kind of defect, it is a bug patch and not a breaking change.