You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
if ci, err := strconv.ParseUint(string(b), 10, 32); err == nil {
if ci >= _maxCode {
return fmt.Errorf("invalid code: %q", ci)
}
*c = Code(ci)
return nil
}
After strconv.ParseUint is successful, we've decided to treat "ci" as an unsigned base-10 integer. So, I think we should output in the error message using %d. That way, if someone tries to unmarshal 200, 17, etc., it will show up more clearly in the error logs.
In the branch reached if ParseUint err != nil, maybe it still makes sense to use %q, since we've not made a decision on how to interpret the byte[] value.
Maybe it is a standard best practice to use %q in this manner and the appearance of a random Unicode rune in error output suggests just generically "bad bytes" without committing to a particular datatype/interpretation. But, I did feel this error message was a little misleading. The phrasing "invalid code" didn't indicate the issue had originated from "gRPC codes" and when working in the context of JSON unmarshalling the generic term "code" could relate to any number of issues in converting data to and from byte-level and textual representations. It doesn't really point one to gRPC codes as a specific starting place for debugging.
The text was updated successfully, but these errors were encountered:
What version of gRPC are you using?
google.golang.org/grpc v1.63.2
What version of Go are you using (
go version
)?go1.20.10 darwin/arm64
What operating system (Linux, Windows, …) and version?
macOS
What did you do?
Attempted to call standard json.Unmarshal on a string
{"body":"redactedBase64","statusCode":200,"delay":0}
to unmarshall it into a struct defined:What did you expect to see?
invalid code: 200
What did you see instead?
invalid code: 'È'
Suggestion / Question
In the implementation (https://github.com/grpc/grpc-go/blob/b433b9467d87d70de277ee7e1139ef2ad900bfa4/codes/codes.go):
After strconv.ParseUint is successful, we've decided to treat "ci" as an unsigned base-10 integer. So, I think we should output in the error message using %d. That way, if someone tries to unmarshal 200, 17, etc., it will show up more clearly in the error logs.
In the branch reached if ParseUint err != nil, maybe it still makes sense to use %q, since we've not made a decision on how to interpret the byte[] value.
Maybe it is a standard best practice to use %q in this manner and the appearance of a random Unicode rune in error output suggests just generically "bad bytes" without committing to a particular datatype/interpretation. But, I did feel this error message was a little misleading. The phrasing "invalid code" didn't indicate the issue had originated from "gRPC codes" and when working in the context of JSON unmarshalling the generic term "code" could relate to any number of issues in converting data to and from byte-level and textual representations. It doesn't really point one to gRPC codes as a specific starting place for debugging.
The text was updated successfully, but these errors were encountered: