Add example of addings details to errors #1915
Conversation
|
This is an update of #506 using |
There was a problem hiding this comment.
Thanks, this is great!
I think the "usage" part of the doc should ideally go into the Documentation directory under grpc-go. We don't have any documentation about errors/statuses yet, but a single doc to cover the basics and also adding details would be great. If you have time, please expand on this and add something there. Otherwise, we can leave it here for now and file a bug to make sure we eventually cover errors in the main docs and move this section into it.
examples/error_metadata/README.md
Outdated
| Output: | ||
|
|
||
| ``` | ||
| 2018/03/12 19:39:35 ResourceInfo details: violations:<subject:"name:world" description:"Limit one greeting per person" > |
There was a problem hiding this comment.
"ResourceInfo"? I think the code has been changed since this was generated.
There was a problem hiding this comment.
Good catch. I'll fix this as well.
| if err != nil { | ||
| s := status.Convert(err) | ||
| for _, d := range s.Details() { | ||
| info := d.(*epb.QuotaFailure) |
There was a problem hiding this comment.
Better practice would be to check ", ok" of the type assertion, or do a type switch:
switch info := d.(type) {
case *epb.QuotaFailure:
log.Printf("Error details: %s", info)
}
// case ....
}(Interestingly, though, the type switch isn't necessary if you're just logging it.)
There was a problem hiding this comment.
Good point. I'll update the example to include the ok on the type assertion.
|
@dfawley Thank you for the review. I'll expand on the usage as suggested and move the addition to the |
|
@dfawley I've made the discussed updates. Let me know if you would like me to expand on the documentation. |
Documentation/error-details.md
Outdated
|
|
||
| gRPC servers return a single `error` in response to client requests. The error | ||
| value contains a code and a description. In some cases, though, it may be | ||
| necessary to add details about the particular error. gRPC includes a |
There was a problem hiding this comment.
This makes it sound like gRPC has status.Status only because of the details. All servers must return errors implemented by the status package or else clients will only see unknown status codes.
Would you mind renaming this file to rpc-status.md or rpc-errors.md and either:
- Add something to the effect of: All service method handlers should return
nilor status errors (or be coerced to UNKNOWN); clients will see those ~directly. Maybe an example of how to use it (status.Error(codes.Whatever, "description")) as well. - Leave a placeholder for us to fill in later.
Either way, "Error Details" should ideally be just a sub-section of a bigger "Errors" doc.
There was a problem hiding this comment.
I think rpc-errors.md sounds good. I'll make the changes and update the PR, adding an initial, general discussion of errors.
|
@dfawley There are likely additional improvements to the wording. What do you think? |
Fixes #478