tolbertam commented on code in PR #1889:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1889#discussion_r2083597152
##########
session.go:
##########
@@ -454,6 +454,10 @@ func (s *Session) Bind(stmt string, b func(q *QueryInfo)
([]interface{}, error))
// Close closes all connections. The session is unusable after this
// operation.
func (s *Session) Close() {
+ // Closing a nil session is a non-event, return.
+ if s == nil {
Review Comment:
Agree that since there is nothing to do here, simply no-oping and returning
is how I'd want this to behave too, and seems more pleasant than SIGSEV'ing.
I guess the question is how far this should go? e.g. calling
`session.Closed()` would still panic when maybe it would be ok to return `true`
here as well? Doing so would also prevent other methods `KeyspaceMetadata()`
from panicking and instead return an error instead which could be nice (but
other methods would still panic which creates an inconsistency).
But I think that changing the behavior for API methods broadly around nil
receivers might be something hard to address without a major release. For
example `Query` returns `* Query`, and panics with a nil session, but i'm not
sure how it should behave if we handled nil gracefully since we can't change
the API to return an error without a major release.
Making this particular change around `Close()` seems fine to me before a
major release. I can't imagine anyone depending on this to panic.
Curious what others think about this and how the API should behave generally
around nil receivers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
