Don't return error to caller with RetryType Ignore

RetryType Ignore is documented and should work according to the documentation. Error is not returned to caller on ignore but can be seen in ObservedQuery. RetryType should be checked even if RetryPolicy.Attempt returns false, otherwise Ignore will not work on last attempt.

Patch by Rikkuru; reviewed by João Reis, Jackson Fleming for CASSGO-28

Author: Rikkuru

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Retry policy now takes into account query idempotency (CASSGO-27)
+- Don't return error to caller with RetryType Ignore (CASSGO-28)
 
 ## [1.7.0] - 2024-09-23
 

query_executor.go

@@ -165,27 +165,39 @@ func (q *queryExecutor) do(ctx context.Context, qry ExecutableQuery, hostIter Ne
 		}
 
 		// Exit if the query was successful
-		// or no retry policy defined or retry attempts were reached
-		if iter.err == nil || !qry.IsIdempotent() || rt == nil || !rt.Attempt(qry) {
+		// or query is not idempotent or no retry policy defined
+		if iter.err == nil || !qry.IsIdempotent() || rt == nil {
 			return iter
 		}
-		lastErr = iter.err
+
+		attemptsReached := !rt.Attempt(qry)
+		retryType := rt.GetRetryType(iter.err)
+
+		var stopRetries bool
 
 		// If query is unsuccessful, check the error with RetryPolicy to retry
-		switch rt.GetRetryType(iter.err) {
+		switch retryType {
 		case Retry:
 			// retry on the same host
-			continue
-		case Rethrow, Ignore:
-			return iter
 		case RetryNextHost:
 			// retry on the next host
 			selectedHost = hostIter()
-			continue
+		case Ignore:
+			iter.err = nil
+			stopRetries = true
+		case Rethrow:
+			stopRetries = true
 		default:
 			// Undefined? Return nil and error, this will panic in the requester
 			return &Iter{err: ErrUnknownRetryType}
 		}
+
+		if stopRetries || attemptsReached {
+			return iter
+		}
+
+		lastErr = iter.err
+		continue
 	}
 
 	if lastErr != nil {

session_test.go

@@ -347,3 +347,70 @@ func TestIsUseStatement(t *testing.T) {
 		}
 	}
 }
+
+type simpleTestRetryPolycy struct {
+	RetryType  RetryType
+	NumRetries int
+}
+
+func (p *simpleTestRetryPolycy) Attempt(q RetryableQuery) bool {
+	return q.Attempts() <= p.NumRetries
+}
+
+func (p *simpleTestRetryPolycy) GetRetryType(error) RetryType {
+	return p.RetryType
+}
+
+// TestRetryType_IgnoreRethrow verify that with Ignore/Rethrow retry types:
+// - retries stopped
+// - return error is nil on Ignore
+// - return error is not nil on Rethrow
+// - observed error is not nil
+func TestRetryType_IgnoreRethrow(t *testing.T) {
+	session := createSession(t)
+	defer session.Close()
+
+	var observedErr error
+	var observedAttempts int
+
+	resetObserved := func() {
+		observedErr = nil
+		observedAttempts = 0
+	}
+
+	observer := funcQueryObserver(func(ctx context.Context, o ObservedQuery) {
+		observedErr = o.Err
+		observedAttempts++
+	})
+
+	for _, caseParams := range []struct {
+		retries   int
+		retryType RetryType
+	}{
+		{0, Ignore},  // check that error ignored even on last attempt
+		{1, Ignore},  // check thet ignore stops retries
+		{1, Rethrow}, // check thet rethrow stops retries
+	} {
+		retryPolicy := &simpleTestRetryPolycy{RetryType: caseParams.retryType, NumRetries: caseParams.retries}
+
+		err := session.Query("INSERT INTO gocql_test.invalid_table(value) VALUES(1)").Idempotent(true).RetryPolicy(retryPolicy).Observer(observer).Exec()
+
+		if err != nil && caseParams.retryType == Ignore {
+			t.Fatalf("[%v] Expected no error, got: %s", caseParams.retryType, err)
+		}
+
+		if err == nil && caseParams.retryType == Rethrow {
+			t.Fatalf("[%v] Expected unconfigured table error, got: nil", caseParams.retryType)
+		}
+
+		if observedErr == nil {
+			t.Fatal("Expected unconfigured table error in Obserer, got: nil")
+		}
+
+		if observedAttempts > 1 {
+			t.Fatalf("Expected one attempt, got: %d", observedAttempts)
+		}
+
+		resetObserved()
+	}
+}