PubSub.Channel and Ring.Subscribe/PSubscribe/SSubscribe panic instead of returning error

[quocvibui]

Summary

Several exported methods on PubSub and Ring call panic() on recoverable error conditions instead of returning errors. Callers cannot anticipate or guard against these panics because the function signatures do not include error returns and the doc comments do not mention panic behavior.

Version: go-redis/v9 (commit 68197a9)
Go: 1.24

Affected Code

1. PubSub.Channel() / ChannelWithSubscriptions() — mutual exclusion panic

// pubsub.go:562-597
func (c *PubSub) Channel(opts ...ChannelOption) <-chan *Message {
    c.chOnce.Do(func() {
        c.msgCh = newChannel(c, opts...)
        c.msgCh.initMsgChan()
    })
    if c.msgCh == nil {
        err := fmt.Errorf("redis: Channel can't be called after ChannelWithSubscriptions")
        panic(err)  // panics instead of returning error
    }
    return c.msgCh.msgCh
}

The doc comment for ChannelWithSubscriptions says "can not be used together with Channel or ChannelSize" but does not mention that violating this constraint causes a panic. A caller who accidentally uses both gets an unrecoverable crash.

2. Ring.Subscribe() / PSubscribe() / SSubscribe() — empty channels + shard error panic

// ring.go:684-722
func (c *Ring) Subscribe(ctx context.Context, channels ...string) *PubSub {
    if len(channels) == 0 {
        panic("at least one channel is required")
    }
    shard, err := c.sharding.GetByKey(channels[0])
    if err != nil {
        // TODO: return PubSub with sticky error
        panic(err)
    }
    return shard.Client.Subscribe(ctx, channels...)
}

All three methods (Subscribe, PSubscribe, SSubscribe) have the same two panic paths: (a) empty channels slice, (b) shard lookup failure. The source even has // TODO: return PubSub with sticky error acknowledging the panic should be replaced.

Reproduction

package main

import (
    "context"
    "github.com/redis/go-redis/v9"
)

func main() {
    // Bug 1: PubSub mutual exclusion panic
    client := redis.NewClient(&redis.Options{Addr: "localhost:6379"})
    pubsub := client.Subscribe(context.Background())
    _ = pubsub.ChannelWithSubscriptions()
    _ = pubsub.Channel()
    // panic: redis: Channel can't be called after ChannelWithSubscriptions

    // Bug 2: Ring empty channels panic
    ring := redis.NewRing(&redis.RingOptions{})
    ring.Subscribe(context.Background())
    // panic: at least one channel is required
}

Why This Matters

• PubSub.Channel(), Ring.Subscribe(), Ring.PSubscribe(), and Ring.SSubscribe() are exported public API methods on core types
• None of the doc comments mention panic behavior
• The panics are triggered by recoverable misuse (calling methods in wrong order, empty arguments, shard errors) — exactly the cases where returning an error would be appropriate
• The existing // TODO comments in ring.go confirm the maintainers already identified this as a problem

Suggested Fix

For PubSub.Channel / ChannelWithSubscriptions: return a closed channel and log a warning, or add an error return value.

For Ring.Subscribe / PSubscribe / SSubscribe: return a *PubSub with a sticky error (as the TODO suggests), or change the signature to return (*PubSub, error).

// Example for Ring.Subscribe:
func (c *Ring) Subscribe(ctx context.Context, channels ...string) (*PubSub, error) {
    if len(channels) == 0 {
        return nil, fmt.Errorf("redis: at least one channel is required")
    }
    shard, err := c.sharding.GetByKey(channels[0])
    if err != nil {
        return nil, fmt.Errorf("redis: %w", err)
    }
    return shard.Client.Subscribe(ctx, channels...), nil
}

[ndyakov]

@quocvibui thank you for reporting this. Your suggestion good and would be the correct approach to handle errors here, however we cannot do this breaking change as part of v9. We will look into it for v10.

[thani-ath-nain]

@ndyakov I see this is tracked for v10. Since changing these panics to errors will likely involve signature changes, I'm happy to handle the refactor and update the corresponding tests to ensure v10 stability. Are you open to a PR for this now that can be staged for the v10 branch? I’ve used go-redis for 3+ years and would love to contribute this fix.