Skip to content

Commit

Permalink
*: use errors.HasType / HasInterface / As instead of casts
Browse files Browse the repository at this point in the history
Release note: None
  • Loading branch information
knz committed May 7, 2020
1 parent 15c7371 commit a8ae1bf
Show file tree
Hide file tree
Showing 124 changed files with 431 additions and 523 deletions.
3 changes: 2 additions & 1 deletion pkg/acceptance/localcluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,8 @@ func (n *Node) startAsyncInnerLocked(ctx context.Context, joins ...string) error

log.Infof(ctx, "process %d: %s", cmd.Process.Pid, cmd.ProcessState)

execErr, _ := waitErr.(*exec.ExitError)
var execErr *exec.ExitError
_ = errors.As(waitErr, &execErr)
n.Lock()
n.setNotRunningLocked(execErr)
n.Unlock()
Expand Down
3 changes: 2 additions & 1 deletion pkg/base/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/envutil"
"github.com/cockroachdb/cockroach/pkg/util/mon"
"github.com/cockroachdb/cockroach/pkg/util/retry"
"github.com/cockroachdb/errors"
)

// Base config defaults.
Expand Down Expand Up @@ -219,7 +220,7 @@ type Config struct {
}

func wrapError(err error) error {
if _, ok := err.(*security.Error); !ok {
if !errors.HasType(err, (*security.Error)(nil)) {
return &security.Error{
Message: "problem using security settings",
Err: err,
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/restore_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ func WriteTableDescs(
b.InitPut(kv.Key, &kv.Value, false)
}
if err := txn.Run(ctx, b); err != nil {
if _, ok := errors.UnwrapAll(err).(*roachpb.ConditionFailedError); ok {
if errors.HasType(err, (*roachpb.ConditionFailedError)(nil)) {
return pgerror.Newf(pgcode.DuplicateObject, "table already exists")
}
return err
Expand Down
61 changes: 35 additions & 26 deletions pkg/ccl/changefeedccl/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ package changefeedccl

import (
"fmt"
"reflect"
"strings"

"github.com/cockroachdb/errors"
)

const retryableErrorString = "retryable changefeed error"
Expand Down Expand Up @@ -40,40 +43,46 @@ func (e *retryableError) Unwrap() error { return e.wrapped }
// IsRetryableError returns true if the supplied error, or any of its parent
// causes, is a IsRetryableError.
func IsRetryableError(err error) bool {
for {
if err == nil {
return false
}
if _, ok := err.(*retryableError); ok {
return true
}
errStr := err.Error()
if strings.Contains(errStr, retryableErrorString) {
// If a RetryableError occurs on a remote node, DistSQL serializes it such
// that we can't recover the structure and we have to rely on this
// unfortunate string comparison.
return true
}
if strings.Contains(errStr, `rpc error`) {
// When a crdb node dies, any DistSQL flows with processors scheduled on
// it get an error with "rpc error" in the message from the call to
// `(*DistSQLPlanner).Run`.
return true
}
if e, ok := err.(interface{ Unwrap() error }); ok {
err = e.Unwrap()
continue
}
if err == nil {
return false
}
if errors.HasType(err, (*retryableError)(nil)) {
return true
}

// TODO(knz): this is a bad implementation. Make it go away
// by avoiding string comparisons.

errStr := err.Error()
if strings.Contains(errStr, retryableErrorString) {
// If a RetryableError occurs on a remote node, DistSQL serializes it such
// that we can't recover the structure and we have to rely on this
// unfortunate string comparison.
return true
}
if strings.Contains(errStr, `rpc error`) {
// When a crdb node dies, any DistSQL flows with processors scheduled on
// it get an error with "rpc error" in the message from the call to
// `(*DistSQLPlanner).Run`.
return true
}
return false
}

// MaybeStripRetryableErrorMarker performs some minimal attempt to clean the
// RetryableError marker out. This won't do anything if the RetryableError
// itself has been wrapped, but that's okay, we'll just have an uglier string.
func MaybeStripRetryableErrorMarker(err error) error {
if e, ok := err.(*retryableError); ok {
err = e.wrapped
// The following is a hack to work around the error cast linter.
// What we're doing here is really not kosher; this function
// has no business in assuming that the retryableError{} wrapper
// has not been wrapped already. We could even expect that
// it gets wrapped in the common case.
// TODO(knz): Remove/replace this.
if reflect.TypeOf(err) == retryableErrorType {
err = errors.UnwrapOnce(err)
}
return err
}

var retryableErrorType = reflect.TypeOf((*retryableError)(nil))
9 changes: 4 additions & 5 deletions pkg/ccl/changefeedccl/kvfeed/kv_feed.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,16 +276,15 @@ func (f *kvFeed) runUntilTableEvent(
// buffer, its seems that we could do this without having to destroy and
// recreate the rangefeeds.
err = g.Wait()
switch err := err.(type) {
case nil:
if err == nil {
log.Fatalf(ctx, "feed exited with no error and no scan boundary")
return hlc.Timestamp{}, nil // unreachable
case *errBoundaryReached:
} else if tErr := (*errBoundaryReached)(nil); errors.As(err, &tErr) {
// TODO(ajwerner): iterate the spans and add a Resolved timestamp.
// We'll need to do this to ensure that a resolved timestamp propagates
// when we're trying to exit.
return err.Timestamp().Prev(), nil
default:
return tErr.Timestamp().Prev(), nil
} else {
return hlc.Timestamp{}, err
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/ccl/importccl/import_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,14 @@ func ingestKvs(
// more efficient than parsing every kv.
if indexID == 1 {
if err := pkIndexAdder.Add(ctx, kv.Key, kv.Value.RawBytes); err != nil {
if _, ok := err.(storagebase.DuplicateKeyError); ok {
if errors.HasType(err, (*storagebase.DuplicateKeyError)(nil)) {
return errors.Wrap(err, "duplicate key in primary index")
}
return err
}
} else {
if err := indexAdder.Add(ctx, kv.Key, kv.Value.RawBytes); err != nil {
if _, ok := err.(storagebase.DuplicateKeyError); ok {
if errors.HasType(err, (*storagebase.DuplicateKeyError)(nil)) {
return errors.Wrap(err, "duplicate key in index")
}
return err
Expand All @@ -363,14 +363,14 @@ func ingestKvs(
}

if err := pkIndexAdder.Flush(ctx); err != nil {
if err, ok := err.(storagebase.DuplicateKeyError); ok {
if errors.HasType(err, (*storagebase.DuplicateKeyError)(nil)) {
return nil, errors.Wrap(err, "duplicate key in primary index")
}
return nil, err
}

if err := indexAdder.Flush(ctx); err != nil {
if err, ok := err.(storagebase.DuplicateKeyError); ok {
if errors.HasType(err, (*storagebase.DuplicateKeyError)(nil)) {
return nil, errors.Wrap(err, "duplicate key in index")
}
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/read_import_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ type importFileContext struct {
func handleCorruptRow(ctx context.Context, fileCtx *importFileContext, err error) error {
log.Errorf(ctx, "%v", err)

if rowErr, isRowErr := err.(*importRowError); isRowErr && fileCtx.rejected != nil {
if rowErr := (*importRowError)(nil); errors.As(err, &rowErr) && fileCtx.rejected != nil {
fileCtx.rejected <- rowErr.row + "\n"
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ func removeDeadReplicas(
return nil, errors.Wrap(err, "loading MVCCStats")
}
err = storage.MVCCPutProto(ctx, batch, &ms, key, clock.Now(), nil /* txn */, &desc)
if wiErr, ok := err.(*roachpb.WriteIntentError); ok {
if wiErr := (*roachpb.WriteIntentError)(nil); errors.As(err, &wiErr) {
if len(wiErr.Intents) != 1 {
return nil, errors.Errorf("expected 1 intent, found %d: %s", len(wiErr.Intents), wiErr)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (f *formattedError) Error() string {

// Extract the fields.
var message, code, hint, detail, location string
if pqErr, ok := errors.UnwrapAll(f.err).(*pq.Error); ok {
if pqErr := (*pq.Error)(nil); errors.As(f.err, &pqErr) {
if pqErr.Severity != "" {
severity = pqErr.Severity
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func TestErrorReporting(t *testing.T) {
checked := checkAndMaybeShoutTo(tt.err, got.Log)
assert.Equal(t, tt.err, checked, "should return error unchanged")
assert.Equal(t, tt.wantSeverity, got.Severity, "wrong severity log")
_, gotCLI := got.Err.(*cliError)
gotCLI := errors.HasType(got.Err, (*cliError)(nil))
if tt.wantCLICause {
assert.True(t, gotCLI, "logged cause should be *cliError, got %T", got.Err)
} else {
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/quit.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func doDrain(
hardError, remainingWork, err = doDrainNoTimeout(ctx, c)
return err
})
if _, ok := err.(*contextutil.TimeoutError); ok {
if errors.HasType(err, (*contextutil.TimeoutError)(nil)) {
log.Infof(ctx, "drain timed out: %v", err)
err = errors.New("drain timeout")
}
Expand Down Expand Up @@ -255,7 +255,7 @@ func doShutdown(ctx context.Context, c serverpb.AdminClient) (hardError bool, er
}
}
})
if _, ok := err.(*contextutil.TimeoutError); !ok {
if !errors.HasType(err, (*contextutil.TimeoutError)(nil)) {
hardError = true
}
return hardError, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ If problems persist, please see %s.`

// Attempt to start the server.
if err := s.Start(ctx); err != nil {
if le, ok := err.(server.ListenError); ok {
if le := (*server.ListenError)(nil); errors.As(err, &le) {
const errorPrefix = "consider changing the port via --"
if le.Addr == serverCfg.Addr {
err = errors.Wrap(err, errorPrefix+cliflags.ListenAddr.Name)
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/zip.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,8 +719,8 @@ func dumpTableDataForZip(
if cErr := z.createError(name, err); cErr != nil {
return cErr
}
pqErr, ok := errors.UnwrapAll(err).(*pq.Error)
if !ok {
var pqErr *pq.Error
if !errors.As(err, &pqErr) {
// Not a SQL error. Nothing to retry.
break
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/internal/issues/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,8 @@ func (p *poster) parameters() []string {
}

func isInvalidAssignee(err error) bool {
e, ok := errors.Cause(err).(*github.ErrorResponse)
if !ok {
var e *github.ErrorResponse
if !errors.As(err, &e) {
return false
}
if e.Response.StatusCode != 422 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/cmd/reduce/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ func reduceSQL(path, contains string, workers int, verbose bool) (string, error)
}
cmd.Stdin = strings.NewReader(sql)
out, err := cmd.CombinedOutput()
switch err := err.(type) {
case *exec.Error:
if errors.Is(err.Err, exec.ErrNotFound) {
switch {
case errors.Is(err, (*exec.Error)(nil)):
if errors.Is(err, exec.ErrNotFound) {
log.Fatal(err)
}
case *os.PathError:
case errors.Is(err, (*os.PathError)(nil)):
log.Fatal(err)
}
return containsRE.Match(out)
Expand Down
30 changes: 11 additions & 19 deletions pkg/cmd/roachprod/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"fmt"
"os/exec"

crdberrors "github.com/cockroachdb/errors"
"github.com/cockroachdb/errors"
)

// Error is an interface for error types used by the main.wrap() function
Expand Down Expand Up @@ -54,7 +54,7 @@ func (e Cmd) ExitCode() int {

// Format passes formatting responsibilities to cockroachdb/errors
func (e Cmd) Format(s fmt.State, verb rune) {
crdberrors.FormatError(e, s, verb)
errors.FormatError(e, s, verb)
}

// Unwrap the wrapped the non-cockroach command error.
Expand All @@ -80,7 +80,7 @@ func (e Cockroach) ExitCode() int {

// Format passes formatting responsibilities to cockroachdb/errors
func (e Cockroach) Format(s fmt.State, verb rune) {
crdberrors.FormatError(e, s, verb)
errors.FormatError(e, s, verb)
}

// Unwrap the wrapped cockroach error.
Expand All @@ -104,7 +104,7 @@ func (e SSH) ExitCode() int {

// Format passes formatting responsibilities to cockroachdb/errors
func (e SSH) Format(s fmt.State, verb rune) {
crdberrors.FormatError(e, s, verb)
errors.FormatError(e, s, verb)
}

// Unwrap the wrapped SSH error.
Expand All @@ -128,7 +128,7 @@ func (e Unclassified) ExitCode() int {

// Format passes formatting responsibilities to cockroachdb/errors
func (e Unclassified) Format(s fmt.State, verb rune) {
crdberrors.FormatError(e, s, verb)
errors.FormatError(e, s, verb)
}

// Unwrap the wrapped unclassified error.
Expand Down Expand Up @@ -173,26 +173,18 @@ func ClassifyCockroachError(err error) Error {

// Extract the an ExitError from err's error tree or (nil, false) if none exists.
func asExitError(err error) (*exec.ExitError, bool) {
if exitErr, ok := crdberrors.If(err, func(err error) (interface{}, bool) {
if err, ok := err.(*exec.ExitError); ok {
return err, true
}
return nil, false
}); ok {
return exitErr.(*exec.ExitError), true
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
return exitErr, true
}
return nil, false
}

// AsError extracts the Error from err's error tree or (nil, false) if none exists.
func AsError(err error) (Error, bool) {
if rpErr, ok := crdberrors.If(err, func(err error) (interface{}, bool) {
if rpErr, ok := err.(Error); ok {
return rpErr, true
}
return nil, false
}); ok {
return rpErr.(Error), true
var e Error
if errors.As(err, &e) {
return e, true
}
return nil, false
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/roachprod/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,12 @@ type clusterAlreadyExistsError struct {
name string
}

func (e clusterAlreadyExistsError) Error() string {
func (e *clusterAlreadyExistsError) Error() string {
return fmt.Sprintf("cluster %s already exists", e.name)
}

func newClusterAlreadyExistsError(name string) error {
return clusterAlreadyExistsError{name: name}
return &clusterAlreadyExistsError{name: name}
}

var createCmd = &cobra.Command{
Expand Down Expand Up @@ -355,7 +355,7 @@ Local Clusters
if retErr == nil || clusterName == config.Local {
return
}
if _, ok := retErr.(clusterAlreadyExistsError); ok {
if errors.HasType(retErr, (*clusterAlreadyExistsError)(nil)) {
return
}
fmt.Fprintf(os.Stderr, "Cleaning up partially-created cluster (prev err: %s)\n", retErr)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachprod/vm/aws/support.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (p *Provider) runCommand(args []string) ([]byte, error) {
cmd.Stderr = &stderrBuf
output, err := cmd.Output()
if err != nil {
if exitErr, ok := err.(*exec.ExitError); ok {
if exitErr := (*exec.ExitError)(nil); errors.As(err, &exitErr) {
log.Println(string(exitErr.Stderr))
}
return nil, errors.Wrapf(err, "failed to run: aws %s: stderr: %v",
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/roachprod/vm/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,8 @@ func (p *Provider) createVNets(
if err == nil {
return group, true, nil
}
if detail, ok := err.(autorest.DetailedError); ok {
var detail autorest.DetailedError
if errors.As(err, &detail) {
if code, ok := detail.StatusCode.(int); ok {
if code == 404 {
return resources.Group{}, false, nil
Expand Down
Loading

0 comments on commit a8ae1bf

Please sign in to comment.