gax-go: APIError.GRPCStatus returns "OK" for all http errors.
When using apierror.FromError with an http error, the status field does not get populated, so the GRPCStatus() method returns nil. It seems like this is intended to mean “no status is available”. However, the status package treats nil as “OK”:
https://github.com/grpc/grpc-go/blob/dba26e15a07f43875ccf806a2dd6cbcbc1c12eab/internal/status/status.go#L72
The status package also says that no non-nil error should have status of “OK”.
To properly fulfil this contract, it seems that the APIError.status should never be nil. I would expect it to at least return UNKNOWN, but it looks like the json error may also include a status field with a grpc status code, and some of the http status code seem to map directly to grpc codes (e.g. 404, 429, 503):
https://cloud.google.com/apis/design/errors#handling_errors
About this issue
- Original URL
- State: open
- Created a year ago
- Comments: 19 (11 by maintainers)
Commits related to this issue
- fix(v2/apierror): return Unknown GRPCStatus when err source is HTTP * return Unknown status when googleapi.Error because gRPC treats nil as OK closes: #254 — committed to quartzmo/gax-go by quartzmo a year ago
- fix(v2/apierror): return Unknown GRPCStatus when err source is HTTP (#260) * return Unknown status when googleapi.Error because gRPC treats nil as OK closes: #254 — committed to googleapis/gax-go by quartzmo a year ago
Yes.
I just want to mention what has been the most confusing thing for us. When we use the classroom api, we see errors that look a lot like grpc errors. For example, they say “permission denied” instead of “forbidden”. But then we can’t handle them using the concise
if status.Code(err) == codes.PermissionDenied. Instead we have to remember to cast them to a googleapi.Error and check forhttp.StatusForbidden. Or for example right now we’re getting errors in the logs that say failed precondition. I don’t even know how we’re supposed to handle that in our go code because the http code 400 is used for multiple different errors. Maybe we have to look at the error details? It would certainly be a lot easier ifstatus.Codejust returnedcodes.FailedPrecondition.