Merge pull request cockroachdb#47710 from knz/backport20.1-47278

Gopkg.toml

@@ -75,6 +75,10 @@ ignored = [
   name = "github.com/cockroachdb/circuitbreaker"
   branch = "master"
 
+[[constraint]]
+  name = "github.com/cockroachdb/errors"
+  branch = "v1.2.4-cockroach20.1"
+
 [[constraint]]
   name = "vitess.io/vitess"
   source = "https://github.com/cockroachdb/vitess"

pkg/cmd/roachtest/cluster.go

@@ -396,24 +396,64 @@ func execCmd(ctx context.Context, l *logger, args ...string) error {
 		// Context errors opaquely appear as "signal killed" when manifested.
 		// We surface this error explicitly.
 		if ctx.Err() != nil {
-			err = ctx.Err()
+			err = errors.CombineErrors(ctx.Err(), err)
 		}
 
 		// Synchronize access to ring buffers before using them to create an
 		// error to return.
 		cancel()
 		wg.Wait()
-		return errors.Wrapf(
-			err,
-			"%s returned:\nstderr:\n%s\nstdout:\n%s",
-			strings.Join(args, " "),
-			debugStderrBuffer.String(),
-			debugStdoutBuffer.String(),
-		)
+		if err != nil {
+			err = &withCommandDetails{
+				cause:  err,
+				cmd:    strings.Join(args, " "),
+				stderr: debugStderrBuffer.String(),
+				stdout: debugStdoutBuffer.String(),
+			}
+		}
+		return err
 	}
 	return nil
 }
 
+type withCommandDetails struct {
+	cause  error
+	cmd    string
+	stderr string
+	stdout string
+}
+
+var _ error = (*withCommandDetails)(nil)
+var _ errors.Formatter = (*withCommandDetails)(nil)
+
+// Error implements error.
+func (e *withCommandDetails) Error() string { return e.cause.Error() }
+
+// Cause implements causer.
+func (e *withCommandDetails) Cause() error { return e.cause }
+
+// Format implements fmt.Formatter.
+func (e *withCommandDetails) Format(s fmt.State, verb rune) { errors.FormatError(e, s, verb) }
+
+// FormatError implements errors.Formatter.
+func (e *withCommandDetails) FormatError(p errors.Printer) error {
+	p.Printf("%s returned", e.cmd)
+	if p.Detail() {
+		p.Printf("stderr:\n%s\nstdout:\n%s", e.stderr, e.stdout)
+	}
+	return e.cause
+}
+
+// GetStderr retrieves the stderr output of a command that
+// returned with an error, or the empty string if there was no stderr.
+func GetStderr(err error) string {
+	var c *withCommandDetails
+	if errors.As(err, &c) {
+		return c.stderr
+	}
+	return ""
+}
+
 // execCmdWithBuffer executes the given command and returns its stdout/stderr
 // output. If the return code is not 0, an error is also returned.
 // l is used to log the command before running it. No output is logged.
@@ -1117,7 +1157,7 @@ func (f *clusterFactory) newCluster(
 			break
 		}
 		l.PrintfCtx(ctx, "Failed to create cluster.")
-		if !strings.Contains(err.Error(), "already exists") {
+		if !strings.Contains(GetStderr(err), "already exists") {
 			l.PrintfCtx(ctx, "Cleaning up in case it was partially created.")
 			c.Destroy(ctx, closeLogger, l)
 		} else {

pkg/cmd/roachtest/cluster_test.go

@@ -173,14 +173,9 @@ func TestClusterMonitor(t *testing.T) {
 			time.Sleep(30 * time.Millisecond)
 			return execCmd(ctx, logger, "/bin/bash", "-c", "echo hi && notthere")
 		})
-		expectedErr := regexp.QuoteMeta(`/bin/bash -c echo hi && notthere returned:
-stderr:
-/bin/bash: notthere: command not found
-
-stdout:
-hi
-: exit status 127`)
+		expectedErr := regexp.QuoteMeta(`exit status 127`)
 		if err := m.wait("sleep", "100"); !testutils.IsError(err, expectedErr) {
+			t.Logf("error details: %+v", err)
 			t.Error(err)
 		}
 	})
@@ -202,6 +197,7 @@ hi
 		})
 		expectedErr := regexp.QuoteMeta(`Goexit() was called`)
 		if err := m.wait("sleep", "100"); !testutils.IsError(err, expectedErr) {
+			t.Logf("error details: %+v", err)
 			t.Error(err)
 		}
 	})

pkg/cmd/roachtest/disk_full.go

@@ -68,7 +68,7 @@ func registerDiskFull(r *testRegistry) {
 					m.ExpectDeath()
 					if err := c.StartE(ctx, c.Node(n)); err == nil {
 						t.Fatalf("node successfully started unexpectedly")
-					} else if strings.Contains(err.Error(), "a panic has occurred") {
+					} else if strings.Contains(GetStderr(err), "a panic has occurred") {
 						t.Fatal(err)
 					}
 				}

