Changed behaviour when wrapped net.Error is returned in a ClusterClient Watch function

[bjorngylling]

I am not sure if this really is a bug or not. Either way it was a change in behaviour that took me by surprise so I figured I would post it to see what you think.

Expected Behavior

In go-redis versions before v9.17.0 when returning a wrapped net.Error inside a redis.ClusterClient Watch function it was simply propagated out to the surrounding code.

Current Behavior

Since v9.17.0 when a redis.ClusterClient Watch function returns a wrapped net.Error the Watch code will retry and run the function again.

This behaviour does not appear if using a regular redis.Client, if the example is changed to use a redis.Client it will pass both on v9.16.0 and v9.17.0.

Possible Solution

Perhaps it would be possible to only trigger retries on net.Errors caused by Redis operations?

Steps to Reproduce

The following test illustrates the difference when running with v9.16.0 and v9.17.0.

package main

import (
	"context"
	"net"
	"testing"

	"github.com/alicebob/miniredis/v2"
	"github.com/redis/go-redis/v9"
)

func TestNetErrorInClusterClientWatch(t *testing.T) {
	r := miniredis.RunT(t)

	cl := redis.NewClusterClient(&redis.ClusterOptions{
		Addrs: []string{r.Addr()},
	})

	count := 0
	txFn := func(tx *redis.Tx) error {
		count++
		return fmt.Errorf("%w", net.ErrClosed)
	}

	_ = cl.Watch(context.Background(), txFn, "somekey")

	if count != 1 {
		t.Errorf("txFn called %d times", count)
	}
}

Context (Environment)

When doing network calls unrelated to Redis inside a Watch function they can result in a net.Error. To avoid excessive retries I would have to avoid wrapping the error which makes error-handling harder.

[ndyakov]

@bjorngylling thank you for reporting, i will investigate. Maybe it is related to #3602 ?

[bjorngylling]

Maybe it is related to #3602 ?

That seems highly likely. I haven't identified any specific commit though.