review

Author: joshjennings98

changes/20250807151301.feature

@@ -1 +1 @@
-:sparkles: `commonerrors` Add ErrFailed
+: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

@@ -45,7 +45,11 @@ var (
 	ErrEOF                = errors.New("end of file")
 	ErrMalicious          = errors.New("suspected malicious intent")
 	ErrOutOfRange         = errors.New("out of range")
-	ErrFailed             = errors.New("failed")
+	// ErrFailed should be used as a generic error where an error is an expected and valid state.
+	// For example a failing command may cause subproccess.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("failed")
 	// 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.
@@ -305,7 +309,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 {
@@ -315,7 +319,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)
 		}
 	}
 }

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