Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CHANGED] Remove unnecessary checks of nil for Connection #1576

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[CHANGED] Remove unnecessary checks of nil for Connection #1576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
692f840
[CHANGED] Remove unnecessary checks of `nil` for `Connection`
mrazimi99 Feb 29, 2024
93610d1
[Changed] Keep `ErrInvalidConnection` for backward compatibility
mrazimi99 Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions context.go
View file
Open in desktop
Expand Up @@ -41,9 +41,6 @@ func (nc *Conn) requestWithContext(ctx context.Context, subj string, hdr, data [
if ctx == nil {
return nil, ErrInvalidContext
}
if nc == nil {
return nil, ErrInvalidConnection
}
// Check whether the context is done already before making
// the request.
if ctx.Err() != nil {
Expand Down Expand Up @@ -172,9 +169,6 @@ func (s *Subscription) NextMsgWithContext(ctx context.Context) (*Msg, error) {
// of a Flush() call. This context should be non-nil and should
// have a deadline set. We will return an error if none is present.
func (nc *Conn) FlushWithContext(ctx context.Context) error {
if nc == nil {
return ErrInvalidConnection
}
if ctx == nil {
return ErrInvalidContext
}
Expand Down
22 changes: 0 additions & 22 deletions nats.go
View file
Open in desktop
Expand Up @@ -3533,9 +3533,6 @@ func (nc *Conn) processAsyncInfo(info []byte) {
// It can be used reliably within ClosedCB in order to find out reason
// why connection was closed for example.
func (nc *Conn) LastError() error {
if nc == nil {
return ErrInvalidConnection
}
nc.mu.RLock()
err := nc.err
nc.mu.RUnlock()
Expand Down Expand Up @@ -3768,9 +3765,6 @@ const digits = "0123456789"
// Sends a protocol data message by queuing into the bufio writer
// and kicking the flush go routine. These writes should be protected.
func (nc *Conn) publish(subj, reply string, hdr, data []byte) error {
if nc == nil {
return ErrInvalidConnection
}
if subj == "" {
return ErrBadSubject
}
Expand Down Expand Up @@ -3976,10 +3970,6 @@ func (nc *Conn) useOldRequestStyle() bool {
}

func (nc *Conn) request(subj string, hdr, data []byte, timeout time.Duration) (*Msg, error) {
if nc == nil {
return nil, ErrInvalidConnection
}

var m *Msg
var err error

Expand Down Expand Up @@ -4159,9 +4149,6 @@ func (nc *Conn) ChanQueueSubscribe(subj, group string, ch chan *Msg) (*Subscript
// SubscribeSync will express interest on the given subject. Messages will
// be received synchronously using Subscription.NextMsg().
func (nc *Conn) SubscribeSync(subj string) (*Subscription, error) {
if nc == nil {
return nil, ErrInvalidConnection
}
mch := make(chan *Msg, nc.Opts.SubChanLen)
return nc.subscribe(subj, _EMPTY_, nil, mch, true, nil)
}
Expand Down Expand Up @@ -4215,18 +4202,12 @@ func badQueue(qname string) bool {

// subscribe is the internal subscribe function that indicates interest in a subject.
func (nc *Conn) subscribe(subj, queue string, cb MsgHandler, ch chan *Msg, isSync bool, js *jsSub) (*Subscription, error) {
if nc == nil {
return nil, ErrInvalidConnection
}
nc.mu.Lock()
defer nc.mu.Unlock()
return nc.subscribeLocked(subj, queue, cb, ch, isSync, js)
}

func (nc *Conn) subscribeLocked(subj, queue string, cb MsgHandler, ch chan *Msg, isSync bool, js *jsSub) (*Subscription, error) {
if nc == nil {
return nil, ErrInvalidConnection
}
if badSubject(subj) {
return nil, ErrBadSubject
}
Expand Down Expand Up @@ -4939,9 +4920,6 @@ func (nc *Conn) processPingTimer() {

// FlushTimeout allows a Flush operation to have an associated timeout.
func (nc *Conn) FlushTimeout(timeout time.Duration) (err error) {
if nc == nil {
return ErrInvalidConnection
}
if timeout <= 0 {
return ErrBadTimeout
}
Expand Down
50 changes: 2 additions & 48 deletions test/basic_test.go
View file
Open in desktop
Expand Up @@ -772,7 +772,7 @@ func TestRequestClose(t *testing.T) {
nc.Close()
}()
nc.SubscribeSync("foo")
if _, err := nc.Request("foo", []byte("help"), 2*time.Second); err != nats.ErrInvalidConnection && err != nats.ErrConnectionClosed {
if _, err := nc.Request("foo", []byte("help"), 2*time.Second); err != nats.ErrConnectionClosed {
t.Fatalf("Expected connection error: got %v", err)
}
wg.Wait()
Expand Down Expand Up @@ -968,53 +968,7 @@ func TestOptions(t *testing.T) {
}
}

func TestNilConnection(t *testing.T) {
var nc *nats.Conn
data := []byte("ok")

// Publish
if err := nc.Publish("foo", data); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if err := nc.PublishMsg(nil); err == nil || err != nats.ErrInvalidMsg {
t.Fatalf("Expected ErrInvalidMsg error, got %v\n", err)
}
if err := nc.PublishMsg(&nats.Msg{}); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if err := nc.PublishRequest("foo", "reply", data); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}

// Subscribe
if _, err := nc.Subscribe("foo", nil); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if _, err := nc.SubscribeSync("foo"); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if _, err := nc.QueueSubscribe("foo", "bar", nil); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
ch := make(chan *nats.Msg)
if _, err := nc.ChanSubscribe("foo", ch); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if _, err := nc.ChanQueueSubscribe("foo", "bar", ch); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if _, err := nc.QueueSubscribeSyncWithChan("foo", "bar", ch); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}

// Flush
if err := nc.Flush(); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}
if err := nc.FlushTimeout(time.Millisecond); err == nil || err != nats.ErrInvalidConnection {
t.Fatalf("Expected ErrInvalidConnection error, got %v\n", err)
}

func TestNilSubscriber(t *testing.T) {
// Nil Subscribers
var sub *nats.Subscription
if sub.Type() != nats.NilSubscription {
Expand Down
15 changes: 0 additions & 15 deletions test/context_test.go
View file
Open in desktop
Expand Up @@ -26,21 +26,6 @@ import (
"github.com/nats-io/nats.go"
)

func TestContextRequestWithNilConnection(t *testing.T) {
var nc *nats.Conn

ctx, cancelCB := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancelCB() // should always be called, not discarded, to prevent context leak

_, err := nc.RequestWithContext(ctx, "fast", []byte(""))
if err == nil {
t.Fatal("Expected request with context and nil connection to fail")
}
if err != nats.ErrInvalidConnection {
t.Fatalf("Expected nats.ErrInvalidConnection, got %v\n", err)
}
}

func testContextRequestWithTimeout(t *testing.T, nc *nats.Conn) {
nc.Subscribe("slow", func(m *nats.Msg) {
// Simulates latency into the client so that timeout is hit.
Expand Down