diff --git a/changes/20250807151301.feature b/changes/20250807151301.feature new file mode 100644 index 0000000000..0eb42f0c10 --- /dev/null +++ b/changes/20250807151301.feature @@ -0,0 +1 @@ +:sparkles: `commonerrors` Add ErrFailed to be used as a generic error where an error is an expected and valid state that should be distinguished from a system error diff --git a/utils/commonerrors/errors.go b/utils/commonerrors/errors.go index 34d23f3402..cc95c340bb 100644 --- a/utils/commonerrors/errors.go +++ b/utils/commonerrors/errors.go @@ -11,6 +11,8 @@ import ( "errors" "fmt" "strings" + + "github.com/hashicorp/go-multierror" ) // List of common errors used to qualify and categorise go errors @@ -45,19 +47,30 @@ var ( ErrEOF = errors.New("end of file") ErrMalicious = errors.New("suspected malicious intent") ErrOutOfRange = errors.New("out of range") + // ErrFailed should be used as a generic error where an error is an expected and valid state. + // For example a failing command may cause subprocess.Execute to return an error if the command exits with 1 but + // this wouldn't be a system error, and you might want to distinguish between this and the subprocess wrapper erroring + // when you pass the message up the stack. + ErrFailed = errors.New(failureStr) // ErrWarning is a generic error that can be used when an error should be raised but it shouldn't necessary be // passed up the chain, for example in cases where an error should be logged but the program should continue. In // these situations it should be handled immediately and then ignored/set to nil. ErrWarning = errors.New(warningStr) ) -const warningStr = "warning" +const ( + warningStr = "warning" + failureStr = "failed" +) -var warningStrPrepend = fmt.Sprintf("%v: ", warningStr) +var ( + warningStrPrepend = fmt.Sprintf("%v%v ", warningStr, string(TypeReasonErrorSeparator)) + failureStrPrepend = fmt.Sprintf("%v%v ", failureStr, string(TypeReasonErrorSeparator)) +) // IsCommonError returns whether an error is a commonerror func IsCommonError(target error) bool { - return Any(target, ErrNotImplemented, ErrNoExtension, ErrNoLogger, ErrNoLoggerSource, ErrNoLogSource, ErrUndefined, ErrInvalidDestination, ErrTimeout, ErrLocked, ErrStaleLock, ErrExists, ErrNotFound, ErrUnsupported, ErrUnavailable, ErrWrongUser, ErrUnauthorised, ErrUnknown, ErrInvalid, ErrConflict, ErrMarshalling, ErrCancelled, ErrEmpty, ErrUnexpected, ErrTooLarge, ErrForbidden, ErrCondition, ErrEOF, ErrMalicious, ErrWarning, ErrOutOfRange) + return Any(target, ErrNotImplemented, ErrNoExtension, ErrNoLogger, ErrNoLoggerSource, ErrNoLogSource, ErrUndefined, ErrInvalidDestination, ErrTimeout, ErrLocked, ErrStaleLock, ErrExists, ErrNotFound, ErrUnsupported, ErrUnavailable, ErrWrongUser, ErrUnauthorised, ErrUnknown, ErrInvalid, ErrConflict, ErrMarshalling, ErrCancelled, ErrEmpty, ErrUnexpected, ErrTooLarge, ErrForbidden, ErrCondition, ErrEOF, ErrMalicious, ErrWarning, ErrOutOfRange, ErrFailed) } // Any determines whether the target error is of the same type as any of the errors `err` @@ -183,6 +196,8 @@ func deserialiseCommonError(errStr string) (bool, error) { return true, ErrWarning case CorrespondTo(ErrOutOfRange, errStr): return true, ErrOutOfRange + case CorrespondTo(ErrFailed, errStr): + return true, ErrFailed } return false, ErrUnknown } @@ -203,11 +218,24 @@ func ConvertContextError(err error) error { // IsWarning will return whether an error is actually a warning func IsWarning(target error) bool { + return isSpecialCase(target, ErrWarning, warningStrPrepend) +} + +// IsFailure returns whether an error is unexpected (i.e. deviation from an expected state) but not a system error e.g. test failure +func IsFailure(target error) bool { + return isSpecialCase(target, ErrFailed, failureStrPrepend) +} + +func isSpecialCase(target, specialErrorCase error, prefix string) bool { if target == nil { return false } - if Any(target, ErrWarning) { + if Any(target, specialErrorCase) { + return true + } + + if strings.HasPrefix(target.Error(), prefix) { return true } @@ -216,7 +244,33 @@ func IsWarning(target error) bool { return false } - return strings.TrimSuffix(target.Error(), underlyingErr.Error()) == warningStrPrepend + return strings.TrimSuffix(target.Error(), underlyingErr.Error()) == prefix +} + +// MarkAsFailure will tent an error as failure. It will retain its original error type but IsFailure should return true. +func MarkAsFailure(err error) error { + if Any(err, nil, ErrFailed) { + return err + } + result := multierror.Append(err, ErrFailed) + result.ErrorFormat = func(e []error) string { + builder := strings.Builder{} + _, _ = builder.WriteString(failureStr) + for i := range e { + if None(e[i], nil, ErrFailed) { + _, _ = builder.WriteString(string(TypeReasonErrorSeparator)) + _, _ = builder.WriteString(" ") + _, _ = builder.WriteString(e[i].Error()) + } + } + return builder.String() + } + return result.ErrorOrNil() +} + +// NewFailure creates a failure object. +func NewFailure(msgFormat string, args ...any) error { + return Newf(ErrFailed, msgFormat, args...) } // NewWarning will create a warning wrapper around an existing commonerror so that it can be easily recovered. If the @@ -293,7 +347,8 @@ func Errorf(targetErr error, format string, args ...any) error { } } -// WrapError wraps an error into a particular targetError. However, if the original error has to do with a contextual error (i.e. ErrCancelled or ErrTimeout), it will be passed through without having is type changed. +// WrapError wraps an error into a particular targetError. However, if the original error has to do with a contextual error (i.e. ErrCancelled or ErrTimeout) or should be considered as a failure rather than an error, it will be passed through without having its type changed. +// Same is true with warnings. // This method should be used to safely wrap errors without losing information about context control information. // If the target error is not set, the wrapped error will be of type ErrUnknown. func WrapError(targetError, originalError error, msg string) error { @@ -302,7 +357,7 @@ func WrapError(targetError, originalError error, msg string) error { tErr = ErrUnknown } origErr := ConvertContextError(originalError) - if Any(origErr, ErrTimeout, ErrCancelled) { + if Any(origErr, ErrTimeout, ErrCancelled, ErrWarning, ErrFailed) { tErr = origErr } if originalError == nil { @@ -312,7 +367,8 @@ func WrapError(targetError, originalError error, msg string) error { if cleansedMsg == "" { return New(tErr, originalError.Error()) } else { - return Errorf(tErr, "%v%v %v", cleansedMsg, string(TypeReasonErrorSeparator), originalError.Error()) + return Errorf( + tErr, "%v%v %v", cleansedMsg, string(TypeReasonErrorSeparator), originalError.Error()) } } } diff --git a/utils/commonerrors/errors_test.go b/utils/commonerrors/errors_test.go index dc5227cf6b..7c433dda7d 100644 --- a/utils/commonerrors/errors_test.go +++ b/utils/commonerrors/errors_test.go @@ -27,6 +27,8 @@ func TestAny(t *testing.T) { assert.False(t, Any(nil, ErrInvalid, ErrUnknown)) assert.True(t, Any(fmt.Errorf("an error %w", ErrNotImplemented), ErrInvalid, ErrNotImplemented, ErrUnknown)) assert.False(t, Any(fmt.Errorf("an error %w", ErrNotImplemented), ErrInvalid, ErrUnknown)) + assert.True(t, Any(WrapError(ErrUnexpected, WrapError(ErrNotFound, WrapError(ErrFailed, errors.New(faker.Sentence()), "failure!!!"), faker.Sentence()), faker.Sentence()), ErrFailed)) + assert.False(t, Any(WrapError(ErrUnexpected, WrapError(ErrNotFound, WrapError(ErrFailed, errors.New(faker.Sentence()), "failure!!!"), faker.Sentence()), faker.Sentence()), ErrUnexpected, ErrNotFound)) } func TestNone(t *testing.T) { @@ -38,6 +40,7 @@ func TestNone(t *testing.T) { assert.False(t, None(nil, nil, ErrInvalid, ErrNotImplemented, ErrUnknown)) assert.False(t, None(fmt.Errorf("an error %w", ErrNotImplemented), ErrInvalid, ErrNotImplemented, ErrUnknown)) assert.True(t, None(fmt.Errorf("an error %w", ErrNotImplemented), ErrInvalid, ErrUnknown)) + assert.False(t, None(WrapError(ErrUnexpected, WrapError(ErrNotFound, WrapError(ErrFailed, errors.New(faker.Sentence()), "failure!!!"), faker.Sentence()), faker.Sentence()), ErrFailed, ErrUnauthorised)) } func TestCorrespondTo(t *testing.T) { @@ -112,6 +115,7 @@ func TestIsCommonError(t *testing.T) { ErrMalicious, ErrWarning, ErrOutOfRange, + ErrFailed, } for i := range commonErrors { assert.True(t, IsCommonError(commonErrors[i])) @@ -124,8 +128,22 @@ func TestIsWarning(t *testing.T) { assert.True(t, IsWarning(ErrWarning)) assert.False(t, IsWarning(ErrUnexpected)) assert.False(t, IsWarning(nil)) - assert.True(t, IsWarning(fmt.Errorf("%w: i am i warning", ErrWarning))) - assert.True(t, IsWarning(fmt.Errorf("%w: i am i warning too: %v", ErrWarning, ErrUnknown))) + assert.True(t, IsWarning(fmt.Errorf("%w: i am a warning", ErrWarning))) + assert.True(t, IsWarning(fmt.Errorf("%w: i am a warning too: %v", ErrWarning, ErrUnknown))) +} + +func TestIsFailure(t *testing.T) { + assert.True(t, IsFailure(ErrFailed)) + assert.False(t, IsFailure(ErrUnexpected)) + assert.True(t, IsFailure(MarkAsFailure(ErrUnexpected))) + assert.True(t, Any(MarkAsFailure(ErrUnexpected), ErrUnexpected)) + assert.True(t, Any(MarkAsFailure(ErrUnexpected), ErrFailed)) + assert.False(t, IsFailure(nil)) + assert.True(t, IsFailure(NewFailure("i am a failure and I know it"))) + assert.True(t, IsFailure(fmt.Errorf("%w: i am a failure", ErrFailed))) + assert.True(t, IsFailure(fmt.Errorf("%w: i am a failure too: %v", ErrFailed, ErrUnknown))) + assert.True(t, IsFailure(fmt.Errorf("%v: %w", ErrFailed, ErrUnknown))) + assert.True(t, IsFailure(fmt.Errorf("%v: %w: i am a failure too too", ErrFailed, ErrUnknown))) } func TestNewWarning(t *testing.T) { diff --git a/utils/go.mod b/utils/go.mod index 29163bb93d..681ddea0c8 100644 --- a/utils/go.mod +++ b/utils/go.mod @@ -26,6 +26,7 @@ require ( github.com/gogs/chardet v0.0.0-20211120154057-b7413eaefb8f github.com/hashicorp/go-cleanhttp v0.5.2 github.com/hashicorp/go-hclog v1.6.3 + github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-retryablehttp v0.7.8 github.com/iamacarpet/go-win64api v0.0.0-20240507095429-873e84e85847 github.com/joho/godotenv v1.5.1 @@ -73,6 +74,7 @@ require ( github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect github.com/google/cabbie v1.0.2 // indirect github.com/google/glazier v0.0.0-20211029225403-9f766cca891d // indirect + github.com/hashicorp/errwrap v1.0.0 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/kevinburke/ssh_config v1.2.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect diff --git a/utils/go.sum b/utils/go.sum index 4633f517bd..990295e6e7 100644 --- a/utils/go.sum +++ b/utils/go.sum @@ -126,10 +126,14 @@ github.com/google/subcommands v1.2.0/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/winops v0.0.0-20210803215038-c8511b84de2b/go.mod h1:ShbX8v8clPm/3chw9zHVwtW3QhrFpL8mXOwNxClt4pg= github.com/groob/plist v0.0.0-20210519001750-9f754062e6d6/go.mod h1:itkABA+w2cw7x5nYUS/pLRef6ludkZKOigbROmCTaFw= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48= github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k= github.com/hashicorp/go-hclog v1.6.3/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/go-retryablehttp v0.7.8 h1:ylXZWnqa7Lhqpk0L1P1LzDtGcCR0rPVUrx/c8Unxc48= github.com/hashicorp/go-retryablehttp v0.7.8/go.mod h1:rjiScheydd+CxvumBsIrFKlx3iS0jrZ7LvzFGFmuKbw= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=