Return APIErrors directly instead of as a stringified error#22
Return APIErrors directly instead of as a stringified error#22
Conversation
|
|
||
| // Check for any errors returned by the API | ||
| if err := r.Errors.ToError(); err != nil { | ||
| return nil, nil, nil, nil, &APIError{e: fmt.Errorf("unexpected error during enrollment: %v", err), ReqID: reqID} |
There was a problem hiding this comment.
Looks like we're no longer capturing the request ID. We log this in mobile_nebula. I think we either need to come up with a solution to continue returning it in APIErrors, or maybe add it to every element in the []APIError (and add it to APIError), .... or just take the easier approach of returning a specific code here (ErrInvalidCode), which will also mean that callers don't all have to parse this themselves.
There was a problem hiding this comment.
Wouldn't that lose information if the api server were to return an error other than "invalid code"?
There was a problem hiding this comment.
How do you think we might be able to avoid that, and still return an explicit error for invalid codes here? How would the caller do it?
There was a problem hiding this comment.
I don't think the server ever returns invalid code along with an additional error. If it did, the caller might want to know that information. So I think what I would do is only return ErrInvalidCode if that's the only error. If there are multiple errors, leave the return as it was before.
client.go
Outdated
| return "invalid credentials" | ||
| } | ||
| var InvalidCredentialsError = fmt.Errorf("invalid credentials") | ||
| var InvalidCodeError = fmt.Errorf("invalid enrollment code") |
There was a problem hiding this comment.
i know you were just following the existing code, but these should be in the format ErrInvalidCredentials and ErrInvalidCode.
Since we'll have to update callers anyway (errors.As -> errors.Is for InvalidCredentialsError), I think we should fix the names now.
jasikpark
force-pushed
the
change-api-errors-type
branch
from
93ce8c7 to
d3a1c74
Compare
2 participants
This alters
ToError() errorto beError() stringto allowAPIErrorsto implement the Error interface.