Skip to content

Commit

Permalink
chore(pkger): improve error handling in service
Browse files Browse the repository at this point in the history
this provides influxdb.Errors whenever possible from the pker service layer.
the behavior that caused the error is somewhat implicated in the errro code.
it is not perfect but is a big step forward. using the http server/client to
run pkger stuff makes it abundantly clear the errors need to be communicated
better.

closes: #16313
  • Loading branch information
jsteenb2 committed Dec 23, 2019
1 parent e33d74b commit aef32b0
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 63 deletions.
46 changes: 43 additions & 3 deletions pkger/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,40 @@ func (e *parseErr) ValidationErrs() []ValidationErr {
errs = append(errs, traverseErrs(rootErr, v)...)
}
}
return errs

// used to provide a means to == or != in the map lookup
// to remove duplicate errors
type key struct {
kind string
fields string
indexes string
reason string
}

m := make(map[key]bool)
var out []ValidationErr
for _, verr := range errs {
k := key{
kind: verr.Kind,
fields: strings.Join(verr.Fields, ":"),
reason: verr.Reason,
}
var indexes []string
for _, idx := range verr.Indexes {
if idx == nil {
continue
}
indexes = append(indexes, strconv.Itoa(*idx))
}
k.indexes = strings.Join(indexes, ":")
if m[k] {
continue
}
m[k] = true
out = append(out, verr)
}

return out
}

// ValidationErr represents an error during the parsing of a package.
Expand Down Expand Up @@ -1471,8 +1504,15 @@ func (e *parseErr) append(errs ...resourceErr) {
// it will return nil values for the parseErr, making it unsafe
// to use.
func IsParseErr(err error) bool {
_, ok := err.(*parseErr)
return ok
if _, ok := err.(*parseErr); ok {
return true
}

iErr, ok := err.(*influxdb.Error)
if !ok {
return false
}
return IsParseErr(iErr.Err)
}

func normStr(s string) string {
Expand Down
55 changes: 55 additions & 0 deletions pkger/parser_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package pkger

import (
"errors"
"path/filepath"
"strconv"
"strings"
Expand Down Expand Up @@ -4338,6 +4339,60 @@ spec:
})
}

func Test_IsParseError(t *testing.T) {
tests := []struct {
name string
err error
expected bool
}{
{
name: "base case",
err: &parseErr{},
expected: true,
},
{
name: "wrapped by influxdb error",
err: &influxdb.Error{
Err: &parseErr{},
},
expected: true,
},
{
name: "deeply nested in influxdb error",
err: &influxdb.Error{
Err: &influxdb.Error{
Err: &influxdb.Error{
Err: &influxdb.Error{
Err: &parseErr{},
},
},
},
},
expected: true,
},
{
name: "influxdb error without nested parse err",
err: &influxdb.Error{
Err: errors.New("nope"),
},
expected: false,
},
{
name: "plain error",
err: errors.New("nope"),
expected: false,
},
}

for _, tt := range tests {
fn := func(t *testing.T) {
isParseErr := IsParseErr(tt.err)
assert.Equal(t, tt.expected, isParseErr)
}
t.Run(tt.name, fn)
}
}

func Test_PkgValidationErr(t *testing.T) {
iPtr := func(i int) *int {
return &i
Expand Down
Loading

0 comments on commit aef32b0

Please sign in to comment.