From 11618723243bd3496d2f59f9f3f87ff24673ac50 Mon Sep 17 00:00:00 2001 From: James Shubin Date: Fri, 26 Jul 2019 02:53:40 -0400 Subject: [PATCH] etcd: fs: Errors should start with lower case --- engine/resources/aws_ec2.go | 2 +- engine/resources/file.go | 2 +- engine/resources/group.go | 2 +- engine/resources/packagekit/packagekit.go | 50 +++++++++++------------ etcd/etcd.go | 2 +- etcd/fs/fs.go | 6 +-- lang/funcs/core/math/sqrt_func_test.go | 4 +- test/test-govet.sh | 17 ++++++++ yamlgraph/gconfig.go | 4 +- 9 files changed, 53 insertions(+), 36 deletions(-) diff --git a/engine/resources/aws_ec2.go b/engine/resources/aws_ec2.go index d20cc70c..24224a89 100644 --- a/engine/resources/aws_ec2.go +++ b/engine/resources/aws_ec2.go @@ -1025,7 +1025,7 @@ func (obj *AwsEc2Res) snsMakeTopic() (string, error) { } obj.init.Logf("Created SNS Topic") if topic.TopicArn == nil { - return "", fmt.Errorf("TopicArn is nil") + return "", fmt.Errorf("the TopicArn is nil") } return *topic.TopicArn, nil } diff --git a/engine/resources/file.go b/engine/resources/file.go index 390bd06c..6c323bd0 100644 --- a/engine/resources/file.go +++ b/engine/resources/file.go @@ -140,7 +140,7 @@ func (obj *FileRes) Validate() error { // XXX: should this specify that we create an empty directory instead? //if obj.Source == "" && obj.isDir() { - // return fmt.Errorf("Can't specify an empty source when creating a Dir.") + // return fmt.Errorf("can't specify an empty source when creating a Dir.") //} return nil diff --git a/engine/resources/group.go b/engine/resources/group.go index 0146c70f..fecce752 100644 --- a/engine/resources/group.go +++ b/engine/resources/group.go @@ -58,7 +58,7 @@ func (obj *GroupRes) Default() engine.Res { // Validate if the params passed in are valid data. func (obj *GroupRes) Validate() error { if obj.State != "exists" && obj.State != "absent" { - return fmt.Errorf("State must be 'exists' or 'absent'") + return fmt.Errorf("state must be 'exists' or 'absent'") } return nil } diff --git a/engine/resources/packagekit/packagekit.go b/engine/resources/packagekit/packagekit.go index 9beb6385..9099362f 100644 --- a/engine/resources/packagekit/packagekit.go +++ b/engine/resources/packagekit/packagekit.go @@ -352,7 +352,7 @@ loop: // should already be broken break loop } else { - return []string{}, fmt.Errorf("PackageKit: Error: %v", signal.Body) + return []string{}, fmt.Errorf("error in body: %v", signal.Body) } } } @@ -363,9 +363,9 @@ loop: func (obj *Conn) IsInstalledList(packages []string) ([]bool, error) { var filter uint64 // initializes at the "zero" value of 0 filter += PkFilterEnumArch // always search in our arch - packageIDs, e := obj.ResolvePackages(packages, filter) - if e != nil { - return nil, fmt.Errorf("ResolvePackages error: %v", e) + packageIDs, err := obj.ResolvePackages(packages, filter) + if err != nil { + return nil, errwrap.Wrapf(err, "error resolving packages") } var m = make(map[string]int) @@ -443,7 +443,7 @@ loop: } if signal.Name == FmtTransactionMethod("ErrorCode") { - return fmt.Errorf("PackageKit: Error: %v", signal.Body) + return fmt.Errorf("error in body: %v", signal.Body) } else if signal.Name == FmtTransactionMethod("Package") { // a package was installed... // only start the timer once we're here... @@ -454,14 +454,14 @@ loop: } else if signal.Name == FmtTransactionMethod("Destroy") { return nil // success } else { - return fmt.Errorf("PackageKit: Error: %v", signal.Body) + return fmt.Errorf("error in body: %v", signal.Body) } case <-util.TimeAfterOrBlock(timeout): if finished { obj.Logf("Timeout: InstallPackages: Waiting for 'Destroy'") return nil // got tired of waiting for Destroy } - return fmt.Errorf("PackageKit: Timeout: InstallPackages: %s", strings.Join(packageIDs, ", ")) + return fmt.Errorf("timeout installing packages: %s", strings.Join(packageIDs, ", ")) } } } @@ -500,7 +500,7 @@ loop: } if signal.Name == FmtTransactionMethod("ErrorCode") { - return fmt.Errorf("PackageKit: Error: %v", signal.Body) + return fmt.Errorf("error in body: %v", signal.Body) } else if signal.Name == FmtTransactionMethod("Package") { // a package was installed... continue loop @@ -511,7 +511,7 @@ loop: // should already be broken break loop } else { - return fmt.Errorf("PackageKit: Error: %v", signal.Body) + return fmt.Errorf("error in body: %v", signal.Body) } } } @@ -549,7 +549,7 @@ loop: } if signal.Name == FmtTransactionMethod("ErrorCode") { - return fmt.Errorf("PackageKit: Error: %v", signal.Body) + return fmt.Errorf("error in body: %v", signal.Body) } else if signal.Name == FmtTransactionMethod("Package") { } else if signal.Name == FmtTransactionMethod("Finished") { // TODO: should we wait for the Destroy signal? @@ -558,7 +558,7 @@ loop: // should already be broken break loop } else { - return fmt.Errorf("PackageKit: Error: %v", signal.Body) + return fmt.Errorf("error in body: %v", signal.Body) } } } @@ -601,7 +601,7 @@ loop: } if signal.Name == FmtTransactionMethod("ErrorCode") { - err = fmt.Errorf("PackageKit: Error: %v", signal.Body) + err = fmt.Errorf("error in body: %v", signal.Body) return // one signal returned per packageID found... @@ -626,7 +626,7 @@ loop: // should already be broken break loop } else { - err = fmt.Errorf("PackageKit: Error: %v", signal.Body) + err = fmt.Errorf("error in body: %v", signal.Body) return } } @@ -669,7 +669,7 @@ loop: } if signal.Name == FmtTransactionMethod("ErrorCode") { - return nil, fmt.Errorf("PackageKit: Error: %v", signal.Body) + return nil, fmt.Errorf("error in body: %v", signal.Body) } else if signal.Name == FmtTransactionMethod("Package") { //pkg_int, ok := signal.Body[0].(int) @@ -692,7 +692,7 @@ loop: // should already be broken break loop } else { - return nil, fmt.Errorf("PackageKit: Error: %v", signal.Body) + return nil, fmt.Errorf("error in body: %v", signal.Body) } } } @@ -718,9 +718,9 @@ func (obj *Conn) PackagesToPackageIDs(packageMap map[string]string, filter uint6 if obj.Debug { obj.Logf("PackagesToPackageIDs(): %s", strings.Join(packages, ", ")) } - resolved, e := obj.ResolvePackages(packages, filter) - if e != nil { - return nil, fmt.Errorf("Resolve error: %v", e) + resolved, err := obj.ResolvePackages(packages, filter) + if err != nil { + return nil, errwrap.Wrapf(err, "error resolving") } found := make([]bool, count) // default false @@ -758,7 +758,7 @@ func (obj *Conn) PackagesToPackageIDs(packageMap map[string]string, filter uint6 } state := packageMap[pkg] // lookup the requested state/version if state == "" { - return nil, fmt.Errorf("Empty package state for %v", pkg) + return nil, fmt.Errorf("empty package state for: `%s`", pkg) } found[index] = true stateIsVersion := (state != "installed" && state != "uninstalled" && state != "newest") // must be a ver. string @@ -794,9 +794,9 @@ func (obj *Conn) PackagesToPackageIDs(packageMap map[string]string, filter uint6 // to be done, and if so, anything that needs updating isn't newest! // if something isn't installed, we can't verify it with this method // FIXME: https://github.com/hughsie/PackageKit/issues/116 - updates, e := obj.GetUpdates(filter) - if e != nil { - return nil, fmt.Errorf("Updates error: %v", e) + updates, err := obj.GetUpdates(filter) + if err != nil { + return nil, errwrap.Wrapf(err, "updates error") } for _, packageID := range updates { //obj.Logf("* %v", packageID) @@ -844,9 +844,9 @@ func (obj *Conn) PackagesToPackageIDs(packageMap map[string]string, filter uint6 if obj.Debug { obj.Logf("PackagesToPackageIDs(): Recurse: %s", strings.Join(checkPackages, ", ")) } - recursion, e = obj.PackagesToPackageIDs(filteredPackageMap, filter+PkFilterEnumNewest) - if e != nil { - return nil, fmt.Errorf("Recursion error: %v", e) + recursion, err = obj.PackagesToPackageIDs(filteredPackageMap, filter+PkFilterEnumNewest) + if err != nil { + return nil, errwrap.Wrapf(err, "recursion error") } } } diff --git a/etcd/etcd.go b/etcd/etcd.go index 9a081906..ecbf3381 100644 --- a/etcd/etcd.go +++ b/etcd/etcd.go @@ -400,7 +400,7 @@ func (obj *EmbdEtcd) Validate() error { if obj.NoNetwork { if len(obj.Seeds) != 0 || len(obj.ClientURLs) != 0 || len(obj.ServerURLs) != 0 { - return fmt.Errorf("NoNetwork is mutually exclusive with Seeds, ClientURLs and ServerURLs") + return fmt.Errorf("option NoNetwork is mutually exclusive with Seeds, ClientURLs and ServerURLs") } } diff --git a/etcd/fs/fs.go b/etcd/fs/fs.go index 1f9a4dd1..255befe8 100644 --- a/etcd/fs/fs.go +++ b/etcd/fs/fs.go @@ -70,9 +70,9 @@ var ( // ErrNotExist is returned when we can't find the requested path. ErrNotExist = os.ErrNotExist - ErrFileClosed = errors.New("File is closed") - ErrFileReadOnly = errors.New("File handle is read only") - ErrOutOfRange = errors.New("Out of range") + ErrFileClosed = errors.New("file is closed") + ErrFileReadOnly = errors.New("file handle is read only") + ErrOutOfRange = errors.New("out of range") ) // Fs is a specialized afero.Fs implementation for etcd. It implements a small diff --git a/lang/funcs/core/math/sqrt_func_test.go b/lang/funcs/core/math/sqrt_func_test.go index 56b444c3..36951a13 100644 --- a/lang/funcs/core/math/sqrt_func_test.go +++ b/lang/funcs/core/math/sqrt_func_test.go @@ -33,7 +33,7 @@ func testSqrtSuccess(input, sqrt float64) error { return err } if val.Float() != sqrt { - return fmt.Errorf("Invalid output, expected %f, got %f", sqrt, val.Float()) + return fmt.Errorf("invalid output, expected %f, got %f", sqrt, val.Float()) } return nil } @@ -42,7 +42,7 @@ func testSqrtError(input float64) error { inputVal := &types.FloatValue{V: input} _, err := Sqrt([]types.Value{inputVal}) if err == nil { - return fmt.Errorf("Expected error for input %f, got nil", input) + return fmt.Errorf("expected error for input %f, got nil", input) } return nil } diff --git a/test/test-govet.sh b/test/test-govet.sh index dccbaead..edd96824 100755 --- a/test/test-govet.sh +++ b/test/test-govet.sh @@ -49,6 +49,22 @@ function naked-error() { return 0 } +# catch errors that start with a capital +function lowercase-errors() { + if [[ $1 == *"/bindata.go" ]]; then # ends with bindata.go ? + return 0 # skip those generated files + fi + + if grep -E 'errors\.New\(\"[A-Z]' "$1"; then + return 1 + fi + if grep -E 'fmt\.Errorf\(\"[A-Z]' "$1"; then + return 1 + fi + # TODO: add errwrap.Wrap* related matching + return 0 +} + function consistent-imports() { if [ "$1" = './util/errwrap/errwrap.go' ]; then return 0 @@ -93,6 +109,7 @@ for file in `find . -maxdepth 9 -type f -name '*.go' -not -path './old/*' -not - run-test simplify-gocase "$file" run-test token-coloncheck "$file" run-test naked-error "$file" + run-test lowercase-errors "$file" run-test consistent-imports "$file" done diff --git a/yamlgraph/gconfig.go b/yamlgraph/gconfig.go index 4f4d380f..2bfdd25d 100644 --- a/yamlgraph/gconfig.go +++ b/yamlgraph/gconfig.go @@ -227,7 +227,7 @@ func (obj *GraphConfig) NewGraphFromConfig(hostname string, world engine.World, // store in backend (usually etcd) if err := world.ResExport(context.TODO(), resourceList); err != nil { - return nil, fmt.Errorf("Config: Could not export resources: %v", err) + return nil, fmt.Errorf("config: could not export resources: %v", err) } // lookup from backend (usually etcd) @@ -243,7 +243,7 @@ func (obj *GraphConfig) NewGraphFromConfig(hostname string, world engine.World, var err error resourceList, err = world.ResCollect(context.TODO(), hostnameFilter, kindFilter) if err != nil { - return nil, fmt.Errorf("Config: Could not collect resources: %v", err) + return nil, fmt.Errorf("config: could not collect resources: %v", err) } } for _, res := range resourceList {