Separate retry package for re-use (#641)

go/common/retry/README.md

@@ -0,0 +1,7 @@
+Util package for retrying, functions and tools for situations where we are retrying/polling for a change.
+
+Based around a 'RetryStrategy' interface that allows different approaches such as:
+- retry X times
+- retry for X seconds
+- retry for 2 minutes but backing off
+

go/common/retry/retry.go

@@ -0,0 +1,26 @@
+package retry
+
+import (
+	"fmt"
+	"time"
+)
+
+func Do(fn func() error, retryStrat Strategy) error {
+	// Reset tells the strategy we are about to start making attempts (it might reset attempts counter/record start time)
+	retryStrat.Reset()
+
+	for {
+		// attempt to execute the function
+		err := fn()
+		if err == nil {
+			// success
+			return nil
+		}
+
+		if retryStrat.Done() {
+			return fmt.Errorf("%s - latest error: %w", retryStrat.Summary(), err)
+		}
+
+		time.Sleep(retryStrat.NextRetryInterval())
+	}
+}

go/obsclient/clientutil/retry_strategy.go → go/common/retry/retry_strategy.go

@@ -1,13 +1,13 @@
-package clientutil
+package retry
 
 import (
 	"fmt"
 	"time"
 )
 
-// retryStrategy interface allows for flexible strategies for retrying/polling functions
-type retryStrategy interface {
-	// WaitInterval calls can be considered as marking the completion of an attempt
+// Strategy interface allows for flexible strategies for retrying/polling functions, the usage should be:
+type Strategy interface {
+	// NextRetryInterval calls can be considered as marking the completion of an attempt
 	NextRetryInterval() time.Duration // returns the duration to sleep before making the next attempt (may not be fixed, e.g. if strategy is to back-off)
 	Done() bool                       // returns true when caller should stop retrying
 	Summary() string                  // message to summarise usage (i.e. number of retries, time take, if it failed and why, e.g. "timed out after 120 seconds (8 attempts)"

go/common/retry/retry_test.go

@@ -0,0 +1,42 @@
+package retry
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDoWithTimeoutStrategy_SuccessAfterRetries(t *testing.T) {
+	var count int
+	testFunc := func() error {
+		count = count + 1
+		fmt.Printf("c: %d\n", count)
+		if count < 3 {
+			return fmt.Errorf("attempt number %d", count)
+		}
+		return nil
+	}
+	err := Do(testFunc, NewTimeoutStrategy(1*time.Second, 100*time.Millisecond))
+	if err != nil {
+		assert.Fail(t, "Expected function to succeed before timeout but failed", err)
+	}
+
+	assert.Equal(t, 3, count, "expected function to be called 3 times before succeeding")
+}
+
+func TestDoWithTimeoutStrategy_FailAfterTimeout(t *testing.T) {
+	var count int
+	testFunc := func() error {
+		count = count + 1
+		fmt.Printf("c: %d\n", count)
+		return fmt.Errorf("attempt number %d", count)
+	}
+	err := Do(testFunc, NewTimeoutStrategy(600*time.Millisecond, 100*time.Millisecond))
+	if err == nil {
+		assert.Fail(t, "expected failure from timeout but no err received")
+	}
+
+	assert.Greater(t, count, 5, "expected function to be called at least 5 times before timing out")
+}

go/obsclient/clientutil/clientutil.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/obscuronet/go-obscuro/go/common/retry"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/obscuronet/go-obscuro/go/obsclient"
@@ -13,11 +15,11 @@ import (
 var defaultTimeoutInterval = 1 * time.Second
 
 func AwaitTransactionReceipt(ctx context.Context, client *obsclient.AuthObsClient, txHash common.Hash, timeout time.Duration) (*types.Receipt, error) {
-	timeoutStrategy := NewTimeoutStrategy(timeout, defaultTimeoutInterval)
+	timeoutStrategy := retry.NewTimeoutStrategy(timeout, defaultTimeoutInterval)
 	return AwaitTransactionReceiptWithRetryStrategy(ctx, client, txHash, timeoutStrategy)
 }
 
-func AwaitTransactionReceiptWithRetryStrategy(ctx context.Context, client *obsclient.AuthObsClient, txHash common.Hash, retryStrategy retryStrategy) (*types.Receipt, error) {
+func AwaitTransactionReceiptWithRetryStrategy(ctx context.Context, client *obsclient.AuthObsClient, txHash common.Hash, retryStrategy retry.Strategy) (*types.Receipt, error) {
 	retryStrategy.Reset()
 	for {
 		select {

tools/contractdeployer/contract_deployer.go

@@ -8,6 +8,8 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/obscuronet/go-obscuro/go/common/retry"
+
 	"github.com/obscuronet/go-obscuro/contracts/managementcontract"
 	"github.com/obscuronet/go-obscuro/integration/erc20contract"
 	"github.com/obscuronet/go-obscuro/integration/guessinggame"
@@ -135,20 +137,22 @@ func signAndSendTxWithReceipt(wallet wallet.Wallet, deployer contractDeployerCli
 		return nil, fmt.Errorf("failed to send contract deploy transaction: %w", err)
 	}
 
-	var start time.Time
-	for start = time.Now(); time.Since(start) < timeoutWait; time.Sleep(retryInterval) {
-		receipt, err := deployer.TransactionReceipt(signedTx.Hash())
-		if err == nil && receipt != nil {
-			if receipt.Status != types.ReceiptStatusSuccessful {
-				return nil, fmt.Errorf("unable to deploy contract, receipt status unsuccessful: %v", receipt)
-			}
-			log.Info("Contract successfully deployed to %s", receipt.ContractAddress)
-			return &receipt.ContractAddress, nil
-		}
+	log.Info("Waiting (up to %s) for deploy tx to be mined into a block...", timeoutWait)
+
+	var receipt *types.Receipt
+	err = retry.Do(func() error {
+		receipt, err = deployer.TransactionReceipt(signedTx.Hash())
+		return err
+	}, retry.NewTimeoutStrategy(timeoutWait, retryInterval))
 
-		log.Info("Contract deploy tx %s has not been mined into a block after %s...", signedTx.Hash(), time.Since(start))
+	if err != nil {
+		return nil, fmt.Errorf("failed to deploy contract - %w", err)
+	}
+	if receipt.Status != types.ReceiptStatusSuccessful {
+		return nil, fmt.Errorf("unable to deploy contract, receipt status unsuccessful: %v", receipt)
 	}
-	return nil, fmt.Errorf("failed to mine contract deploy tx %s into a block after %s. Aborting", signedTx.Hash(), time.Since(start))
+
+	return &receipt.ContractAddress, nil
 }
 
 func getContractCode(cfg *Config) ([]byte, error) {