diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c0054952..a83e6f861 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/query_executor.go b/query_executor.go index 03687361a..d6be02e53 100644 --- a/query_executor.go +++ b/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 { diff --git a/session_test.go b/session_test.go index 0319a8a4c..c7bafbb04 100644 --- a/session_test.go +++ b/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() + } +}