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

Add sample code for setting and reading error details #506

Closed
wants to merge 1 commit into from
Closed

Add sample code for setting and reading error details #506

wants to merge 1 commit into from

Conversation

kelseyhightower
Copy link

Fixes #478.


// Contact the server and capture the trailer metadata in md using a call option.
r, err := c.SayHello(context.Background(), &pb.HelloRequest{Name: name}, grpc.Trailer(&md))
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing I was trying to figure out is that:

is this error a rpc error from server side or this is a client-side/network error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the best way to check that would be to test the error code:

grpc.Code(err) != codes.Unknown

From the docs:

Code returns the error code for err if it was produced by the rpc system. Otherwise, it returns codes.Unknown.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I am doing. However, in the doc it also says https://github.com/grpc/grpc-go/blob/master/codes/codes.go#L53.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Network errors should be Unavailable IIRC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking error code is not right way to differentiate client and server side errors. The common practice is that the server application need pack a bit more info into the error description (e.g., serialized error protobuf).

@xiang90
Copy link
Contributor

xiang90 commented Jan 28, 2016

@kelseyhightower Thanks! This is what I wanted to add after I tried to understand errors and had to read the grpc code a little bit.

@jcanizales
Copy link

CC: @iamqizhao for review

@jcanizales
Copy link

The example and the tutorial text look great to me, and the way it's done in Go is refreshingly easy. I'm not fluent enough to review the Go code in detail, though.

If it's not too much to ask, it would be ideal if one of the standard error-detail protos were sent in the trailers (for example, QuotaFailure). They are what Google APIs use, so the situation of a client app having to read one of them is going to be common. And is one step more complicated than sending and receiving text trailers.

@kelseyhightower
Copy link
Author

@jcanizales I maybe missing something, but it seems I can only set the trailers once using grpc.SetTrailer. This function only takes a metadata type, which supports basic string key/value pairs.

I'm not sure where to return the QuotaFailure message. Again, I maybe overlooking something.

@jcanizales
Copy link

Oh, interesting! @iamqizhao, how does one set binary headers/trailers in the Go library?

@kelseyhightower, this is exactly the kind of things I expect we'll find as we create these samples across all languages :)

@iamqizhao
Copy link
Contributor

There is nothing special for setting binary value. You only need to make sure the key has a "-bin" suffix.

// SayHello implements helloworld.GreeterServer
func (s *server) SayHello(ctx context.Context, in *pb.HelloRequest) (*pb.HelloReply, error) {
// Track the number of times the user has been greeted.
s.count[in.Name] += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the goal of this example is to demonstrate how to set and read error details. I am wondering if involving metadata handling makes it over complicated.

BTW, you need mutex to guard the access of s.count. It is racy if you have multiple concurrent clients.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: metadata, see grpc/grpc#4543, whose Go version this PR is trying to solve.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Then I need think through how this can be achieved in a nice way.

@jcanizales
Copy link

What's the function to serialize a proto message into a string? (I've never used Go protos) Does string accept \0 bytes in the middle?

@dfawley
Copy link
Member

dfawley commented Apr 28, 2017

Error details may now be transmitted using status.FromProto instead of trailer metadata, and received via status.FromError().Proto().Details. An example for this could be added to the route_guide example, possibly by returning an error instead of an unnamed feature when nothing is found here: https://github.com/grpc/grpc-go/blob/master/examples/route_guide/server/server.go#L82.

@dfawley dfawley closed this Apr 28, 2017
@jcanizales
Copy link

using status.FromProto instead of trailer metadata

To be clear: On the wire, they'll be sent as trailer metadata, correct? It's important that all languages interoperate in this respect.

@dfawley
Copy link
Member

dfawley commented May 1, 2017

On the wire, it's proto-serialized and sent with the trailer metadata as "grpc-status-details-bin". It's not available through the metadata handed back to the user, however -- it's returned to the client in the error.
I believe support for this is available in all languages now.

@jcanizales
Copy link

What's the type of the proto sent there?

And can we confirm that grpc-status-details-bin is the trailer key used by all languages?

@dfawley
Copy link
Member

dfawley commented May 1, 2017

It's a google.rpc.Status proto:

https://github.com/grpc/grpc-go/blob/master/status/status.go#L131
https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto#L80

The C and Java repos have also implemented this:

https://github.com/grpc/grpc/blob/master/include/grpc%2B%2B/impl/codegen/call.h#L67
https://github.com/grpc/grpc-java/blob/master/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java:

I believe their implementations do not create or parse that field as a proto by default, to avoid the proto dependency. They have libraries to create/parse the field instead. But in Go, grpc already has a dependency on proto, and status is generally handled differently, so it made more sense to implement it through the errors returned by the library.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants