feat: change operations retry logic to be opt-in

BREAKING CHANGE: The retry logic for operations is now opt-in. To
enable the retry, the caller must now provide the `WithRetry` option
to the `ExecuteOperation`. This change is intended to provide greater
control over the retry behavior and to avoid unintended retries in
certain scenarios.

Author: jkongie

.changeset/tangy-plums-draw.md

@@ -0,0 +1,5 @@
+---
+"chainlink-deployments-framework": patch
+---
+
+BREAKING: Operations retry logic is now opt in. Use the `WithRetry` method in your `ExecuteOperation` call to enable retries

operations/execute.go

@@ -18,16 +18,59 @@ type ExecuteConfig[IN, DEP any] struct {
 type ExecuteOption[IN, DEP any] func(*ExecuteConfig[IN, DEP])
 
 type RetryConfig[IN, DEP any] struct {
-	// DisableRetry disables the retry mechanism if set to true.
-	DisableRetry bool
+	// Enabled dermines if the retry is enabled for the operation.
+	Enabled bool
+
+	// Policy is the retry policy to control the behavior of the retry.
+	Policy RetryPolicy
+
 	// InputHook is a function that returns an updated input before retrying the operation.
 	// The operation when retried will use the input returned by this function.
 	// This is useful for scenarios like updating the gas limit.
-	// This will be ignored if DisableRetry is set to true.
 	InputHook func(input IN, deps DEP) IN
 }
 
-// WithRetryConfig is an ExecuteOption that sets the retry configuration.
+// newDisabledRetryConfig returns a default retry configuration that is initially disabled.
+func newDisabledRetryConfig[IN, DEP any]() RetryConfig[IN, DEP] {
+	return RetryConfig[IN, DEP]{
+		Enabled: false,
+		Policy: RetryPolicy{
+			MaxAttempts: 10,
+		},
+	}
+}
+
+// RetryPolicy defines the arguments to control the retry behavior.
+type RetryPolicy struct {
+	MaxAttempts uint
+}
+
+// options returns the 'avast/retry' functional options for the retry policy.
+func (p RetryPolicy) options() []retry.Option {
+	return []retry.Option{
+		retry.Attempts(p.MaxAttempts),
+	}
+}
+
+// WithRetry is an ExecuteOption that enables the default retry for the operation.
+func WithRetry[IN, DEP any]() ExecuteOption[IN, DEP] {
+	return func(c *ExecuteConfig[IN, DEP]) {
+		c.retryConfig.Enabled = true
+	}
+}
+
+// WithRetryInput is an ExecuteOption that enables the default retry and provide an input
+// transform function which will modify the input on each retry attempt.
+func WithRetryInput[IN, DEP any](inputHookFunc func(IN, DEP) IN) ExecuteOption[IN, DEP] {
+	return func(c *ExecuteConfig[IN, DEP]) {
+		c.retryConfig.Enabled = true
+		c.retryConfig.InputHook = inputHookFunc
+	}
+}
+
+// WithRetryConfig is an ExecuteOption that sets the retry configuration. This provides a way to
+// customize the retry behavior specific to the needs of the operation. Use this for the most
+// flexibility and control over the retry behavior.
 func WithRetryConfig[IN, DEP any](config RetryConfig[IN, DEP]) ExecuteOption[IN, DEP] {
 	return func(c *ExecuteConfig[IN, DEP]) {
 		c.retryConfig = config
@@ -69,39 +112,53 @@ func ExecuteOperation[IN, OUT, DEP any](
 		return previousReport, nil
 	}
 
-	executeConfig := &ExecuteConfig[IN, DEP]{retryConfig: RetryConfig[IN, DEP]{}}
+	executeConfig := &ExecuteConfig[IN, DEP]{
+		retryConfig: newDisabledRetryConfig[IN, DEP](),
+	}
 	for _, opt := range opts {
 		opt(executeConfig)
 	}
 
 	var output OUT
 	var err error
 
-	if executeConfig.retryConfig.DisableRetry {
-		output, err = operation.execute(b, deps, input)
-	} else {
+	if executeConfig.retryConfig.Enabled {
 		var inputTemp = input
-		output, err = retry.DoWithData(func() (OUT, error) {
-			return operation.execute(b, deps, inputTemp)
-		}, retry.OnRetry(func(attempt uint, err error) {
+
+		// Generate the configurable options for the retry
+		retryOpts := executeConfig.retryConfig.Policy.options()
+		// Use the operation context in the retry
+		retryOpts = append(retryOpts, retry.Context(b.GetContext()))
+		// Append the retry logic which will log the retry and attempt to transform the input
+		// if the user provided a custom input hook.
+		retryOpts = append(retryOpts, retry.OnRetry(func(attempt uint, err error) {
 			b.Logger.Infow("Operation failed. Retrying...",
 				"operation", operation.def.ID, "attempt", attempt, "error", err)
 
 			if executeConfig.retryConfig.InputHook != nil {
 				inputTemp = executeConfig.retryConfig.InputHook(inputTemp, deps)
 			}
 		}))
+
+		output, err = retry.DoWithData(
+			func() (OUT, error) {
+				return operation.execute(b, deps, inputTemp)
+			},
+			retryOpts...,
+		)
+	} else {
+		output, err = operation.execute(b, deps, input)
 	}
 
 	if err == nil && !IsSerializable(b.Logger, output) {
 		return Report[IN, OUT]{}, fmt.Errorf("operation %s output: %w", operation.def.ID, ErrNotSerializable)
 	}
 
 	report := NewReport(operation.def, input, output, err)
-	err = b.reporter.AddReport(genericReport(report))
-	if err != nil {
+	if err = b.reporter.AddReport(genericReport(report)); err != nil {
 		return Report[IN, OUT]{}, err
 	}
+
 	if report.Err != nil {
 		return report, report.Err
 	}
@@ -175,14 +232,15 @@ func ExecuteSequence[IN, OUT, DEP any](
 		childReports...,
 	)
 
-	err = b.reporter.AddReport(genericReport(report))
-	if err != nil {
+	if err = b.reporter.AddReport(genericReport(report)); err != nil {
 		return SequenceReport[IN, OUT]{}, err
 	}
+
 	executionReports, err := b.reporter.GetExecutionReports(report.ID)
 	if err != nil {
 		return SequenceReport[IN, OUT]{}, err
 	}
+
 	if report.Err != nil {
 		return SequenceReport[IN, OUT]{report, executionReports}, report.Err
 	}
@@ -229,6 +287,7 @@ func loadPreviousSuccessfulReport[IN, OUT any](
 			return typedReport, true
 		}
 	}
+
 	// No previous execution was found
 	return Report[IN, OUT]{}, false
 }

operations/execute_test.go

@@ -24,22 +24,52 @@ func Test_ExecuteOperation(t *testing.T) {
 		wantErr           string
 	}{
 		{
-			name:              "DefaultRetry",
+			name:              "no retry",
+			wantOpCalledTimes: 1,
+			wantErr:           "test error",
+		},
+		{
+			name: "with default retry",
+			options: []ExecuteOption[int, any]{
+				WithRetry[int, any](),
+			},
+			wantOpCalledTimes: 3,
+			wantOutput:        2,
+		},
+		{
+			name: "with custom retry eventual success",
+			options: []ExecuteOption[int, any]{
+				WithRetryConfig(RetryConfig[int, any]{
+					Enabled: true,
+					Policy: RetryPolicy{
+						MaxAttempts: 10,
+					},
+				}),
+			},
 			wantOpCalledTimes: 3,
 			wantOutput:        2,
 		},
 		{
-			name:              "NoRetry",
-			options:           []ExecuteOption[int, any]{WithRetryConfig[int, any](RetryConfig[int, any]{DisableRetry: true})},
+			name: "with custom retry eventual failure",
+			options: []ExecuteOption[int, any]{
+				WithRetryConfig(RetryConfig[int, any]{
+					Enabled: true,
+					Policy: RetryPolicy{
+						MaxAttempts: 1,
+					},
+				}),
+			},
 			wantOpCalledTimes: 1,
 			wantErr:           "test error",
 		},
 		{
 			name: "NewInputHook",
-			options: []ExecuteOption[int, any]{WithRetryConfig[int, any](RetryConfig[int, any]{InputHook: func(input int, deps any) int {
-				// update input to 5 after first failed attempt
-				return 5
-			}})},
+			options: []ExecuteOption[int, any]{
+				WithRetryInput(func(input int, deps any) int {
+					// update input to 5 after first failed attempt
+					return 5
+				}),
+			},
 			wantOpCalledTimes: 3,
 			wantOutput:        6,
 		},

operations/report.go

@@ -44,7 +44,7 @@ type SequenceReport[IN, OUT any] struct {
 // This is useful when we want to return the report as a generic type in the changeset.output.
 func (r SequenceReport[IN, OUT]) ToGenericSequenceReport() SequenceReport[any, any] {
 	return SequenceReport[any, any]{
-		Report:           genericReport[IN, OUT](r.Report),
+		Report:           genericReport(r.Report),
 		ExecutionReports: r.ExecutionReports,
 	}
 }

operations/report_test.go

@@ -69,7 +69,7 @@ func Test_NewReport(t *testing.T) {
 
 	testErr := errors.New("test error")
 	childOperationID := uuid.New().String()
-	report := NewReport[int, int](op.def, 1, 2, testErr, childOperationID)
+	report := NewReport(op.def, 1, 2, testErr, childOperationID)
 	assert.NotEmpty(t, report.ID)
 	assert.Equal(t, op.def, report.Def)
 	assert.Equal(t, 1, report.Input)