feat: Add MaxMovedRedirections option to prevent infinite redirect loops (#92)

* implement MovedRetries

Signed-off-by: nirchetrit <[email protected]>

* - Changed module path from github.com/valkey-io/valkey-go to github.com/nirchetrit/valkey-go
- Added dependency for github.com/valkey-io/valkey-go v1.0.67
- Removed indirect dependencies for github.com/kr/pretty and github.com/kr/text

Signed-off-by: nirchetrit <[email protected]>

* update module path from github.com/nirchetrit/valkey-go to github.com/valkey-io/valkey-go

Signed-off-by: nirchetrit <[email protected]>

* added MaxMovedRetries to the readme

Signed-off-by: nirchetrit <[email protected]>

* Implement MaxMovedRetries logic in DoMulti and DoMultiCache methods, ensuring proper error handling and retry behavior. Update README to clarify the behavior of MaxMovedRetries for multi-command operations.

Signed-off-by: nirchetrit <[email protected]>

* Update module path from github.com/valkey-io/valkey-go to github.com/nirchetrit/valkey-go

Signed-off-by: nirchetrit <[email protected]>

* Refactor MaxMovedRetries handling in DoMulti and DoMultiCache methods to use a local movedRetries counter. This improves error handling for exceeded retry limits and ensures consistent behavior across retry attempts.

Signed-off-by: nirchetrit <[email protected]>

* Update module path from github.com/nirchetrit/valkey-go to github.com/valkey-io/valkey-go and remove outdated dependencies. Adjust golang.org/x/tools version to v0.28.0.

Signed-off-by: nirchetrit <[email protected]>

* Revert changes to README.md

Signed-off-by: nirchetrit <[email protected]>

* Rename MaxMovedRetries to MaxMovedRedirections and update related error handling in cluster and test files for consistency.

Signed-off-by: nirchetrit <[email protected]>

* Enhance error handling for MaxMovedRedirections in cluster.go by refining the condition checks for ValkeyError. This ensures that errors related to moved keys are correctly identified and handled during retries.

Signed-off-by: nirchetrit <[email protected]>

* Clarify comments in cluster.go regarding error handling for MOVED responses during retries. This enhances code readability and maintains consistency with previous error handling updates.

Signed-off-by: nirchetrit <[email protected]>

* Rename MaxMovedRedirections to MaxRedirects and handle ASK redirections. expose the last received error instead of a generic max moved errors exceeded

Signed-off-by: nirchetrit <[email protected]>

* Add tests for handling ASK and MOVED errors in cluster client

This commit introduces multiple test cases in cluster_test.go to validate the behavior of the cluster client when exceeding the maximum allowed ASK retries and handling mixed MOVED and ASK errors. The tests ensure that the client correctly identifies and returns ValkeyError for these scenarios, enhancing the robustness of error handling in the client.

The new tests cover:
- Exceeding max ASK retries for Do, DoCache, DoMulti, and DoMultiCache methods.
- Handling mixed MOVED and ASK errors during retries.

These additions improve the overall reliability of the cluster client in error-prone situations.

Signed-off-by: nirchetrit <[email protected]>

* revert go.mod

Signed-off-by: Rueian <[email protected]>

---------

Signed-off-by: nirchetrit <[email protected]>
Signed-off-by: Rueian <[email protected]>
Co-authored-by: Rueian <[email protected]>
Signed-off-by: Rueian <[email protected]>

Author: nirchetrit

cluster.go

@@ -537,6 +537,7 @@ func (c *clusterClient) Do(ctx context.Context, cmd Completed) (resp RedisResult
 
 func (c *clusterClient) do(ctx context.Context, cmd Completed) (resp RedisResult) {
 	attempts := 1
+	redirects := 0
 retry:
 	cc, err := c.pick(ctx, cmd.Slot(), c.toReplica(cmd))
 	if err != nil {
@@ -549,6 +550,10 @@ retry:
 process:
 	switch addr, mode := c.shouldRefreshRetry(resp.Error(), ctx); mode {
 	case RedirectMove:
+		redirects++
+		if c.opt.ClusterOption.MaxMovedRedirections > 0 && redirects > c.opt.ClusterOption.MaxMovedRedirections {
+			return resp
+		}
 		ncc := c.redirectOrNew(addr, cc, cmd.Slot(), mode)
 	recover1:
 		resp = ncc.Do(ctx, cmd)
@@ -557,6 +562,10 @@ process:
 		}
 		goto process
 	case RedirectAsk:
+		redirects++
+		if c.opt.ClusterOption.MaxMovedRedirections > 0 && redirects > c.opt.ClusterOption.MaxMovedRedirections {
+			return resp
+		}
 		ncc := c.redirectOrNew(addr, cc, cmd.Slot(), mode)
 	recover2:
 		results := ncc.DoMulti(ctx, cmds.AskingCmd, cmd)
@@ -868,6 +877,7 @@ func (c *clusterClient) DoMulti(ctx context.Context, multi ...Completed) []Redis
 	results := resultsp.Get(len(multi), len(multi))
 
 	attempts := 1
+	redirects := 0
 
 retry:
 	retries.RetryDelay = -1 // Assume no retry. Because a client retry flag can be set to false.
@@ -892,6 +902,10 @@ retry:
 
 	if len(retries.m) != 0 {
 		if retries.Redirects > 0 {
+			redirects++
+			if c.opt.ClusterOption.MaxMovedRedirections > 0 && redirects > c.opt.ClusterOption.MaxMovedRedirections {
+				return results.s
+			}
 			retries.Redirects = 0
 			goto retry
 		}
@@ -920,6 +934,7 @@ func fillErrs(n int, err error) (results []RedisResult) {
 
 func (c *clusterClient) doCache(ctx context.Context, cmd Cacheable, ttl time.Duration) (resp RedisResult) {
 	attempts := 1
+	redirects := 0
 
 retry:
 	cc, err := c.pick(ctx, cmd.Slot(), c.toReplica(Completed(cmd)))
@@ -933,6 +948,10 @@ retry:
 process:
 	switch addr, mode := c.shouldRefreshRetry(resp.Error(), ctx); mode {
 	case RedirectMove:
+		redirects++
+		if c.opt.ClusterOption.MaxMovedRedirections > 0 && redirects > c.opt.ClusterOption.MaxMovedRedirections {
+			return resp
+		}
 		ncc := c.redirectOrNew(addr, cc, cmd.Slot(), mode)
 	recover:
 		resp = ncc.DoCache(ctx, cmd, ttl)
@@ -941,6 +960,10 @@ process:
 		}
 		goto process
 	case RedirectAsk:
+		redirects++
+		if c.opt.ClusterOption.MaxMovedRedirections > 0 && redirects > c.opt.ClusterOption.MaxMovedRedirections {
+			return resp
+		}
 		results := c.askingMultiCache(c.redirectOrNew(addr, cc, cmd.Slot(), mode), ctx, []CacheableTTL{CT(cmd, ttl)})
 		resp = results.s[0]
 		resultsp.Put(results)
@@ -1235,6 +1258,7 @@ func (c *clusterClient) DoMultiCache(ctx context.Context, multi ...CacheableTTL)
 	results := resultsp.Get(len(multi), len(multi))
 
 	attempts := 1
+	redirects := 0
 
 retry:
 	retries.RetryDelay = -1 // Assume no retry. Because a client retry flag can be set to false.
@@ -1259,6 +1283,10 @@ retry:
 
 	if len(retries.m) != 0 {
 		if retries.Redirects > 0 {
+			redirects++
+			if c.opt.ClusterOption.MaxMovedRedirections > 0 && redirects > c.opt.ClusterOption.MaxMovedRedirections {
+				return results.s
+			}
 			retries.Redirects = 0
 			goto retry
 		}