From b12219b2ef9806302bca3b178542579411dfaaf4 Mon Sep 17 00:00:00 2001
From: Rafi Shamim <rafi@cockroachlabs.com>
Date: Tue, 2 Nov 2021 16:53:16 -0400
Subject: [PATCH] server,cli: fix improperly wrapped errors

I'm working on a linter that detects errors that are not wrapped
correctly, and it discovered these.

Release note: None
---
 pkg/base/store_spec.go            | 4 ++--
 pkg/base/store_spec_test.go       | 2 +-
 pkg/cli/clisqlclient/conn_test.go | 4 ++--
 pkg/cli/clisqlshell/sql.go        | 2 +-
 pkg/server/drain_test.go          | 2 +-
 pkg/server/node.go                | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/pkg/base/store_spec.go b/pkg/base/store_spec.go
index 6ce9e9c89724..a8d6c77a9a2e 100644
--- 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
index 046a5d2cb7b1..f7fd65984222 100644
--- 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
index b385017b82bf..ae7e384b3787 100644
--- 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
index 7c5d974e23e8..6f29c2fdc003 100644
--- 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
index 11bd604f352f..3ee0762f434e 100644
--- 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
index fcc05e71e5f8..2208ebd1f992 100644
--- 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.