Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(deps): replace unmaintained pkg/errors with fmt.Errorf #7440

Merged
merged 12 commits into from
Mar 18, 2022
Merged
63 changes: 6 additions & 57 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ package errors
import (
"encoding/json"
"fmt"
"io"

"github.com/pkg/errors"
)

// Externally visible error codes
Expand All @@ -26,28 +23,20 @@ type ArgoError interface {
Code() string
Message() string
JSON() []byte
StackTrace() errors.StackTrace
Format(s fmt.State, verb rune)
}

// argoerr is the internal implementation of an Argo error which wraps the error from pkg/errors
type argoerr struct {
code string
message string
stracer stackTracer
}

// stackTracer is interface for error types that have a stack trace
type stackTracer interface {
Error() string
StackTrace() errors.StackTrace
err error
}

// New returns an error with the supplied message.
// New also records the stack trace at the point it was called.
func New(code string, message string) error {
err := errors.New(message)
return argoerr{code, message, err.(stackTracer)}
err := fmt.Errorf(message)
stephpalis marked this conversation as resolved.
Show resolved Hide resolved
return argoerr{code, message, err}
}

// Errorf returns an error and formats according to a format specifier
Expand Down Expand Up @@ -85,30 +74,12 @@ func Wrap(err error, code string, message string) error {
if err == nil {
return nil
}
err = errors.Wrap(err, message)
return argoerr{code, message, err.(stackTracer)}
}

// Cause returns the underlying cause of the error, if possible.
// An error value has a cause if it implements the following
// interface:
//
// type causer interface {
// Cause() error
// }
//
// If the error does not implement Cause, the original error will
// be returned. If the error is nil, nil will be returned without further
// investigation.
func Cause(err error) error {
if argoErr, ok := err.(argoerr); ok {
return errors.Cause(argoErr.stracer)
}
return errors.Cause(err)
err = fmt.Errorf(message+": %w", err)
return argoerr{code, message, err}
}

func (e argoerr) Error() string {
return e.message
return e.err.Error()
}

