[YARPCERRORS] Why do we use `code.String()` instead of `.Name()`?

[unscrew]

Why do we use code.String() instead of .Name() for yarpcerror status? The name field is not deprecated according to the comments on the Name() function, but found that the yarpcerrors is using code.String() and .Name() mixed.

// Status represents a YARPC error.
type Status struct {
    code    Code
    name    string
    err     error
    details []byte
}

I was looking into our service logs, and found that we are mapping error_name to error_code and leaving out the error_name field.

The author of our internal error probably referred to this yarpc Error(). This is used often but isn't clear to clients using the library as it still returns the name if it's not nil. This might have been for backward compatibility but confusing to the clients.

// Error implements the error interface.
func (s *Status) Error() string {
    buffer := bytes.NewBuffer(nil)
    _, _ = buffer.WriteString(`code:`)
    _, _ = buffer.WriteString(s.code.String())
    if s.name != "" {
        _, _ = buffer.WriteString(` name:`)
        _, _ = buffer.WriteString(s.name)
    }
    if s.err != nil && s.err.Error() != "" {
        _, _ = buffer.WriteString(` message:`)
        _, _ = buffer.WriteString(s.err.Error())
    }
    return buffer.String()
}

I was making a change for a pull request (maybe it's hard to change the existing behavior due to many unit/integration tests using the current version) but I would like to understand why we started to use code.String() instead of .Name() before fixing all the test assertions.

// Name returns the name of the error for this Status.
//
// It may be customized by using WithName.
func (s *Status) Name() string {
    if s == nil {
        return ""
    }
    if s.name != "" {
        return s.name
    }
    return s.code.String()
}

[rabbbit]

I haven't looked in depth, but https://github.com/yarpc/yarpc-go/commit/33f018e504b97d682f0d62e5359f4b16484a9e94 suggests that Name is indeed deprecated?

[unscrew]

@rabbbit I think that's only that method WithName() but Name() method does not mention it's deprecated.

[AllenLuUber]

I think it is a miss on the original commit https://github.com/yarpc/yarpc-go/commit/33f018e504b97d682f0d62e5359f4b16484a9e94. But the only way to set the value returned from Name() is via WithName(s name) in the first place.

I think the Name() itself allows you to have any types of named error. And code is making it very restricted that you can only have a smaller set of all the codes from gRPC. I think the idea here is, as a client who gets this error, you should NOT care the name but the status/code. Just like in HTTP, you would usually check the responseCode first before you parse/access the body.

[unscrew]

@AllenLuUber Got it, thanks. Then I'll replace error.Name() with error.Code().string() in our codebase. Thanks!