server,cli: fix improperly wrapped errors

[rafiss]

refs #42510

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

Release note: None

[cockroach-teamcity]

This change is 

[knz]

very nice. I already love your linter.

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

---

pkg/cli/clisqlclient/conn_test.go, line 68 at r1 (raw file):

	testutils.SucceedsSoon(t, func() error {
		if sqlRows, err := conn.Query(`SELECT 1`, nil); !errors.Is(err, driver.ErrBadConn) {
			return errors.AssertionFailedf("expected ErrBadConn, got %v", err)

NewAssertionErrorWithWrappedErrf? same below

---

pkg/server/drain_test.go, line 90 at r1 (raw file):

			return nil
		}
		return errors.AssertionFailedf("server not yet refusing RPC, got %v", err)

assertion with wrap

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz)

---

pkg/cli/clisqlclient/conn_test.go, line 68 at r1 (raw file):

Previously, knz (kena) wrote…

NewAssertionErrorWithWrappedErrf? same below

Done.

---

pkg/server/drain_test.go, line 90 at r1 (raw file):

Previously, knz (kena) wrote…

assertion with wrap

Done.

[rafiss]

tftr!

bors r=knz

[craig]

Build failed:

• GitHub CI (Cockroach)

[rafiss]

bors r=knz

[craig]

Build failed:

• GitHub CI (Cockroach)

[rafiss]

bors r=knz

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[knz]

bors r-

[craig]

Canceled.

[knz]

made a mistake in review

[knz]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rafiss)

---

pkg/server/drain_test.go, line 90 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

Done.

please revert this one. we want an error to be returned by this function when Dial returns a nil error.

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz)

---

pkg/server/drain_test.go, line 90 at r1 (raw file):

Previously, knz (kena) wrote…

please revert this one. we want an error to be returned by this function when Dial returns a nil error.

not sure what the best way to do this is while still keeping the forthcoming linter happy. i could do

		errS := "<nil>"
		if err != nil {
			errS = err.Error()
		}
		return errors.Newf("server not yet refusing RPC, got %s", errS)

which seems gross.

should i keep it as is and instead allow a //nolint directive similar to

cockroach/pkg/testutils/lint/passes/fmtsafe/fmtsafe.go

Line 274 in f339331

if strings.Contains(c.Text, "nolint:fmtsafe") {

[knz]

yes a nolint comment in this case seems appropriate.
you can also do:

if err != nil {
   if grpc.XXX(err) {
     return nil
   }
   return errors.NewAssertion...(err, "...")
}
// nil err
return errors.AssertionFailed("dial did not fail")

[rafiss]

thanks, i'll add nolint support when i introduce the linter

[rafiss]

tftr!

bors r=knz

[craig]

Build succeeded:

• GitHub CI (Cockroach)