Skip to content

Minor 'invalid code' error output suggestion #7174

Closed
@curly-brace-butler

Description

@curly-brace-butler

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:

type ExampleRequest struct {
	Body       []byte     `json:"body"`
	StatusCode codes.Code `json:"statusCode"`
	Delay      int        `json:"delay"` // response delay in ms
}

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):

	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.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions