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

Panic on Calling Drain on a nil NATS Connection #1562

Open
mrazimi99 opened this issue Feb 21, 2024 · 2 comments · May be fixed by #1567
Open

Panic on Calling Drain on a nil NATS Connection #1562

mrazimi99 opened this issue Feb 21, 2024 · 2 comments · May be fixed by #1567
Labels
defect Suspected defect such as a bug or regression

Comments

@mrazimi99
Copy link

Observed behavior

When Drain is called on a NATS connection object and it is nil, the program panics; while calling Drain on a Subscription does not lead to a panic and returns error.

Expected behavior

I expect that calling Drain() on a NATS connection, an error be returned, instead of panic.

Server and client version

v1.32.0

Host environment

No response

Steps to reproduce

Create a NATS connection with an invalid URL, and the call Drain() on it.
@mrazimi99 mrazimi99 added the defect Suspected defect such as a bug or regression label Feb 21, 2024
mrazimi99 added a commit to mrazimi99/nats.go that referenced this issue Feb 24, 2024
@mrazimi99
[Fixed] Check if nats connection is nil in Drain()
7b2ac25 
Resolves nats-io#1562
@mrazimi99 mrazimi99 linked a pull request Feb 25, 2024 that will close this issue
Open
@Jarema
Copy link
Member

Jarema commented Feb 29, 2024

Thanks for the issue and the PR!

However, I think it's user responsibility to check error on Connect, as if it is not nil, the Conn can be nil, which is an idiomatic pattern in Go.

Unless you found a scenario, that Connect does not return an error, yet Conn is nil.
Then, we should fix that, but not by checking that nil, but instead by ensuring that error is returned.

@mrazimi99
Copy link
Author

Thank you for your reply!

Correct, however, there's a similar code for Subscription's Drain, here, checking that the Subscription is not nil. Is there any difference between these two?

@mrazimi99 mrazimi99 mentioned this issue Feb 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Fixed] Check if nats connection is nil in Drain() mrazimi99/nats.go
2 participants
@Jarema @mrazimi99