diff --git a/redis/connection_test.go b/redis/connection_test.go index 2eccde5..657f973 100644 --- a/redis/connection_test.go +++ b/redis/connection_test.go @@ -2,7 +2,6 @@ package redis_test import ( "fmt" - netURL "net/url" . "github.com/onsi/ginkgo" diff --git a/redis/pool.go b/redis/pool.go index 1278e14..59f4bcb 100644 --- a/redis/pool.go +++ b/redis/pool.go @@ -50,11 +50,11 @@ func (m *hosts) Get(host string) bool { } func generateConnection(url *netURL.URL) (redigo.Conn, error) { - // Then we expec the server to not ask for a password + // Then we expect the server to not ask for a password if hostsNotUsingAuth.Get(url.Host) { url.User = nil conn, err := redisurl.ConnectToURL(url.String()) - if err == redigoErrNoAuth { + if errors.Is(err, redigoErrNoAuth) { hostsNotUsingAuth.Remove(url.Host) return generateConnection(url) } @@ -63,7 +63,7 @@ func generateConnection(url *netURL.URL) (redigo.Conn, error) { // Then we expect the server to potentially ask for a password conn, err := redisurl.ConnectToURL(url.String()) - if err == redigoErrSentAuth { + if errors.Is(err, redigoErrSentAuth) || errors.Is(err, redigoErrSentAuth2) { hostsNotUsingAuth.Add(url.Host) return generateConnection(url) } diff --git a/redis/redis.go b/redis/redis.go index 3411afe..86e416e 100644 --- a/redis/redis.go +++ b/redis/redis.go @@ -7,8 +7,9 @@ import ( var ( ErrNil = redigo.ErrNil - redigoErrNoAuth = redigo.Error("NOAUTH Authentication required.") - redigoErrSentAuth = redigo.Error("ERR Client sent AUTH, but no password is set") + redigoErrNoAuth = redigo.Error("NOAUTH Authentication required.") + redigoErrSentAuth = redigo.Error("ERR Client sent AUTH, but no password is set") + redigoErrSentAuth2 = redigo.Error("ERR AUTH called without any password configured for the default user. Are you sure your configuration is correct?") ) type Z struct { diff --git a/redis/redisurl/redisurl.go b/redis/redisurl/redisurl.go index a02a3cd..92e6bbd 100644 --- a/redis/redisurl/redisurl.go +++ b/redis/redisurl/redisurl.go @@ -17,13 +17,11 @@ func Connect() (redis.Conn, error) { func ConnectToURL(s string) (c redis.Conn, err error) { redisURL, err := url.Parse(s) - if err != nil { - return + return nil, err } auth := "" - if redisURL.User != nil { if password, ok := redisURL.User.Password(); ok { auth = password @@ -31,25 +29,26 @@ func ConnectToURL(s string) (c redis.Conn, err error) { } c, err = redis.Dial("tcp", redisURL.Host) - if err != nil { fmt.Println(err) - return + return nil, err } if len(auth) > 0 { _, err = c.Do("AUTH", auth) - if err != nil { fmt.Println(err) - return + return c, err } } if len(redisURL.Path) > 1 { db := strings.TrimPrefix(redisURL.Path, "/") - c.Do("SELECT", db) + _, err = c.Do("SELECT", db) + if err != nil { + return c, err + } } - return + return c, err } diff --git a/redis/redisurl/redisurl_test.go b/redis/redisurl/redisurl_test.go index 3fb1921..1b0b34d 100644 --- a/redis/redisurl/redisurl_test.go +++ b/redis/redisurl/redisurl_test.go @@ -9,10 +9,14 @@ import ( func TestConnect(t *testing.T) { c, err := redisurl.Connect() - if err != nil { - t.Errorf("Error returned") + t.Fatalf("Error returned: %v", err) + } + + if c == nil { + t.Fatal("Connection is nil") } + defer c.Close() pong, err := redis.String(c.Do("PING")) @@ -24,3 +28,42 @@ func TestConnect(t *testing.T) { t.Errorf("Wanted PONG, got %v\n", pong) } } + +func TestConnectToURL_InvalidURL(t *testing.T) { + c, err := redisurl.ConnectToURL("://invalid url") + if err == nil { + t.Error("Expected error for invalid URL, got nil") + } + + if c != nil { + t.Error("Expected nil connection for invalid URL") + } +} + +func TestConnectToURL_InvalidHost(t *testing.T) { + c, err := redisurl.ConnectToURL("redis://nonexistent-host-12345:6379") + if err == nil { + t.Error("Expected error for invalid host, got nil") + } + + // This is the critical check - connection should be nil on error + if c != nil { + t.Error("Expected nil connection when dial fails") + } +} + +func TestConnectToURL_WithAuth(t *testing.T) { + c, err := redisurl.ConnectToURL("redis://:password@localhost:6379/10") + // When AUTH fails, connection is returned so caller can check error type and retry + if err != nil { + if c == nil { + t.Error("Connection should be returned even when AUTH fails (for fallback logic)") + } else { + c.Close() // Caller's responsibility to close on error + } + } + + if c != nil && err == nil { + defer c.Close() + } +}