From 7be00c8725a02d7984e4f2d4271e37af3fe4f6bc Mon Sep 17 00:00:00 2001 From: Sourabh Date: Tue, 28 Oct 2025 15:02:34 +0530 Subject: [PATCH 1/3] fix(panic): Return error instead of panic from commands (#3568) Instead of panic in few commands, we can return an error to avoid unexpected panics in application code. --- bitmap_commands.go | 8 ++++++-- commands.go | 4 +++- commands_test.go | 15 +++++++++++++++ sortedset_commands.go | 9 +++++++-- 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/bitmap_commands.go b/bitmap_commands.go index 4dbc862a..86aa9b7e 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 04235a2e..f313728d 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/commands_test.go b/commands_test.go index 7e0cdc37..17b4dd03 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() { diff --git a/sortedset_commands.go b/sortedset_commands.go index e48e7367..7827babc 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...) From 9c77386b08ec1fb87cce49f253fd2b0218f515a2 Mon Sep 17 00:00:00 2001 From: iliya <68940374+12ya@users.noreply.github.com> Date: Tue, 28 Oct 2025 18:41:45 +0900 Subject: [PATCH 2/3] =?UTF-8?q?chore(tests):=20internal/proto/peek=5Fpush?= =?UTF-8?q?=5Fnotification=5Ftest=20:=20Refactor=20test=20helpers=20to?= =?UTF-8?q?=E2=80=A6=20(#3563)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * internal/proto/peek_push_notification_test : Refactor test helpers to use fmt.Fprintf for buffers Replaced buf.WriteString(fmt.Sprintf(...)) with fmt.Fprintf or fmt.Fprint in test helper functions for improved clarity and efficiency. This change affects push notification and RESP3 test utilities. * peek_push_notification_test: revert prev formatting * all: replace buf.WriteString with fmt.FprintF for consistency --------- Co-authored-by: Nedyalko Dyakov <1547186+ndyakov@users.noreply.github.com> --- internal/proto/peek_push_notification_test.go | 39 ++++++++++--------- internal/proto/reader_test.go | 3 +- push/push_test.go | 34 ++++++++-------- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/internal/proto/peek_push_notification_test.go b/internal/proto/peek_push_notification_test.go index 58a794b8..7d439e59 100644 --- a/internal/proto/peek_push_notification_test.go +++ b/internal/proto/peek_push_notification_test.go @@ -95,7 +95,7 @@ func TestPeekPushNotificationName(t *testing.T) { t.Run("NotPushNotification", func(t *testing.T) { // Test with regular array instead of push notification buf := &bytes.Buffer{} - buf.WriteString("*2\r\n$6\r\nMOVING\r\n$4\r\ndata\r\n") + fmt.Fprint(buf, "*2\r\n$6\r\nMOVING\r\n$4\r\ndata\r\n") reader := NewReader(buf) _, err := reader.PeekPushNotificationName() @@ -112,7 +112,7 @@ func TestPeekPushNotificationName(t *testing.T) { t.Run("InsufficientData", func(t *testing.T) { // Test with buffer smaller than peek size - this might panic due to bounds checking buf := &bytes.Buffer{} - buf.WriteString(">") + fmt.Fprint(buf, ">") reader := NewReader(buf) func() { @@ -146,7 +146,7 @@ func TestPeekPushNotificationName(t *testing.T) { t.Run(fmt.Sprintf("Type_%c", respType), func(t *testing.T) { buf := &bytes.Buffer{} buf.WriteByte(respType) - buf.WriteString("test data that fills the buffer completely") + fmt.Fprint(buf, "test data that fills the buffer completely") reader := NewReader(buf) _, err := reader.PeekPushNotificationName() @@ -167,7 +167,7 @@ func TestPeekPushNotificationName(t *testing.T) { t.Run("ZeroLengthArray", func(t *testing.T) { // Create push notification with zero elements: >0\r\n buf := &bytes.Buffer{} - buf.WriteString(">0\r\npadding_data_to_fill_buffer_completely") + fmt.Fprint(buf, ">0\r\npadding_data_to_fill_buffer_completely") reader := NewReader(buf) _, err := reader.PeekPushNotificationName() @@ -209,7 +209,7 @@ func TestPeekPushNotificationName(t *testing.T) { for _, tc := range corruptedCases { t.Run(tc.name, func(t *testing.T) { buf := &bytes.Buffer{} - buf.WriteString(tc.data) + fmt.Fprint(buf, tc.data) reader := NewReader(buf) // Some corrupted data might not error but return unexpected results @@ -230,7 +230,7 @@ func TestPeekPushNotificationName(t *testing.T) { // Create buffer that is exactly 36 bytes (the peek window size) buf := &bytes.Buffer{} // ">1\r\n$4\r\nTEST\r\n" = 14 bytes, need 22 more - buf.WriteString(">1\r\n$4\r\nTEST\r\n1234567890123456789012") + fmt.Fprint(buf, ">1\r\n$4\r\nTEST\r\n1234567890123456789012") if buf.Len() != 36 { t.Errorf("Expected buffer length 36, got %d", buf.Len()) } @@ -295,25 +295,26 @@ func createValidPushNotification(notificationName, data string) *bytes.Buffer { buf := &bytes.Buffer{} simpleOrString := rand.Intn(2) == 0 + defMsg := fmt.Sprintf("$%d\r\n%s\r\n", len(notificationName), notificationName) if data == "" { // Single element notification - buf.WriteString(">1\r\n") + fmt.Fprint(buf, ">1\r\n") if simpleOrString { - buf.WriteString(fmt.Sprintf("+%s\r\n", notificationName)) + fmt.Fprintf(buf, "+%s\r\n", notificationName) } else { - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(notificationName), notificationName)) + fmt.Fprint(buf, defMsg) } } else { // Two element notification - buf.WriteString(">2\r\n") + fmt.Fprint(buf, ">2\r\n") if simpleOrString { - buf.WriteString(fmt.Sprintf("+%s\r\n", notificationName)) - buf.WriteString(fmt.Sprintf("+%s\r\n", data)) + fmt.Fprintf(buf, "+%s\r\n", notificationName) + fmt.Fprintf(buf, "+%s\r\n", data) } else { - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(notificationName), notificationName)) - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(notificationName), notificationName)) + fmt.Fprint(buf, defMsg) + fmt.Fprint(buf, defMsg) } } @@ -333,14 +334,14 @@ func createPushNotificationWithArgs(notificationName string, args ...string) *by buf := &bytes.Buffer{} totalElements := 1 + len(args) - buf.WriteString(fmt.Sprintf(">%d\r\n", totalElements)) + fmt.Fprintf(buf, ">%d\r\n", totalElements) // Write notification name - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(notificationName), notificationName)) + fmt.Fprintf(buf, "$%d\r\n%s\r\n", len(notificationName), notificationName) // Write arguments for _, arg := range args { - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(arg), arg)) + fmt.Fprintf(buf, "$%d\r\n%s\r\n", len(arg), arg) } return buf @@ -349,8 +350,8 @@ func createPushNotificationWithArgs(notificationName string, args ...string) *by // createSingleElementPushNotification creates a push notification with single element func createSingleElementPushNotification(notificationName string) *bytes.Buffer { buf := &bytes.Buffer{} - buf.WriteString(">1\r\n") - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(notificationName), notificationName)) + fmt.Fprint(buf, ">1\r\n") + fmt.Fprintf(buf, "$%d\r\n%s\r\n", len(notificationName), notificationName) return buf } diff --git a/internal/proto/reader_test.go b/internal/proto/reader_test.go index 2d5f56c7..ace954fa 100644 --- a/internal/proto/reader_test.go +++ b/internal/proto/reader_test.go @@ -2,6 +2,7 @@ package proto_test import ( "bytes" + "fmt" "io" "testing" @@ -86,7 +87,7 @@ func TestReader_ReadLine(t *testing.T) { func benchmarkParseReply(b *testing.B, reply string, wanterr bool) { buf := new(bytes.Buffer) for i := 0; i < b.N; i++ { - buf.WriteString(reply) + fmt.Fprint(buf, reply) } p := proto.NewReader(buf) b.ResetTimer() diff --git a/push/push_test.go b/push/push_test.go index b98b5f16..4ce4a24d 100644 --- a/push/push_test.go +++ b/push/push_test.go @@ -837,14 +837,14 @@ func createFakeRESP3PushNotification(notificationType string, args ...string) *b // RESP3 Push notification format: >\r\n\r\n totalElements := 1 + len(args) // notification type + arguments - buf.WriteString(fmt.Sprintf(">%d\r\n", totalElements)) + fmt.Fprintf(buf, ">%d\r\n", totalElements) // Write notification type as bulk string - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(notificationType), notificationType)) + fmt.Fprintf(buf, "$%d\r\n%s\r\n", len(notificationType), notificationType) // Write arguments as bulk strings for _, arg := range args { - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(arg), arg)) + fmt.Fprintf(buf, "$%d\r\n%s\r\n", len(arg), arg) } return buf @@ -868,11 +868,11 @@ func createFakeRESP3Array(elements ...string) *bytes.Buffer { buf := &bytes.Buffer{} // RESP3 Array format: *\r\n\r\n - buf.WriteString(fmt.Sprintf("*%d\r\n", len(elements))) + fmt.Fprintf(buf, "*%d\r\n", len(elements)) // Write elements as bulk strings for _, element := range elements { - buf.WriteString(fmt.Sprintf("$%d\r\n%s\r\n", len(element), element)) + fmt.Fprintf(buf, "$%d\r\n%s\r\n", len(element), element) } return buf @@ -881,7 +881,7 @@ func createFakeRESP3Array(elements ...string) *bytes.Buffer { // createFakeRESP3Error creates a fake RESP3 error func createFakeRESP3Error(message string) *bytes.Buffer { buf := &bytes.Buffer{} - buf.WriteString(fmt.Sprintf("-%s\r\n", message)) + fmt.Fprintf(buf, "-%s\r\n", message) return buf } @@ -1117,7 +1117,7 @@ func TestProcessorWithFakeBuffer(t *testing.T) { // Create fake RESP3 push notification with no elements buf := &bytes.Buffer{} - buf.WriteString(">0\r\n") // Empty push notification + fmt.Fprint(buf, ">0\r\n") // Empty push notification reader := createReaderWithPrimedBuffer(buf) ctx := context.Background() @@ -1154,9 +1154,9 @@ func TestProcessorWithFakeBuffer(t *testing.T) { // Create fake RESP3 push notification with integer as first element buf := &bytes.Buffer{} - buf.WriteString(">2\r\n") // 2 elements - buf.WriteString(":123\r\n") // Integer instead of string - buf.WriteString("$4\r\ndata\r\n") // String data + fmt.Fprint(buf, ">2\r\n") // 2 elements + fmt.Fprint(buf, ":123\r\n") // Integer instead of string + fmt.Fprint(buf, "$4\r\ndata\r\n") // String data reader := proto.NewReader(buf) ctx := context.Background() @@ -1273,8 +1273,8 @@ func TestVoidProcessorWithFakeBuffer(t *testing.T) { // Create invalid RESP3 data buf := &bytes.Buffer{} - buf.WriteString(">1\r\n") // Push notification with 1 element - buf.WriteString("invalid\r\n") // Invalid format (should be $\r\n\r\n) + fmt.Fprint(buf, ">1\r\n") // Push notification with 1 element + fmt.Fprint(buf, "invalid\r\n") // Invalid format (should be $\r\n\r\n) reader := proto.NewReader(buf) ctx := context.Background() @@ -1332,9 +1332,9 @@ func TestProcessorErrorHandling(t *testing.T) { // Create buffer with corrupted RESP3 data buf := &bytes.Buffer{} - buf.WriteString(">2\r\n") // Says 2 elements - buf.WriteString("$6\r\nMOVING\r\n") // First element OK - buf.WriteString("corrupted") // Second element corrupted (no proper format) + fmt.Fprint(buf, ">2\r\n") // Says 2 elements + fmt.Fprint(buf, "$6\r\nMOVING\r\n") // First element OK + fmt.Fprint(buf, "corrupted") // Second element corrupted (no proper format) reader := proto.NewReader(buf) ctx := context.Background() @@ -1360,8 +1360,8 @@ func TestProcessorErrorHandling(t *testing.T) { // Create buffer with partial RESP3 data buf := &bytes.Buffer{} - buf.WriteString(">2\r\n") // Says 2 elements - buf.WriteString("$6\r\nMOVING\r\n") // First element OK + fmt.Fprint(buf, ">2\r\n") // Says 2 elements + fmt.Fprint(buf, "$6\r\nMOVING\r\n") // First element OK // Missing second element reader := proto.NewReader(buf) From f7a8a1c1d7c402c11d29ad7cb1d79aa1d291286d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 28 Oct 2025 11:44:06 +0200 Subject: [PATCH 3/3] chore(deps): bump rojopolis/spellcheck-github-actions (#3569) Bumps [rojopolis/spellcheck-github-actions](https://github.com/rojopolis/spellcheck-github-actions) from 0.52.0 to 0.53.0. - [Release notes](https://github.com/rojopolis/spellcheck-github-actions/releases) - [Changelog](https://github.com/rojopolis/spellcheck-github-actions/blob/master/CHANGELOG.md) - [Commits](https://github.com/rojopolis/spellcheck-github-actions/compare/0.52.0...0.53.0) --- updated-dependencies: - dependency-name: rojopolis/spellcheck-github-actions dependency-version: 0.53.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/spellcheck.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml index 1517c339..12902ba2 100644 --- a/.github/workflows/spellcheck.yml +++ b/.github/workflows/spellcheck.yml @@ -8,7 +8,7 @@ jobs: - name: Checkout uses: actions/checkout@v5 - name: Check Spelling - uses: rojopolis/spellcheck-github-actions@0.52.0 + uses: rojopolis/spellcheck-github-actions@0.53.0 with: config_path: .github/spellcheck-settings.yml task_name: Markdown