Metadata error

[SebastianThiebaud]

I was wondering if we were planning on supporting https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto

We came across this talk: https://schd.ws/hosted_files/kccncna17/ad/2017%20CloudNativeCon%20-%20Mod%20gRPC%20Services.pdf

[MrMage]

How exactly would such support go further than what we have already? We already have CallResult/ServerStatus fulfilling similar purposes (except that they have Metadata instead of details), and any RPC service can be declared to return a message of type Status if desired.

Also, what do you mean with the issue title?

[SebastianThiebaud]

CallResult statusMessage doesn't seem to work properly on my end. Investigating the issue, I see observers.c line 129 that the server_details used for statusMessage is set to be an empty string. Am I wrong here?

[MrMage]

How are you using this? My RPCs return custom status messages and my client code receives those messages just fine. Please check eg ServerThrowingTests if I remember correctly for an example.

[MrMage]

@SebastianThiebaud do you have any updates on this, or can I close the issue?

[SebastianThiebaud]

Sorry for the delay. I've been looking at this and the main issue is that as described in the design guide, Status should contain a details attribute with a type Any. In grpc-swift, details or metadata is designed to be a hashmap. I think we should revisit that and in the world of Protobuf, we should also have a contract for errors.

[MrMage]

Do you have any documentation, examples or proposals on how this is supposed to be done with gRPC? I can't find any details field in https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/Status.java, for example.

Also, keep in mind that we would need to transfer those details in a standardized way via the gRPC protocol, and I am not aware of any provisions for that in the protocol (the document you are linking to doesn't seem to describe any). And if gRPC says "you should put such information into the metadata field", then I think we should adhere to that.

[parthmehta209]

Hey.. Just helping out Sebastian with searching for examples of other libraries using the the array of protobuf Any type in the details.

Examples from grpc-java:
This is a method that will convert a throwable to a status.proto object in java:
https://github.com/grpc/grpc-java/blob/81da3eb95be37fa0647ce8da2e19de96ab84c600/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java#L134

It uses the metadata key 'grpc-status-details-bin' mentioned here:
https://github.com/grpc/grpc-java/blob/81da3eb95be37fa0647ce8da2e19de96ab84c600/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java#L35

[parthmehta209]

Looking for golang and python examples now.. Will update in a bit.

[parthmehta209]

golang:
example of parsing error to a status.proto object:
https://github.com/grpc/grpc-go/blob/master/status/status.go#L125

example of details being extracted out of the status:
https://github.com/grpc/grpc-go/blob/master/status/status.go#L162

Example of details being added to a status object:
https://github.com/grpc/grpc-go/blob/master/status/status.go#L144

This is how golang decodes the header 'grpc-status-details-bin' into a status.proto object:
https://github.com/grpc/grpc-go/blob/339b6cb107199486c3b352f70db9e977191be8b7/internal/transport/http_util.go#L316

[MrMage]

Thanks for the info! I would be open to offering an optional method to extract such a proto out of a Metadata object (probably by reading the grpc-status-details-bin key you mentioned and serializing it into a proto), but that should not be mandatory nor happen automatically (i.e. without being explicitly requested by the consumer of SwiftGRPC). If you have any proposals on implementing that, happy to take a look :-)

[SebastianThiebaud]

Works for me.

[parthmehta209]

Hi @MrMage, @SebastianThiebaud any updates on this. Is there a plan on making the changes that @MrMage described earlier?

[MrMage]

@SebastianThiebaud could you please elaborate what you mean with that?

[SebastianThiebaud]

@parthmehta209 @MrMage please review #331

I know it's not perfect but it gets the work done. I don't think hosting the generated Protobuf code of Status is an issue as it should not change anytime soon.