fix issue #123: fix critical runtime fault with unhandled panic in Re…

…try goroutine

retry.go

@@ -33,39 +33,65 @@ func Retry(deadline <-chan struct{}, action Action, strategies ...strategy.Strat
 		interrupt uint32
 	)
 	done := make(chan struct{})
+
 	go func() {
+		defer close(done)
+		defer panicHandler{}.recover(&err)
+
 		for attempt := uint(0); (attempt == 0 || err != nil) && shouldAttempt(attempt, err, strategies...) &&
 			!atomic.CompareAndSwapUint32(&interrupt, 1, 0); attempt++ {
 
 			err = action(attempt)
 		}
-		close(done)
 	}()
 
 	select {
 	case <-deadline:
 		atomic.CompareAndSwapUint32(&interrupt, 0, 1)
-		return errTimeout
+		return timeoutErr
 	case <-done:
 		return err
 	}
 }
 
-// IsTimeout checks if passed error is related to the incident deadline on Retry call.
+// IsRecovered checks that the error is related to unhandled Action's panic
+// and returns an original cause of panic.
+func IsRecovered(err error) (interface{}, bool) {
+	if h, is := err.(panicHandler); is {
+		return h.recovered, true
+	}
+	return nil, false
+}
+
+// IsTimeout checks that the error is related to the incident deadline on Retry call.
 func IsTimeout(err error) bool {
-	return err == errTimeout
+	return err == timeoutErr
 }
 
-var errTimeout = errors.New("operation timeout")
+type panicHandler struct {
+	error
+	recovered interface{}
+}
+
+func (panicHandler) recover(err *error) {
+	if r := recover(); r != nil {
+		*err = panicHandler{panicErr, r}
+	}
+}
+
+var (
+	panicErr   = errors.New("unhandled action's panic")
+	timeoutErr = errors.New("operation timeout")
+)
 
 // shouldAttempt evaluates the provided strategies with the given attempt to
 // determine if the Retry loop should make another attempt.
 func shouldAttempt(attempt uint, err error, strategies ...strategy.Strategy) bool {
-	shouldAttempt := true
+	should := true
 
-	for i, repeat := 0, len(strategies); shouldAttempt && i < repeat; i++ {
-		shouldAttempt = shouldAttempt && strategies[i](attempt, err)
+	for i, repeat := 0, len(strategies); should && i < repeat; i++ {
+		should = should && strategies[i](attempt, err)
 	}
 
-	return shouldAttempt
+	return should
 }

retry_test.go

@@ -5,20 +5,49 @@ import (
 	"testing"
 	"time"
 
-	"github.com/kamilsk/retry"
 	"github.com/kamilsk/retry/strategy"
+
+	. "github.com/kamilsk/retry"
 )
 
 func TestRetry(t *testing.T) {
 	action := func(attempt uint) error {
 		return nil
 	}
 
-	err := retry.Retry(nil, action)
+	err := Retry(nil, action)
 
 	if nil != err {
 		t.Error("expected a nil error")
 	}
+
+	if _, is := IsRecovered(err); is {
+		t.Error("recovered error is not expected")
+	}
+
+	if IsTimeout(err) {
+		t.Error("timeout error is not expected")
+	}
+}
+
+func TestRetry_PanickedAction(t *testing.T) {
+	action := func(attempt uint) error {
+		panic("catch me if you can")
+	}
+
+	err := Retry(nil, action, strategy.Infinite())
+
+	if nil == err {
+		t.Error("expected an error")
+	}
+
+	if expected, obtained := "unhandled action's panic", err.Error(); expected != obtained {
+		t.Errorf("an unexpected error. expected: %s; obtained: %v", expected, err)
+	}
+
+	if r, is := IsRecovered(err); !is || r.(string) != "catch me if you can" {
+		t.Errorf("expected recovered error; obtained: %v", r)
+	}
 }
 
 func TestRetry_RetriesUntilNoErrorReturned(t *testing.T) {
@@ -33,10 +62,10 @@ func TestRetry_RetriesUntilNoErrorReturned(t *testing.T) {
 			return nil
 		}
 
-		return errors.New("erroring")
+		return errors.New("error")
 	}
 
-	err := retry.Retry(nil, action, strategy.Infinite())
+	err := Retry(nil, action, strategy.Infinite())
 
 	if nil != err {
 		t.Error("expected a nil error")
@@ -45,29 +74,27 @@ func TestRetry_RetriesUntilNoErrorReturned(t *testing.T) {
 	if errorUntilAttemptNumber != attemptsMade {
 		t.Errorf(
 			"expected %d attempts to be made, but %d were made instead",
-			errorUntilAttemptNumber,
-			attemptsMade,
-		)
+			errorUntilAttemptNumber, attemptsMade)
 	}
 }
 
 func TestRetry_RetriesWithAlreadyDoneContext(t *testing.T) {
-	deadline, expected := retry.WithTimeout(0), "operation timeout"
+	deadline, expected := WithTimeout(0), "operation timeout"
 
-	if err := retry.Retry(deadline, func(uint) error { return nil }, strategy.Infinite()); !retry.IsTimeout(err) {
+	if err := Retry(deadline, func(uint) error { return nil }, strategy.Infinite()); !IsTimeout(err) {
 		t.Errorf("an unexpected error. expected: %s; obtained: %v", expected, err)
 	}
 }
 
 func TestRetry_RetriesWithDeadline(t *testing.T) {
-	deadline, expected := retry.WithTimeout(100*time.Millisecond), "operation timeout"
+	deadline, expected := WithTimeout(100*time.Millisecond), "operation timeout"
 
 	action := func(uint) error {
 		time.Sleep(110 * time.Millisecond)
 		return nil
 	}
 
-	if err := retry.Retry(deadline, action, strategy.Infinite()); !retry.IsTimeout(err) {
+	if err := Retry(deadline, action, strategy.Infinite()); !IsTimeout(err) {
 		t.Errorf("an unexpected error. expected: %s; obtained: %v", expected, err)
 	}
 }