Merge #72352

72352: server,cli: fix improperly wrapped errors r=knz a=rafiss refs #42510 I'm working on a linter that detects errors that are not wrapped correctly, and it discovered these. Release note: None Co-authored-by: Rafi Shamim <[email protected]>
cockroachdb · Nov 3, 2021 · 2154996 · 2154996
2 parents 05d3662 + b12219b
commit 2154996
Show file tree

Hide file tree

Showing 6 changed files with 8 additions and 8 deletions.
diff --git a/pkg/base/store_spec.go b/pkg/base/store_spec.go
@@ -94,7 +94,7 @@ func NewSizeSpec(
 		size.Percent, err = strconv.ParseFloat(factorValue, 64)
 		size.Percent *= percentFactor
 		if err != nil {
-			return SizeSpec{}, errors.Newf("could not parse %s size (%s) %s", field, value, err)
+			return SizeSpec{}, errors.Wrapf(err, "could not parse %s size (%s)", field, value)
 		}
 		if percentRange != nil {
 			if (percentRange.min != nil && size.Percent < *percentRange.min) ||
@@ -112,7 +112,7 @@ func NewSizeSpec(
 		var err error
 		size.InBytes, err = humanizeutil.ParseBytes(value)
 		if err != nil {
-			return SizeSpec{}, errors.Newf("could not parse %s size (%s) %s", field, value, err)
+			return SizeSpec{}, errors.Wrapf(err, "could not parse %s size (%s)", field, value)
 		}
 		if bytesRange != nil {
 			if bytesRange.min != nil && size.InBytes < *bytesRange.min {

diff --git a/pkg/base/store_spec_test.go b/pkg/base/store_spec_test.go
@@ -123,7 +123,7 @@ target_file_size=2097152`
 		{"path=/mnt/hda1,size=.009999", "store size (.009999) must be between 1.000000% and 100.000000%", StoreSpec{}},
 		// errors
 		{"path=/mnt/hda1,size=0", "store size (0) must be larger than 640 MiB", StoreSpec{}},
-		{"path=/mnt/hda1,size=abc", "could not parse store size (abc) strconv.ParseFloat: parsing \"\": invalid syntax", StoreSpec{}},
+		{"path=/mnt/hda1,size=abc", "could not parse store size (abc): strconv.ParseFloat: parsing \"\": invalid syntax", StoreSpec{}},
 		{"path=/mnt/hda1,size=", "no value specified for size", StoreSpec{}},
 		{"size=20GiB,path=/mnt/hda1,size=20GiB", "size field was used twice in store definition", StoreSpec{}},
 		{"size=123TB", "no path specified", StoreSpec{}},

diff --git a/pkg/cli/clisqlclient/conn_test.go b/pkg/cli/clisqlclient/conn_test.go
@@ -65,7 +65,7 @@ func TestConnRecover(t *testing.T) {
 	// this however.
 	testutils.SucceedsSoon(t, func() error {
 		if sqlRows, err := conn.Query(`SELECT 1`, nil); !errors.Is(err, driver.ErrBadConn) {
-			return errors.Newf("expected ErrBadConn, got %v", err)
+			return errors.NewAssertionErrorWithWrappedErrf(err, "expected ErrBadConn")
 		} else if err == nil {
 			if closeErr := sqlRows.Close(); closeErr != nil {
 				t.Fatal(closeErr)
@@ -89,7 +89,7 @@ func TestConnRecover(t *testing.T) {
 	// Ditto from Query().
 	testutils.SucceedsSoon(t, func() error {
 		if err := conn.Exec(`SELECT 1`, nil); !errors.Is(err, driver.ErrBadConn) {
-			return errors.Newf("expected ErrBadConn, got %v", err)
+			return errors.NewAssertionErrorWithWrappedErrf(err, "expected ErrBadConn")
 		}
 		return nil
 	})

diff --git a/pkg/cli/clisqlshell/sql.go b/pkg/cli/clisqlshell/sql.go
@@ -682,7 +682,7 @@ func (c *cliState) execSyscmd(command string) (string, error) {
 	cmd.Stderr = c.iCtx.stderr
 
 	if err := cmd.Run(); err != nil {
-		return "", fmt.Errorf("error in external command: %s", err)
+		return "", errors.Wrap(err, "error in external command")
 	}
 
 	return out.String(), nil

diff --git a/pkg/server/drain_test.go b/pkg/server/drain_test.go
@@ -87,7 +87,7 @@ func doTestDrain(tt *testing.T) {
 		if grpcutil.IsClosedConnection(err) {
 			return nil
 		}
-		return errors.Newf("server not yet refusing RPC, got %v", err)
+		return errors.NewAssertionErrorWithWrappedErrf(err, "server not yet refusing RPC")
 	})
 }
 

diff --git a/pkg/server/node.go b/pkg/server/node.go
@@ -449,7 +449,7 @@ func (n *Node) start(
 	// gossip can bootstrap using the most recently persisted set of
 	// node addresses.
 	if err := n.storeCfg.Gossip.SetStorage(n.stores); err != nil {
-		return fmt.Errorf("failed to initialize the gossip interface: %s", err)
+		return errors.Wrap(err, "failed to initialize the gossip interface")
 	}
 
 	// Initialize remaining stores/engines, if any.