SQLStore: Fix SQLite error propagation if query retries are disabled (#64904)

* SQLStore: Add test when query retrying is disabled

* Fix condition

* Add test cases for sqlite3.ErrLocked

pkg/services/sqlstore/session.go

@@ -95,7 +95,7 @@ func (ss *SQLStore) retryOnLocks(ctx context.Context, callback DBTransactionFunc
 			ctxLogger.Info("Database locked, sleeping then retrying", "error", err, "retry", retry, "code", sqlError.Code)
 			// retryer immediately returns the error (if there is one) without checking the response
 			// therefore we only have to send it if we have reached the maximum retries
-			if retry == ss.dbCfg.QueryRetries {
+			if retry >= ss.dbCfg.QueryRetries {
 				return retryer.FuncError, ErrMaximumRetriesReached.Errorf("retry %d: %w", retry, err)
 			}
 			return retryer.FuncFailure, nil

pkg/services/sqlstore/session_test.go

@@ -7,12 +7,13 @@ import (
 	"testing"
 
 	"github.com/mattn/go-sqlite3"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func TestRetryingOnFailures(t *testing.T) {
+func TestRetryingDisabled(t *testing.T) {
 	store := InitTestDB(t)
-	store.dbCfg.QueryRetries = 5
+	require.Equal(t, 0, store.dbCfg.QueryRetries)
 
 	funcToTest := map[string]func(ctx context.Context, callback DBTransactionFunc) error{
 		"WithDbSession":    store.WithDbSession,
@@ -20,7 +21,7 @@ func TestRetryingOnFailures(t *testing.T) {
 	}
 
 	for name, f := range funcToTest {
-		t.Run(fmt.Sprintf("%s should return the error immediately if it's other than sqlite3.ErrLocked or sqlite3.ErrBusy", name), func(t *testing.T) {
+		t.Run(fmt.Sprintf("%s should return the error immediately", name), func(t *testing.T) {
 			i := 0
 			callback := func(sess *DBSession) error {
 				i++
@@ -31,19 +32,74 @@ func TestRetryingOnFailures(t *testing.T) {
 			require.Equal(t, 1, i)
 		})
 
-		t.Run(fmt.Sprintf("%s should return the sqlite3.Error if all retries have failed", name), func(t *testing.T) {
+		errCodes := []sqlite3.ErrNo{sqlite3.ErrBusy, sqlite3.ErrLocked}
+		for _, c := range errCodes {
+			t.Run(fmt.Sprintf("%s should return the sqlite3.Error %v immediately", name, c.Error()), func(t *testing.T) {
+				i := 0
+				callback := func(sess *DBSession) error {
+					i++
+					return sqlite3.Error{Code: c}
+				}
+				err := f(context.Background(), callback)
+				require.Error(t, err)
+				var driverErr sqlite3.Error
+				require.ErrorAs(t, err, &driverErr)
+				require.Equal(t, 1, i)
+				assert.Equal(t, c, driverErr.Code)
+			})
+		}
+
+		t.Run(fmt.Sprintf("%s should not return error if the callback succeeds", name), func(t *testing.T) {
+			i := 0
+			callback := func(sess *DBSession) error {
+				i++
+				return nil
+			}
+			err := f(context.Background(), callback)
+			require.NoError(t, err)
+			require.Equal(t, 1, i)
+		})
+	}
+}
+
+func TestRetryingOnFailures(t *testing.T) {
+	store := InitTestDB(t)
+	store.dbCfg.QueryRetries = 5
+
+	funcToTest := map[string]func(ctx context.Context, callback DBTransactionFunc) error{
+		"WithDbSession":    store.WithDbSession,
+		"WithNewDbSession": store.WithNewDbSession,
+	}
+
+	for name, f := range funcToTest {
+		t.Run(fmt.Sprintf("%s should return the error immediately if it's other than sqlite3.ErrLocked or sqlite3.ErrBusy", name), func(t *testing.T) {
 			i := 0
 			callback := func(sess *DBSession) error {
 				i++
-				return sqlite3.Error{Code: sqlite3.ErrBusy}
+				return errors.New("some error")
 			}
 			err := f(context.Background(), callback)
 			require.Error(t, err)
-			var driverErr sqlite3.Error
-			require.ErrorAs(t, err, &driverErr)
-			require.Equal(t, store.dbCfg.QueryRetries, i)
+			require.Equal(t, 1, i)
 		})
 
+		errCodes := []sqlite3.ErrNo{sqlite3.ErrBusy, sqlite3.ErrLocked}
+		for _, c := range errCodes {
+			t.Run(fmt.Sprintf("%s should return the sqlite3.Error %v if all retries have failed", name, c.Error()), func(t *testing.T) {
+				i := 0
+				callback := func(sess *DBSession) error {
+					i++
+					return sqlite3.Error{Code: c}
+				}
+				err := f(context.Background(), callback)
+				require.Error(t, err)
+				var driverErr sqlite3.Error
+				require.ErrorAs(t, err, &driverErr)
+				require.Equal(t, store.dbCfg.QueryRetries, i)
+				assert.Equal(t, c, driverErr.Code)
+			})
+		}
+
 		t.Run(fmt.Sprintf("%s should not return the error if successive retries succeed", name), func(t *testing.T) {
 			i := 0
 			callback := func(sess *DBSession) error {