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

introduce MarkTemporary #28

Merged
merged 5 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions func.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ func DoValue[T any](ctx context.Context, policy *Policy, f func() (T, error)) (T
return v, nil
}

// short cut for calling isPermanent and Unwrap
if err, ok := err.(*permanentError); ok {
// short cut for calling Unwrap
if err, ok := err.(*myError); ok {
if err.temporary {
continue
}
Comment on lines +26 to +30
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider refactoring the error handling logic for myError to avoid duplication and improve maintainability.

+ func handleMyError(err error) (bool, error) {
+    if err, ok := err.(*myError); ok {
+        if err.temporary {
+            return true, nil
+        }
+        return false, err.error
+    }
+    return false, nil
+ }
+
+ // Usage in the code
+ shouldContinue, unwrappedErr := handleMyError(err)
+ if shouldContinue {
+     continue
+ }
+ if unwrappedErr != nil {
+     return zero, unwrappedErr
+ }

Also applies to: 47-50

return zero, err.error
}

Expand All @@ -41,5 +44,9 @@ func DoValue[T any](ctx context.Context, policy *Policy, f func() (T, error)) (T
if err := retrier.err; err != nil {
return zero, err
}
if err, ok := err.(*myError); ok {
// Unwrap the error if it's marked as temporary.
return zero, err.error
}
return zero, err
}
23 changes: 22 additions & 1 deletion func_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,32 @@ func TestDoValue_Success(t *testing.T) {

func TestDoValue_MarkPermanent(t *testing.T) {
permanentErr := errors.New("permanent error")
policy := &Policy{}
policy := &Policy{MaxCount: 10}
count := 0
_, err := DoValue(context.Background(), policy, func() (int, error) {
count++
return 0, MarkPermanent(permanentErr)
})
if err != permanentErr {
t.Errorf("want error is %#v, got %#v", err, permanentErr)
}
if count != 1 {
t.Errorf("want %d, got %d", 1, count)
}
}

func TestDoValue_MarkTemporary(t *testing.T) {
temporaryErr := errors.New("temporary error")
policy := &Policy{MaxCount: 10}
count := 0
_, err := DoValue(context.Background(), policy, func() (int, error) {
count++
return 0, MarkTemporary(temporaryErr)
})
if err != temporaryErr {
t.Errorf("want error is %#v, got %#v", err, temporaryErr)
}
if count != 10 {
t.Errorf("want %d, got %d", 10, count)
}
}
31 changes: 23 additions & 8 deletions retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,11 @@ func (p *Policy) Do(ctx context.Context, f func() error) error {
return nil
}

// short cut for calling isPermanent and Unwrap
if err, ok := err.(*permanentError); ok {
// short cut for calling Unwrap
if err, ok := err.(*myError); ok {
if err.temporary {
continue
}
Comment on lines +73 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider refactoring the error handling logic for myError in the Do function to avoid duplication and improve maintainability, similar to the suggestion made in func.go.

Also applies to: 94-97

return err.error
}

Expand All @@ -88,34 +91,46 @@ func (p *Policy) Do(ctx context.Context, f func() error) error {
if err := retrier.err; err != nil {
return err
}
if err, ok := err.(*myError); ok {
// Unwrap the error if it's marked as temporary.
return err.error
}
return err
}

type temporary interface {
Temporary() bool
}

var _ temporary = (*permanentError)(nil)
var _ temporary = (*myError)(nil)

type permanentError struct {
type myError struct {
error
temporary bool
}

// implements interface{ Temporary() bool }
// Inspecting errors https://dave.cheney.net/2014/12/24/inspecting-errors
func (e *permanentError) Temporary() bool {
return false
func (e *myError) Temporary() bool {
return e.temporary
}

// Unwrap implements errors.Wrapper.
func (e *permanentError) Unwrap() error {
func (e *myError) Unwrap() error {
return e.error
}

// MarkPermanent marks err as a permanent error.
// It returns the error that implements interface{ Temporary() bool } and Temporary() returns false.
func MarkPermanent(err error) error {
return &permanentError{err}
return &myError{err, false}
}

// MarkTemporary wraps an error as a temporary error, allowing retry mechanisms to handle it appropriately.
// This is especially useful in scenarios where errors may not require immediate termination of a process,
// but rather can be resolved through retrying operations.
func MarkTemporary(err error) error {
return &myError{err, true}
}

// Continue returns whether retrying should be continued.
Expand Down
24 changes: 23 additions & 1 deletion retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,35 @@ func TestDo_Success(t *testing.T) {

func TestDo_MarkPermanent(t *testing.T) {
permanentErr := errors.New("permanent error")
policy := &Policy{}
// TestDo_MarkPermanent checks that a permanent error stops retries after one occurrence as expected.
policy := &Policy{MaxCount: 10}
count := 0
err := policy.Do(context.Background(), func() error {
count++
return MarkPermanent(permanentErr)
})
if err != permanentErr {
t.Errorf("want error is %#v, got %#v", err, permanentErr)
}
if count != 1 {
t.Errorf("want %d, got %d", 1, count)
}
}
shogo82148 marked this conversation as resolved.
Show resolved Hide resolved

func TestDo_MarkTemporary(t *testing.T) {
temporaryErr := errors.New("temporary error")
policy := &Policy{MaxCount: 10}
count := 0
err := policy.Do(context.Background(), func() error {
count++
return MarkTemporary(temporaryErr)
})
if err != temporaryErr {
t.Errorf("want error is %#v, got %#v", err, temporaryErr)
}
if count != 10 {
t.Errorf("want %d, got %d", 10, count)
}
}

type customError bool
Expand Down