✨ commonerrors Add ErrFailed (#669)

<!--
Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors.
All rights reserved.
SPDX-License-Identifier: Apache-2.0
-->
### Description

<!--
Please add any detail or context that would be useful to a reviewer.
-->

Add ErrFailed. This is useful because ErrUnexpected is used a lot as a
system error even when we are carrying out underlying tasks where a
failure isn't an error. This provides us another generic error but one
that we can use to distinguish between failures and system errors.

### Test Coverage

<!--
Please put an `x` in the correct box e.g. `[x]` to indicate the testing
coverage of this change.
-->

- [x]  This change is covered by existing or additional automated tests.
- [ ] Manual testing has been performed (and evidence provided) as
automated testing was not feasible.
- [ ] Additional tests are not required for this change (e.g.
documentation update).

---------

Co-authored-by: Adrien CABARBAYE <adrien.cabarbaye@arm.com>

Author: joshjennings98

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

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())
 		}
 	}
 }

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) {

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

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=