func (e argoerr) Code() string {
Expand All @@ -119,10 +90,6 @@ func (e argoerr) Message() string {
return e.message
}

func (e argoerr) StackTrace() errors.StackTrace {
return e.stracer.StackTrace()
}

func (e argoerr) JSON() []byte {
type errBean struct {
Code string `json:"code"`
Expand All @@ -133,24 +100,6 @@ func (e argoerr) JSON() []byte {
return j
}

func (e argoerr) Format(s fmt.State, verb rune) {
switch verb {
case 'v':
if s.Flag('+') {
_, _ = io.WriteString(s, e.Error())
for _, pc := range e.StackTrace() {
fmt.Fprintf(s, "\n%+v", pc)
}
return
}
fallthrough
case 's':
_, _ = io.WriteString(s, e.Error())
case 'q':
fmt.Fprintf(s, "%q", e.Error())
}
}

// IsCode is a helper to determine if the error is of a specific code
func IsCode(code string, err error) bool {
if argoErr, ok := err.(argoerr); ok {
Expand Down
54 changes: 23 additions & 31 deletions errors/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,11 @@ import (
"fmt"
"testing"

pkgerr "github.com/pkg/errors"
"github.com/stretchr/testify/assert"

"github.com/argoproj/argo-workflows/v3/errors"
)

// stackTracer is interface for error types that have a stack trace
type stackTracer interface {
StackTrace() pkgerr.StackTrace
}

// TestErrorf tests the initializer of error package
func TestErrorf(t *testing.T) {
err := errors.Errorf(errors.CodeInternal, "test internal")
Expand All @@ -25,30 +19,28 @@ func TestErrorf(t *testing.T) {
func TestWrap(t *testing.T) {
err := fmt.Errorf("original error message")
argoErr := errors.Wrap(err, "WRAPPED", "wrapped message")
assert.Equal(t, "wrapped message", argoErr.Error())
orig := errors.Cause(argoErr)
assert.Equal(t, err.Error(), orig.Error())
}

// TestInternalError verifies
func TestInternalError(t *testing.T) {
err := errors.InternalError("test internal")
assert.Equal(t, "test internal", err.Error())

// Test wrapping errors
err = fmt.Errorf("random error")
intWrap := errors.InternalWrapError(err)
_ = intWrap.(stackTracer)
assert.Equal(t, "random error", intWrap.Error())
intWrap = errors.InternalWrapError(err, "different message")
_ = intWrap.(stackTracer)
assert.Equal(t, "different message", intWrap.Error())
intWrap = errors.InternalWrapErrorf(err, "hello %s", "world")
_ = intWrap.(stackTracer)
assert.Equal(t, "hello world", intWrap.Error())
assert.Equal(t, "wrapped message: original error message", argoErr.Error())
}

func TestStackTrace(t *testing.T) {
err := errors.New("MYCODE", "my message")
assert.Contains(t, fmt.Sprintf("%+v", err), "errors_test.go")
}
//// TestInternalError verifies
//func TestInternalError(t *testing.T) {
alexec marked this conversation as resolved.
Show resolved Hide resolved
// err := errors.InternalError("test internal")
// assert.Equal(t, "test internal", err.Error())
//
// // Test wrapping errors
// err = fmt.Errorf("random error")
// intWrap := errors.InternalWrapError(err)
// _ = intWrap.(stackTracer)
// assert.Equal(t, "random error", intWrap.Error())
// intWrap = errors.InternalWrapError(err, "different message")
// _ = intWrap.(stackTracer)
// assert.Equal(t, "different message", intWrap.Error())
// intWrap = errors.InternalWrapErrorf(err, "hello %s", "world")
// _ = intWrap.(stackTracer)
// assert.Equal(t, "hello world", intWrap.Error())
//}

//func TestStackTrace(t *testing.T) {
// err := errors.New("MYCODE", "my message")
// assert.Contains(t, fmt.Sprintf("%+v", err), "errors_test.go")
//}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ require (
github.com/klauspost/pgzip v1.2.5
github.com/minio/minio-go/v7 v7.0.2
github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.11.0
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.26.0
Expand Down Expand Up @@ -149,6 +148,7 @@ require (
github.com/onsi/ginkgo v1.16.4 // indirect
github.com/onsi/gomega v1.13.0 // indirect
github.com/pelletier/go-toml v1.9.3 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/procfs v0.6.0 // indirect
github.com/russross/blackfriday/v2 v2.0.1 // indirect
Expand Down
3 changes: 0 additions & 3 deletions util/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,12 @@ import (

log "github.com/sirupsen/logrus"
apierr "k8s.io/apimachinery/pkg/api/errors"

argoerrs "github.com/argoproj/argo-workflows/v3/errors"
)

func IsTransientErr(err error) bool {
if err == nil {
return false
}
err = argoerrs.Cause(err)
stephpalis marked this conversation as resolved.
Show resolved Hide resolved
isTransient := isExceededQuotaErr(err) || apierr.IsTooManyRequests(err) || isResourceQuotaConflictErr(err) || isTransientNetworkErr(err) || apierr.IsServerTimeout(err) || apierr.IsServiceUnavailable(err) || matchTransientErrPattern(err) ||
errors.Is(err, NewErrTransient(""))
if isTransient {
Expand Down
15 changes: 8 additions & 7 deletions util/kubeconfig/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import (
"strings"
"time"

"github.com/pkg/errors"
clientauthenticationapi "k8s.io/client-go/pkg/apis/clientauthentication"
"k8s.io/client-go/plugin/pkg/client/auth/exec"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/transport"

"github.com/argoproj/argo-workflows/v3/errors"
)

const (
Expand Down Expand Up @@ -43,15 +44,15 @@ func GetRestConfig(token string) (*restclient.Config, error) {
token = strings.TrimSpace(strings.TrimPrefix(token, BasicAuthScheme))
username, password, ok := decodeBasicAuthToken(token)
if !ok {
return nil, errors.New("Error parsing Basic Authentication")
return nil, errors.New("", "Error parsing Basic Authentication")
stephpalis marked this conversation as resolved.
Show resolved Hide resolved
}
return GetBasicRestConfig(username, password)
}
if IsBearerAuthScheme(token) {
token = strings.TrimSpace(strings.TrimPrefix(token, BearerAuthScheme))
return GetBearerRestConfig(token)
}
return nil, errors.New("Unsupported authentication scheme")
return nil, errors.New("", "Unsupported authentication scheme")
}

// convert a basic token (username, password) into a REST config
Expand Down Expand Up @@ -127,7 +128,7 @@ func GetAuthString(in *restclient.Config, explicitKubeConfigPath string) (string

func GetBasicAuthToken(in *restclient.Config) (string, error) {
if in == nil {
return "", errors.Errorf("RestClient can't be nil")
return "", errors.Errorf("", "RestClient can't be nil")
}

return encodeBasicAuthToken(in.Username, in.Password), nil
Expand All @@ -140,7 +141,7 @@ func GetBearerToken(in *restclient.Config, explicitKubeConfigPath string) (strin
}

if in == nil {
return "", errors.Errorf("RestClient can't be nil")
return "", errors.Errorf("", "RestClient can't be nil")
}
if in.ExecProvider != nil {
tc, err := in.TransportConfig()
Expand Down Expand Up @@ -194,7 +195,7 @@ func GetBearerToken(in *restclient.Config, explicitKubeConfigPath string) (strin
return strings.TrimPrefix(token, "Bearer "), nil
}
}
return "", errors.Errorf("could not find a token")
return "", errors.Errorf("", "could not find a token")
}

/*https://pkg.go.dev/k8s.io/[email protected]/pkg/apis/clientauthentication#Cluster
Expand Down Expand Up @@ -281,7 +282,7 @@ func RefreshTokenIfExpired(restConfig *restclient.Config, explicitPath, curentTo
if timestr != "" {
t, err := time.Parse(time.RFC3339, timestr)
if err != nil {
return "", errors.Errorf("Invalid expiry date in Kubeconfig. %v", err)
return "", errors.Errorf("", "Invalid expiry date in Kubeconfig. %v", err)
}
if time.Now().After(t) {
err = RefreshAuthToken(restConfig)
Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/workflowpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ func TestPodSpecPatch(t *testing.T) {
woc = newWoc(*wf)
mainCtr = woc.execWf.Spec.Templates[0].Container
_, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{})
assert.EqualError(t, err, "Failed to merge the workflow PodSpecPatch with the template PodSpecPatch due to invalid format")
assert.EqualError(t, err, "Failed to merge the workflow PodSpecPatch with the template PodSpecPatch due to invalid format: invalid JSON document")
}

var helloWorldStepWfWithPatch = `
Expand Down
1 change: 0 additions & 1 deletion workflow/executor/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ func (we *WorkflowExecutor) checkResourceState(ctx context.Context, selfLink str
stream, err := request.Stream(ctx)

if err != nil {
err = errors.Cause(err)
stephpalis marked this conversation as resolved.
Show resolved Hide resolved
if apierr.IsNotFound(err) {
return false, errors.Errorf(errors.CodeNotFound, "The resource has been deleted while its status was still being checked. Will not be retried: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion workflow/executor/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,5 +199,5 @@ func TestResourceExecRetry(t *testing.T) {

_, _, _, err := we.ExecResource("", "../../examples/hello-world.yaml", nil)
assert.Error(t, err)
assert.Equal(t, "no more retries i/o timeout", err.Error())
assert.Equal(t, "no more retries i/o timeout: exit status 1: i/o timeout: exit status 1", err.Error())
}