diff --git a/Gopkg.lock b/Gopkg.lock index e405708412eb..bf6e395f88c7 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -405,7 +405,8 @@ revision = "aca09668cb243672cb82f3cbeec73b3871e13cc9" [[projects]] - digest = "1:7cfbe9c31d82a7044c86f36c24354363da85cc0a69ee7f2a20908a620d7e9b8a" + branch = "v1.2.4-cockroach20.1" + digest = "1:f3e110587a7cde6152fe8861a5873ffb552232b8a91ce444075397f4ebabd4cd" name = "github.com/cockroachdb/errors" packages = [ ".", @@ -428,8 +429,7 @@ "withstack", ] pruneopts = "UT" - revision = "9e21257b06ad938e53c24c52b393076a51b61540" - version = "v1.2.4" + revision = "fcfc812fae48db497f9fb1dbc7b4d4041caa06fa" [[projects]] digest = "1:a44e537b3e080ff297315d166956dba9607f070644a89be78b59af6c54876b64" diff --git a/Gopkg.toml b/Gopkg.toml index 71595b623855..0f6dd3cc3d7f 100644 --- a/Gopkg.toml +++ b/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" diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index d5ae8cc8fbaa..dd73143530b3 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/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 { diff --git a/pkg/cmd/roachtest/cluster_test.go b/pkg/cmd/roachtest/cluster_test.go index 3bf35cee76fe..60c293d60d20 100644 --- a/pkg/cmd/roachtest/cluster_test.go +++ b/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) } }) diff --git a/pkg/cmd/roachtest/disk_full.go b/pkg/cmd/roachtest/disk_full.go index eb7bd6e4b357..76c9a129a016 100644 --- a/pkg/cmd/roachtest/disk_full.go +++ b/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) } } diff --git a/pkg/cmd/roachtest/restore.go b/pkg/cmd/roachtest/restore.go index df8590881017..37ccf04d178f 100644 --- a/pkg/cmd/roachtest/restore.go +++ b/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 } diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go index a7b5c013f447..a6414dcf3fb7 100644 --- a/pkg/cmd/roachtest/test_runner.go +++ b/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() } diff --git a/pkg/roachpb/errors.go b/pkg/roachpb/errors.go index 22b6fd3e0163..459b044d1ae1 100644 --- a/pkg/roachpb/errors.go +++ b/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)) } diff --git a/pkg/util/errorutil/error_test.go b/pkg/util/errorutil/error_test.go index a540a8c83687..f227d806ce0a 100644 --- a/pkg/util/errorutil/error_test.go +++ b/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) } diff --git a/pkg/util/grpcutil/grpc_util.go b/pkg/util/grpcutil/grpc_util.go index 8adc240c2e26..4a648acabd98 100644 --- a/pkg/util/grpcutil/grpc_util.go +++ b/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 diff --git a/pkg/util/netutil/net.go b/pkg/util/netutil/net.go index dfa8026f682a..2734f148c78d 100644 --- a/pkg/util/netutil/net.go +++ b/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) } diff --git a/vendor b/vendor index 26ce33a83ff9..0f7614466587 160000 --- a/vendor +++ b/vendor @@ -1 +1 @@ -Subproject commit 26ce33a83ff9e807590610329cbda4ef2f83e289 +Subproject commit 0f76144665879f45bad13b401a9c3c67bdffc727