Skip to content

Commit

Permalink
fix #138: separate Action and InterruptibleAction handling
Browse files Browse the repository at this point in the history
  • Loading branch information
kamilsk committed Nov 25, 2019
1 parent 4f917c3 commit 5e80d01
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 135 deletions.
1 change: 0 additions & 1 deletion .github/FUNDING.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
# github: [kamilsk, octolab, octopot]
patreon: octolab
custom: ['https://www.buymeacoffee.com/kamilsk', 'https://click.octolab.net/digitalocean']
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
## 💡 Idea

The package based on [github.com/Rican7/retry](https://github.com/Rican7/retry) but fully reworked
and focused on integration with the 🚧 [breaker][] package.
and focused on integration with the 🚧 [breaker][] and the built-in [context][] packages.

Full description of the idea is available [here][design].

Expand Down Expand Up @@ -216,6 +216,7 @@ made with ❤️ for everyone
[cli]: https://github.com/kamilsk/retry.cli
[cli.demo]: https://asciinema.org/a/150367
[cli.preview]: https://asciinema.org/a/150367.png
[context]: https://pkg.go.dev/context
[design]: https://www.notion.so/octolab/retry-cab5722faae445d197e44fbe0225cc98?r=0b753cbf767346f5a6fd51194829a2f3
[egg]: https://github.com/kamilsk/egg
[promo]: https://github.com/kamilsk/retry
Expand Down
41 changes: 41 additions & 0 deletions context.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package retry

import "context"

// TryContext takes an interruptable action and performs it, repetitively, until successful.
// It uses the Context as a Breaker to prevent unnecessary action execution.
//
// Optionally, strategies may be passed that assess whether or not an attempt
// should be made.
//
// TODO:v5 not quite honest implementation
func TryContext(
ctx context.Context,
action func(ctx context.Context, attempt uint) error,
strategies ...func(attempt uint, err error) bool,
) (err error) {

// TODO:v5 will be removed
defer func() {
if r := recover(); r != nil {
err = result{err, r}
}
}()

for attempt, repeat, should := uint(0), len(strategies), true; should; attempt++ {
for i := 0; should && i < repeat; i++ {
should = should && ctx.Err() == nil && strategies[i](attempt, err)
}

if !should && ctx.Err() != nil {
return Interrupted
}

if should {
err = action(ctx, attempt)
should = err != nil
}
}

return err
}
83 changes: 83 additions & 0 deletions context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package retry

import (
"context"
"errors"
"reflect"
"testing"
)

func TestTryContext(t *testing.T) {
type Assert struct {
Attempts uint
Error func(error) bool
}

tests := map[string]struct {
ctx func() context.Context
strategies []func(attempt uint, err error) bool
error error
assert Assert
}{
"zero iterations": {
func() context.Context {
ctx, cancel := context.WithCancel(context.Background())
cancel()
return ctx
},
[]func(attempt uint, err error) bool{delay(delta), limit(10000)},
errors.New("zero iterations"),
Assert{0, func(err error) bool { return IsInterrupted(err) && err.Error() == string(Interrupted) }},
},
"one iteration": {
context.Background,
nil,
nil,
Assert{1, func(err error) bool { return err == nil }},
},
"two iterations": {
context.Background,
[]func(attempt uint, err error) bool{limit(2)},
errors.New("two iterations"),
Assert{2, func(err error) bool { return err != nil && err.Error() == "two iterations" }},
},
"three iterations": {
context.Background,
[]func(attempt uint, err error) bool{limit(3)},
errors.New("three iterations"),
Assert{3, func(err error) bool { return err != nil && err.Error() == "three iterations" }},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
var total uint
action := func(ctx context.Context, attempt uint) error {
total = attempt + 1
return test.error
}
err := TryContext(test.ctx(), action, test.strategies...)
if !test.assert.Error(err) {
t.Error("fail error assertion")
}
if _, is := IsRecovered(err); is {
t.Error("recovered panic is not expected")
}
if test.assert.Attempts != total {
t.Errorf("expected %d attempts, obtained %d", test.assert.Attempts, total)
}
})
}
// TODO:v5 will be removed
t.Run("unexpected panic", func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
err := TryContext(ctx, func(context.Context, uint) error { panic("Catch Me If You Can") })
cause, is := IsRecovered(err)
if !is {
t.Fatal("recovered panic is expected")
}
if !reflect.DeepEqual(cause, "Catch Me If You Can") {
t.Fatal("Catch Me If You Can is expected")
}
cancel()
})
}
2 changes: 1 addition & 1 deletion error.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package retry
// Error defines package errors.
type Error string

