Repository: redis/go-redis

Issue:
Avoid using panic in Redis Commands 
In various functions within the go-redis library, there is a use of panic to handle cases. 
Using panic for this purpose is not in line with Go's idiomatic error handling and can result in unexpected program termination.

Suggested Improvement:
Replace occurrences of panic with a more idiomatic error return approach. This allows users to handle argument validation errors gracefully through error checks and aligns with Go's standard error handling practices.

Hints from issue comments:
@SoulPancake: Can you illustrate an example of how we can solve this?
I would like to help with this @ofekshenawa @eidoid: > Can you illustrate an example of how we can solve this? I would like to help with this @ofekshenawa

By using a middleware to capture panics and return as error perhaps.@github-actions: This issue is marked stale. It will be closed in 30 days if it is not updated.@ndyakov: I think we can revisit the panics in `go-redis` and see if all of them make sense. @SoulPancake: @ndyakov We can cascade all errors to callers except the ones related directly with boot, where maybe a panic is the right thing to do
something like this?

![Image](https://github.com/user-attachments/assets/1da6ed96-9d63-40e5-9124-d2923d905cc5)
@elena-kolevska: I agree that we should evaluate every case separately and return the error to caller. @ndyakov: Adding this to our backlog of issues to look at.@dragneelfps: @ndyakov I tried hacking up a [PR](https://github.com/redis/go-redis/pull/3568/files) for this. Could you or anyone else review it? Thanks! 

Solution patch:
diff --git a/bitmap_commands.go b/bitmap_commands.go
index 4dbc862..86aa9b7 100644
--- a/bitmap_commands.go
+++ b/bitmap_commands.go
@@ -141,7 +141,9 @@ func (c cmdable) BitPos(ctx context.Context, key string, bit int64, pos ...int64
 		args[3] = pos[0]
 		args[4] = pos[1]
 	default:
-		panic("too many arguments")
+		cmd := NewIntCmd(ctx)
+		cmd.SetErr(errors.New("too many arguments"))
+		return cmd
 	}
 	cmd := NewIntCmd(ctx, args...)
 	_ = c(ctx, cmd)
@@ -182,7 +184,9 @@ func (c cmdable) BitFieldRO(ctx context.Context, key string, values ...interface
 	args[0] = "BITFIELD_RO"
 	args[1] = key
 	if len(values)%2 != 0 {
-		panic("BitFieldRO: invalid number of arguments, must be even")
+		c := NewIntSliceCmd(ctx)
+		c.SetErr(errors.New("BitFieldRO: invalid number of arguments, must be even"))
+		return c
 	}
 	for i := 0; i < len(values); i += 2 {
 		args = append(args, "GET", values[i], values[i+1])
diff --git a/commands.go b/commands.go
index 04235a2..f313728 100644
--- a/commands.go
+++ b/commands.go
@@ -693,7 +693,9 @@ func (c cmdable) MemoryUsage(ctx context.Context, key string, samples ...int) *I
 	args := []interface{}{"memory", "usage", key}
 	if len(samples) > 0 {
 		if len(samples) != 1 {
-			panic("MemoryUsage expects single sample count")
+			cmd := NewIntCmd(ctx)
+			cmd.SetErr(errors.New("MemoryUsage expects single sample count"))
+			return cmd
 		}
 		args = append(args, "SAMPLES", samples[0])
 	}
diff --git a/sortedset_commands.go b/sortedset_commands.go
index e48e736..7827bab 100644
--- a/sortedset_commands.go
+++ b/sortedset_commands.go
@@ -2,6 +2,7 @@ package redis
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"time"
 
@@ -313,7 +314,9 @@ func (c cmdable) ZPopMax(ctx context.Context, key string, count ...int64) *ZSlic
 	case 1:
 		args = append(args, count[0])
 	default:
-		panic("too many arguments")
+		cmd := NewZSliceCmd(ctx)
+		cmd.SetErr(errors.New("too many arguments"))
+		return cmd
 	}
 
 	cmd := NewZSliceCmd(ctx, args...)
@@ -333,7 +336,9 @@ func (c cmdable) ZPopMin(ctx context.Context, key string, count ...int64) *ZSlic
 	case 1:
 		args = append(args, count[0])
 	default:
-		panic("too many arguments")
+		cmd := NewZSliceCmd(ctx)
+		cmd.SetErr(errors.New("too many arguments"))
+		return cmd
 	}
 
 	cmd := NewZSliceCmd(ctx, args...)



Test patch:
diff --git a/commands_test.go b/commands_test.go
index 7e0cdc3..17b4dd0 100644
--- a/commands_test.go
+++ b/commands_test.go
@@ -735,6 +735,9 @@ var _ = Describe("Commands", func() {
 			n, err = client.MemoryUsage(ctx, "foo", 0).Result()
 			Expect(err).NotTo(HaveOccurred())
 			Expect(n).NotTo(BeZero())
+
+			_, err = client.MemoryUsage(ctx, "foo", 0, 1).Result()
+			Expect(err).Should(MatchError("MemoryUsage expects single sample count"))
 		})
 	})
 
@@ -1598,6 +1601,9 @@ var _ = Describe("Commands", func() {
 			pos, err = client.BitPos(ctx, "mykey", 0, 0, 0).Result()
 			Expect(err).NotTo(HaveOccurred())
 			Expect(pos).To(Equal(int64(-1)))
+
+			_, err = client.BitPos(ctx, "mykey", 0, 0, 0, 0, 0).Result()
+			Expect(err).Should(MatchError("too many arguments"))
 		})
 
 		It("should BitPosSpan", func() {
@@ -1635,6 +1641,9 @@ var _ = Describe("Commands", func() {
 			nn, err = client.BitFieldRO(ctx, "mykey", "u8", 0, "u4", 8, "u4", 12, "u4", 13).Result()
 			Expect(err).NotTo(HaveOccurred())
 			Expect(nn).To(Equal([]int64{0, 15, 15, 14}))
+
+			_, err = client.BitFieldRO(ctx, "mykey", "u8", 0, "u4", 8, "u4", 12, "u4").Result()
+			Expect(err).Should(MatchError("BitFieldRO: invalid number of arguments, must be even"))
 		})
 
 		It("should Decr", func() {
@@ -5254,6 +5263,9 @@ var _ = Describe("Commands", func() {
 				Score:  1,
 				Member: "one",
 			}}))
+
+			_, err = client.ZPopMax(ctx, "zset", 10, 11).Result()
+			Expect(err).Should(MatchError("too many arguments"))
 		})
 
 		It("should ZPopMin", func() {
@@ -5321,6 +5333,9 @@ var _ = Describe("Commands", func() {
 				Score:  3,
 				Member: "three",
 			}}))
+
+			_, err = client.ZPopMin(ctx, "zset", 10, 11).Result()
+			Expect(err).Should(MatchError("too many arguments"))
 		})
 
 		It("should ZRange", func() {