fix(platform): check for (*platform.Error)(nil) in error functions

As of https://github.com/influxdata/platform/pull/2192 a panic was
introduced in the startup of platform. It is the result of
`Error{Code,Op,Message}` being passed a `(*platform.Error)(nil)`
instead of an `(error)(nil)`.

Specifically
```
   if err == nil {
        return ""
    } else if e, ok := err.(*Error); ok && e.Code != "" {
        return e.Code
    }
```
will panic when err is `(*platform.Error)(nil)` as the `err == nil`
check will fail and the `e, ok := err.(*Error); ok && e.Code != ""` will
succeed and panic on `e.Code`, as `e` is nil.

This panic could have been avoided if all methods returned `error`
instead of `*platform.Error`.
pull/10616/head
Michael Desa 2019-01-04 11:02:26 -05:00
parent 3dacbdf162
commit b590259a97
2 changed files with 73 additions and 17 deletions

View File

@ -85,21 +85,51 @@ func (e *Error) Error() string {
func ErrorCode(err error) string { func ErrorCode(err error) string {
if err == nil { if err == nil {
return "" return ""
} else if e, ok := err.(*Error); ok && e.Code != "" { }
e, ok := err.(*Error)
if !ok {
return EInternal
}
if e == nil {
return ""
}
if e.Code != "" {
return e.Code return e.Code
} else if ok && e.Err != nil { }
if e.Err != nil {
return ErrorCode(e.Err) return ErrorCode(e.Err)
} }
return EInternal return EInternal
} }
// ErrorOp returns the op of the error, if available; otherwise return empty string. // ErrorOp returns the op of the error, if available; otherwise return empty string.
func ErrorOp(err error) string { func ErrorOp(err error) string {
if e, ok := err.(*Error); ok && e.Op != "" { if err == nil {
return ""
}
e, ok := err.(*Error)
if !ok {
return ""
}
if e == nil {
return ""
}
if e.Op != "" {
return e.Op return e.Op
} else if ok && e.Err != nil { }
if e.Err != nil {
return ErrorOp(e.Err) return ErrorOp(e.Err)
} }
return "" return ""
} }
@ -108,11 +138,25 @@ func ErrorOp(err error) string {
func ErrorMessage(err error) string { func ErrorMessage(err error) string {
if err == nil { if err == nil {
return "" return ""
} else if e, ok := err.(*Error); ok && e.Msg != "" { }
e, ok := err.(*Error)
if !ok {
return "An internal error has occurred."
}
if e == nil {
return ""
}
if e.Msg != "" {
return e.Msg return e.Msg
} else if ok && e.Err != nil { }
if e.Err != nil {
return ErrorMessage(e.Err) return ErrorMessage(e.Err)
} }
return "An internal error has occurred." return "An internal error has occurred."
} }

View File

@ -60,7 +60,7 @@ func TestErrorMsg(t *testing.T) {
} }
for _, c := range cases { for _, c := range cases {
if c.msg != c.err.Error() { if c.msg != c.err.Error() {
t.Fatalf("%s failed, want %s, got %s", c.name, c.msg, c.err.Error()) t.Errorf("%s failed, want %s, got %s", c.name, c.msg, c.err.Error())
} }
} }
} }
@ -74,6 +74,10 @@ func TestErrorMessage(t *testing.T) {
{ {
name: "nil error", name: "nil error",
}, },
{
name: "nil error of type *platform.Error",
err: (*platform.Error)(nil),
},
{ {
name: "simple error", name: "simple error",
err: &platform.Error{Msg: "simple error"}, err: &platform.Error{Msg: "simple error"},
@ -92,7 +96,7 @@ func TestErrorMessage(t *testing.T) {
} }
for _, c := range cases { for _, c := range cases {
if result := platform.ErrorMessage(c.err); c.want != result { if result := platform.ErrorMessage(c.err); c.want != result {
t.Fatalf("%s failed, want %s, got %s", c.name, c.want, result) t.Errorf("%s failed, want %s, got %s", c.name, c.want, result)
} }
} }
} }
@ -106,6 +110,10 @@ func TestErrorOp(t *testing.T) {
{ {
name: "nil error", name: "nil error",
}, },
{
name: "nil error of type *platform.Error",
err: (*platform.Error)(nil),
},
{ {
name: "simple error", name: "simple error",
err: &platform.Error{Op: "op1"}, err: &platform.Error{Op: "op1"},
@ -129,7 +137,7 @@ func TestErrorOp(t *testing.T) {
} }
for _, c := range cases { for _, c := range cases {
if result := platform.ErrorOp(c.err); c.want != result { if result := platform.ErrorOp(c.err); c.want != result {
t.Fatalf("%s failed, want %s, got %s", c.name, c.want, result) t.Errorf("%s failed, want %s, got %s", c.name, c.want, result)
} }
} }
} }
@ -142,6 +150,10 @@ func TestErrorCode(t *testing.T) {
{ {
name: "nil error", name: "nil error",
}, },
{
name: "nil error of type *platform.Error",
err: (*platform.Error)(nil),
},
{ {
name: "simple error", name: "simple error",
err: &platform.Error{Code: platform.ENotFound}, err: &platform.Error{Code: platform.ENotFound},
@ -165,7 +177,7 @@ func TestErrorCode(t *testing.T) {
} }
for _, c := range cases { for _, c := range cases {
if result := platform.ErrorCode(c.err); c.want != result { if result := platform.ErrorCode(c.err); c.want != result {
t.Fatalf("%s failed, want %s, got %s", c.name, c.want, result) t.Errorf("%s failed, want %s, got %s", c.name, c.want, result)
} }
} }
} }
@ -237,17 +249,17 @@ func TestJSON(t *testing.T) {
result, err := json.Marshal(c.err) result, err := json.Marshal(c.err)
// encode testing // encode testing
if err != nil { if err != nil {
t.Fatalf("%s encode failed, want err: %v, should be nil", c.name, err) t.Errorf("%s encode failed, want err: %v, should be nil", c.name, err)
} }
if string(result) != c.encoded { if string(result) != c.encoded {
t.Fatalf("%s encode failed, want result: %s, got %s", c.name, c.encoded, string(result)) t.Errorf("%s encode failed, want result: %s, got %s", c.name, c.encoded, string(result))
} }
// decode testing // decode testing
got := new(platform.Error) got := new(platform.Error)
err = json.Unmarshal(result, got) err = json.Unmarshal(result, got)
if err != nil { if err != nil {
t.Fatalf("%s decode failed, want err: %v, should be nil", c.name, err) t.Errorf("%s decode failed, want err: %v, should be nil", c.name, err)
} }
decodeEqual(t, c.err, got, "decode: "+c.name) decodeEqual(t, c.err, got, "decode: "+c.name)
} }
@ -255,20 +267,20 @@ func TestJSON(t *testing.T) {
func decodeEqual(t *testing.T, want, result *platform.Error, caseName string) { func decodeEqual(t *testing.T, want, result *platform.Error, caseName string) {
if want.Code != result.Code { if want.Code != result.Code {
t.Fatalf("%s code failed, want %s, got %s", caseName, want.Code, result.Code) t.Errorf("%s code failed, want %s, got %s", caseName, want.Code, result.Code)
} }
if want.Op != result.Op { if want.Op != result.Op {
t.Fatalf("%s op failed, want %s, got %s", caseName, want.Op, result.Op) t.Errorf("%s op failed, want %s, got %s", caseName, want.Op, result.Op)
} }
if want.Msg != result.Msg { if want.Msg != result.Msg {
t.Fatalf("%s msg failed, want %s, got %s", caseName, want.Msg, result.Msg) t.Errorf("%s msg failed, want %s, got %s", caseName, want.Msg, result.Msg)
} }
if want.Err != nil { if want.Err != nil {
if _, ok := want.Err.(*platform.Error); ok { if _, ok := want.Err.(*platform.Error); ok {
decodeEqual(t, want.Err.(*platform.Error), result.Err.(*platform.Error), caseName) decodeEqual(t, want.Err.(*platform.Error), result.Err.(*platform.Error), caseName)
} else { } else {
if want.Err.Error() != result.Err.Error() { if want.Err.Error() != result.Err.Error() {
t.Fatalf("%s Err failed, want %s, got %s", caseName, want.Err.Error(), result.Err.Error()) t.Errorf("%s Err failed, want %s, got %s", caseName, want.Err.Error(), result.Err.Error())
} }
} }
} }