pkg/cmd/roachtest/restore.go

@@ -130,7 +130,7 @@ func (hc *HealthChecker) Runner(ctx context.Context) (err error) {
 
 		if elapsed := timeutil.Since(tBegin); elapsed > 10*time.Second {
 			err := errors.Errorf("health check against node %d took %s", nodeIdx, elapsed)
-			logger.Printf(err.Error() + "\n")
+			logger.Printf("%+v", err)
 			// TODO(tschottdorf): see method comment.
 			// return err
 		}

pkg/cmd/roachtest/test_runner.go

@@ -281,7 +281,7 @@ func (r *testRunner) Run(
 				l,
 			); err != nil {
 				// A worker returned an error. Let's shut down.
-				msg := fmt.Sprintf("Worker %d returned with error. Quiescing. Error: %s", i, err)
+				msg := fmt.Sprintf("Worker %d returned with error. Quiescing. Error: %+v", i, err)
 				shout(ctx, l, lopt.stdout, msg)
 				errs.AddErr(err)
 				// Quiesce the stopper. This will cause all workers to not pick up more
@@ -479,7 +479,7 @@ func (r *testRunner) runWorker(
 		if err != nil || t.Failed() {
 			failureMsg := fmt.Sprintf("%s (%d) - ", testToRun.spec.Name, testToRun.runNum)
 			if err != nil {
-				failureMsg += err.Error()
+				failureMsg += fmt.Sprintf("%+v", err)
 			} else {
 				failureMsg += t.FailureMsg()
 			}

pkg/roachpb/errors.go

@@ -865,10 +865,5 @@ var _ ErrorDetailInterface = &IndeterminateCommitError{}
 
 // IsRangeNotFoundError returns true if err contains a *RangeNotFoundError.
 func IsRangeNotFoundError(err error) bool {
-	// TODO(ajwerner): adopt errors.IsType once the pull request to add it merges.
-	_, isRangeNotFound := errors.If(err, func(err error) (interface{}, bool) {
-		_, isRangeNotFound := err.(*RangeNotFoundError)
-		return err, isRangeNotFound
-	})
-	return isRangeNotFound
+	return errors.HasType(err, (*RangeNotFoundError)(nil))
 }

pkg/util/errorutil/error_test.go

@@ -39,7 +39,7 @@ func TestUnexpectedWithIssueErrorf(t *testing.T) {
 	}
 
 	// Check that the issue number is present in the safe details.
-	exp = "safe details: issue #%d\n    -- arg 1: 1234"
+	exp = "4 safe details enclosed"
 	if !strings.Contains(safeMsg, exp) {
 		t.Errorf("expected substring in error\n%s\ngot:\n%s", exp, safeMsg)
 	}

pkg/util/grpcutil/grpc_util.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/util/netutil"
-	"github.com/pkg/errors"
+	"github.com/cockroachdb/errors"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/connectivity"
@@ -77,13 +77,11 @@ func IsClosedConnection(err error) bool {
 // TODO(bdarnell): Replace this with a cleaner mechanism when/if
 // https://github.com/grpc/grpc-go/issues/1443 is resolved.
 func RequestDidNotStart(err error) bool {
-	if _, ok := err.(connectionNotReadyError); ok {
+	if errors.HasType(err, connectionNotReadyError{}) ||
+		errors.HasType(err, (*netutil.InitialHeartbeatFailedError)(nil)) {
 		return true
 	}
-	if _, ok := err.(*netutil.InitialHeartbeatFailedError); ok {
-		return true
-	}
-	s, ok := status.FromError(err)
+	s, ok := status.FromError(errors.Cause(err))
 	if !ok {
 		// This is a non-gRPC error; assume nothing.
 		return false

pkg/util/netutil/net.go

@@ -178,13 +178,12 @@ var _ error = (*InitialHeartbeatFailedError)(nil)
 var _ fmt.Formatter = (*InitialHeartbeatFailedError)(nil)
 var _ errors.Formatter = (*InitialHeartbeatFailedError)(nil)
 
-// Note: Error is not a causer. If this is changed to implement
-// Cause()/Unwrap(), change the type assertions in package cli and
-// elsewhere to use errors.As() or equivalent.
-
 // Error implements error.
 func (e *InitialHeartbeatFailedError) Error() string { return fmt.Sprintf("%v", e) }
 
+// Cause implements causer.
+func (e *InitialHeartbeatFailedError) Cause() error { return e.WrappedErr }
+
 // Format implements fmt.Formatter.
 func (e *InitialHeartbeatFailedError) Format(s fmt.State, verb rune) { errors.FormatError(e, s, verb) }