// Error implements the error interface.
// Error returns a string representation of an error.
func (err Error) Error() string {
return string(err)
}
Expand Down
9 changes: 1 addition & 8 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,7 @@ import (
var generator = rand.New(rand.NewSource(0))

func Example() {
what := func(uint) (err error) {
defer func() {
if r := recover(); r != nil {
err = fmt.Errorf("unexpected panic: %v", r)
}
}()
return SendRequest()
}
what := func(uint) (err error) { return SendRequest() }

how := retry.How{
strategy.Limit(5),
Expand Down
17 changes: 8 additions & 9 deletions interface.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
package retry

import "context"

// Action defines a callable function that package retry can handle.
type Action func(attempt uint) error

// InterruptibleAction defines a callable function that package retry can handle.
type InterruptibleAction func(ctx context.Context, attempt uint) error

// A Breaker carries a cancellation signal to break an action execution.
//
// It is a subset of context.Context and github.com/kamilsk/breaker.Breaker.
Expand All @@ -18,19 +26,10 @@ type BreakCloser interface {
Close()
}

// Action defines a callable function that package retry can handle.
type Action func(attempt uint) error

// How is an alias for batch of Strategies.
//
// how := retry.How{
// strategy.Limit(3),
// }
//
type How []func(attempt uint, err error) bool

// Interface defines a behavior of stateful executor of Actions in parallel.
// TODO:v5 complete the draft
type Interface interface {
Try(Breaker, Action, ...How) Interface
}
7 changes: 7 additions & 0 deletions internal/draft.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
// +build draft

package internal

import "github.com/kamilsk/retry/v4"

// Interface defines a behavior of stateful executor of Actions in parallel.
type Interface interface {
Try(retry.Breaker, retry.Action, ...retry.How) Interface
}
33 changes: 5 additions & 28 deletions retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@
// to perform actions repetitively until successful.
package retry

import (
"context"
"sync/atomic"
)
import "sync/atomic"

// Retry takes action and performs it, repetitively, until successful.
// Retry takes an action and performs it, repetitively, until successful.
// When it is done it releases resources associated with the Breaker.
//
// Optionally, strategies may be passed that assess whether or not an attempt
Expand All @@ -22,7 +19,7 @@ func Retry(
return err
}

// Try takes action and performs it, repetitively, until successful.
// Try takes an action and performs it, repetitively, until successful.
//
// Optionally, strategies may be passed that assess whether or not an attempt
// should be made.
Expand All @@ -34,26 +31,6 @@ func Try(
return retry(breaker, action, strategies...)
}

// TryContext takes action and performs it, repetitively, until successful.
// It uses the Context as a Breaker to prevent unnecessary action execution.
//
// Optionally, strategies may be passed that assess whether or not an attempt
// should be made.
func TryContext(
ctx context.Context,
action func(ctx context.Context, attempt uint) error,
strategies ...func(attempt uint, err error) bool,
) error {
cascade, cancel := context.WithCancel(ctx)
err := retry(ctx, currying(cascade, action), strategies...)
cancel()
return err
}

func currying(ctx context.Context, action func(context.Context, uint) error) func(uint) error {
return func(attempt uint) error { return action(ctx, attempt) }
}

func retry(
breaker Breaker,
action func(attempt uint) error,
Expand All @@ -77,7 +54,7 @@ func retry(

select {
case <-breaker.Done():
atomic.CompareAndSwapUint32(&interrupted, 0, 1)
atomic.StoreUint32(&interrupted, 1)
return Interrupted
case err := <-done:
if _, is := IsRecovered(err); is {
Expand All @@ -98,5 +75,5 @@ func shouldAttempt(breaker *uint32, attempt uint, err error, strategies ...func(
should = should && strategies[i](attempt, err)
}

return should && !atomic.CompareAndSwapUint32(breaker, 1, 0)
return should && atomic.LoadUint32(breaker) == 0
}
75 changes: 0 additions & 75 deletions retry_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package retry

import (
"context"
"errors"
"reflect"
"testing"
Expand Down Expand Up @@ -145,77 +144,3 @@ func TestTry(t *testing.T) {
}
})
}

func TestTryContext(t *testing.T) {
type Assert struct {
Attempts uint
Error func(error) bool
}

tests := map[string]struct {
ctx func () context.Context
strategies []func(attempt uint, err error) bool
error error
assert Assert
}{
"zero iterations": {
func() context.Context {
ctx, cancel := context.WithCancel(context.Background())
cancel()
return ctx
},
[]func(attempt uint, err error) bool{delay(delta), limit(10000)},
errors.New("zero iterations"),
Assert{0, func(err error) bool { return IsInterrupted(err) && err.Error() == string(Interrupted) }},
},
"one iteration": {
context.Background,
nil,
nil,
Assert{1, func(err error) bool { return err == nil }},
},
"two iterations": {
context.Background,
[]func(attempt uint, err error) bool{limit(2)},
errors.New("two iterations"),
Assert{2, func(err error) bool { return err != nil && err.Error() == "two iterations" }},
},
"three iterations": {
context.Background,
[]func(attempt uint, err error) bool{limit(3)},
errors.New("three iterations"),
Assert{3, func(err error) bool { return err != nil && err.Error() == "three iterations" }},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
var total uint
action := func(ctx context.Context, attempt uint) error {
total = attempt + 1
return test.error
}
err := TryContext(test.ctx(), action, test.strategies...)
if !test.assert.Error(err) {
t.Error("fail error assertion")
}
if _, is := IsRecovered(err); is {
t.Error("recovered panic is not expected")
}
if test.assert.Attempts != total {
t.Errorf("expected %d attempts, obtained %d", test.assert.Attempts, total)
}
})
}
t.Run("unexpected panic", func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
err := TryContext(ctx, func(context.Context, uint) error { panic("Catch Me If You Can") })
cause, is := IsRecovered(err)
if !is {
t.Fatal("recovered panic is expected")
}
if !reflect.DeepEqual(cause, "Catch Me If You Can") {
t.Fatal("Catch Me If You Can is expected")
}
cancel()
})
}

0 comments on commit 5e80d01

Please sign in to comment.