Repository: redis/go-redis

Issue:
Different responses for cmd.Result() in pipelines starting in v9.14.0
## Expected Behavior

When running SET in a pipeline, fetching the response with `value, err := cmd.Result()` used to return a <nil> err in <=v9.13.0.

## Current Behavior
When running SET in a pipeline, fetching the response with `value, err := cmd.Result()` returns a redis.Nil error.

## Possible Solution

<!--- Not obligatory, but suggest a fix/reason for the bug, -->

## Steps to Reproduce

```
// This unit test would passes in v9.13.0 but fails in v9.14.0.
package redis

import (
	"testing"
	"time"

	"github.com/alicebob/miniredis/v2"
	"github.com/redis/go-redis/v9"
	"github.com/stretchr/testify/suite"
	"golang.org/x/net/context"
)

type redisDependencyUpdateSuite struct {
	suite.Suite

	redisServer *miniredis.Miniredis
	redisClient *redis.Client

	ctx context.Context
}

func TestRedisDependencyUpdateSuite(t *testing.T) {
	suite.Run(t, new(redisDependencyUpdateSuite))
}

func (s *redisDependencyUpdateSuite) SetupSuite() {
	s.redisServer = miniredis.RunT(s.T())
}

func (s *redisDependencyUpdateSuite) SetupTest() {
	s.redisClient = redis.NewClient(defaultClientOptions(s.redisServer.Addr()))
	s.ctx = context.Background()
}

func (s *redisDependencyUpdateSuite) TearDownTest() {
	s.redisClient.Close()
}

func (s *redisDependencyUpdateSuite) TearDownSuite() {
	s.redisServer.Close()
}

func (s *redisDependencyUpdateSuite) TestPipeline() {
	pipeline := s.redisClient.Pipeline()
	pipeline.Do(s.ctx, "GET", "unittest")
	pipeline.Do(s.ctx, "SET", "unittest", "tested")
	pipeline.Do(s.ctx, "GET", "unittest")

	cmds, err := pipeline.Exec(s.ctx)

	s.Require().Len(cmds, 3)

	// First GET should not be found.
	cmd, ok := cmds[0].(*redis.Cmd)
	s.Require().True(ok)
	val, err := cmd.Result()
	s.Require().Equal(redis.Nil, err)
	s.Require().Nil(val)

	cmd, ok = cmds[1].(*redis.Cmd)
	s.Require().True(ok)

	// v9.14.0 now returns [redis.Nil] for the err
	// v9.13.0 and earlier returned <nil> for the err.
	val, err = cmd.Result()
	s.Require().NoError(err)
	s.Require().Equal("OK", val)

	// Second GET should return the value we just set.
	cmd, ok = cmds[2].(*redis.Cmd)
	s.Require().True(ok)
	val, err = cmd.Result()
	s.Require().NoError(err)
	s.Require().Equal("tested", val)
}

func defaultClientOptions(addr string) *redis.Options {
	return &redis.Options{
		Addr: addr,
		DB:   0,

		DialTimeout:           10 * time.Second,
		ReadTimeout:           30 * time.Second,
		WriteTimeout:          30 * time.Second,
		ContextTimeoutEnabled: true,

		MaxRetries: -1,

		PoolSize:        10,
		PoolTimeout:     30 * time.Second,
		ConnMaxIdleTime: time.Minute,
	}
}
```

Discussion:
@zhughes3: Was the intended behavior always to return the `redis: nil` err in this case?@cxljs: Thank you for your feedback, the expected behavior of set command is to return nil, which has been fixed in PR #3525.@ndyakov: Hello @zhughes3 , looks related to https://github.com/redis/go-redis/pull/3509 which should fix https://github.com/redis/go-redis/issues/2871, Thank you for reporting@zhughes3: Thanks for the contributions!

Solution patch:
diff --git a/redis.go b/redis.go
index e741569..08c71cd 100644
--- a/redis.go
+++ b/redis.go
@@ -768,7 +768,10 @@ func (c *baseClient) generalProcessPipeline(
 			return err
 		})
 		if lastErr == nil || !canRetry || !shouldRetry(lastErr, true) {
-			setCmdsErr(cmds, lastErr)
+			// The error should be set here only when failing to obtain the conn.
+			if !isRedisError(lastErr) {
+				setCmdsErr(cmds, lastErr)
+			}
 			return lastErr
 		}
 	}
@@ -864,7 +867,7 @@ func (c *baseClient) txPipelineReadQueued(ctx context.Context, cn *pool.Conn, rd
 	}
 
 	// Parse +QUEUED.
-  for _, cmd := range cmds {
+	for _, cmd := range cmds {
 		// To be sure there are no buffered push notifications, we process them before reading the reply
 		if err := c.processPendingPushNotificationWithReader(ctx, cn, rd); err != nil {
 			internal.Logger.Printf(ctx, "push: error processing pending notifications before reading reply: %v", err)



Test patch:
diff --git a/pipeline_test.go b/pipeline_test.go
index 15eacb3..a95df3c 100644
--- a/pipeline_test.go
+++ b/pipeline_test.go
@@ -60,6 +60,39 @@ var _ = Describe("pipelining", func() {
 		Expect(cmds).To(BeEmpty())
 	})
 
+	It("pipeline: basic exec", func() {
+		p := client.Pipeline()
+		p.Get(ctx, "key")
+		p.Set(ctx, "key", "value", 0)
+		p.Get(ctx, "key")
+		cmds, err := p.Exec(ctx)
+		Expect(err).To(Equal(redis.Nil))
+		Expect(cmds).To(HaveLen(3))
+		Expect(cmds[0].Err()).To(Equal(redis.Nil))
+		Expect(cmds[1].(*redis.StatusCmd).Val()).To(Equal("OK"))
+		Expect(cmds[1].Err()).NotTo(HaveOccurred())
+		Expect(cmds[2].(*redis.StringCmd).Val()).To(Equal("value"))
+		Expect(cmds[2].Err()).NotTo(HaveOccurred())
+	})
+
+	It("pipeline: exec pipeline when get conn failed", func() {
+		p := client.Pipeline()
+		p.Get(ctx, "key")
+		p.Set(ctx, "key", "value", 0)
+		p.Get(ctx, "key")
+
+		client.Close()
+
+		cmds, err := p.Exec(ctx)
+		Expect(err).To(Equal(redis.ErrClosed))
+		Expect(cmds).To(HaveLen(3))
+		for _, cmd := range cmds {
+			Expect(cmd.Err()).To(Equal(redis.ErrClosed))
+		}
+
+		client = redis.NewClient(redisOptions())
+	})
+
 	assertPipeline := func() {
 		It("returns no errors when there are no commands", func() {
 			_, err := pipe.Exec(ctx)