GODRIVER-151 change handling of network errors or timeouts during connection handshake

Change-Id: Ic43a7cd77a9249c802820f561f565f9ebac35b2c

Author: Isabella Siu

core/topology/connection.go

@@ -53,8 +53,8 @@ func (sc *sconn) processErr(err error) {
 	if cerr, ok := err.(command.Error); ok && (isRecoveringError(cerr) || isNotMasterError(cerr)) {
 		desc := sc.s.Description()
 		desc.Kind = description.Unknown
-
-		// updates description to unknown and clears the connection pool
+		desc.LastError = err
+		// updates description to unknown
 		sc.s.updateDescription(desc, false)
 	}
 
@@ -70,7 +70,11 @@ func (sc *sconn) processErr(err error) {
 		return
 	}
 
-	_ = sc.s.Drain()
+	desc := sc.s.Description()
+	desc.Kind = description.Unknown
+	desc.LastError = err
+	// updates description to unknown
+	sc.s.updateDescription(desc, false)
 }
 
 func isRecoveringError(err command.Error) bool {

core/topology/connection_test.go

@@ -0,0 +1,72 @@
+package topology
+
+import (
+	"context"
+	"testing"
+
+	"github.com/mongodb/mongo-go-driver/core/address"
+	"github.com/mongodb/mongo-go-driver/core/connection"
+	"github.com/mongodb/mongo-go-driver/core/description"
+	"github.com/mongodb/mongo-go-driver/core/wiremessage"
+	"github.com/stretchr/testify/require"
+)
+
+type netErr struct {
+}
+
+func (n netErr) Error() string {
+	return "error"
+}
+
+func (n netErr) Timeout() bool {
+	return false
+}
+
+func (n netErr) Temporary() bool {
+	return false
+}
+
+type connect struct {
+	err *connection.NetworkError
+}
+
+func (c connect) WriteWireMessage(ctx context.Context, wm wiremessage.WireMessage) error {
+	return *c.err
+}
+func (c connect) ReadWireMessage(ctx context.Context) (wiremessage.WireMessage, error) {
+	return nil, *c.err
+}
+func (c connect) Close() error {
+	return nil
+}
+func (c connect) Alive() bool {
+	return true
+}
+func (c connect) Expired() bool {
+	return false
+}
+func (c connect) ID() string {
+	return ""
+}
+
+// Test case for sconn processErr
+func TestConnectionProcessErrSpec(t *testing.T) {
+	ctx := context.Background()
+	s, err := NewServer(address.Address("localhost"))
+	require.NoError(t, err)
+
+	desc := s.Description()
+	require.Nil(t, desc.LastError)
+
+	s.connectionstate = connected
+
+	innerErr := netErr{}
+	connectErr := connection.NetworkError{"blah", innerErr}
+	c := connect{&connectErr}
+	sc := sconn{c, s, 1}
+	err = sc.WriteWireMessage(ctx, nil)
+	require.NotNil(t, err)
+	desc = s.Description()
+	require.NotNil(t, desc.LastError)
+	require.Equal(t, desc.Kind, (description.ServerKind)(description.Unknown))
+}

core/topology/server.go

@@ -182,6 +182,16 @@ func (s *Server) Connection(ctx context.Context) (connection.Connection, error)
 			// authentication error --> drain connection
 			_ = s.pool.Drain()
 		}
+		if _, ok := err.(*connection.NetworkError); ok {
+			// update description to unknown and clears the connection pool
+			if desc != nil {
+				desc.Kind = description.Unknown
+				desc.LastError = err
+				s.updateDescription(*desc, false)
+			} else {
+				_ = s.pool.Drain()
+			}
+		}
 		return nil, err
 	}
 	if desc != nil {

core/topology/server_test.go

@@ -8,6 +8,7 @@ package topology
 
 import (
 	"context"
+	"sync/atomic"
 	"testing"
 
 	"github.com/mongodb/mongo-go-driver/core/address"
@@ -19,14 +20,19 @@ import (
 
 type pool struct {
 	connectionError bool
-	drainCalled     bool
+	drainCalled     atomic.Value
+	networkError    bool
+	desc            *description.Server
 }
 
 func (p *pool) Get(ctx context.Context) (connection.Connection, *description.Server, error) {
 	if p.connectionError {
-		return nil, nil, &auth.Error{}
+		return nil, p.desc, &auth.Error{}
 	}
-	return nil, nil, nil
+	if p.networkError {
+		return nil, p.desc, &connection.NetworkError{}
+	}
+	return nil, p.desc, nil
 }
 
 func (p *pool) Connect(ctx context.Context) error {
@@ -38,43 +44,61 @@ func (p *pool) Disconnect(ctx context.Context) error {
 }
 
 func (p *pool) Drain() error {
-	p.drainCalled = true
+	p.drainCalled.Store(true)
 	return nil
 }
 
-func NewPool(connectionError bool) (connection.Pool, error) {
+func NewPool(connectionError bool, networkError bool, desc *description.Server) (connection.Pool, error) {
 	p := &pool{
 		connectionError: connectionError,
+		networkError:    networkError,
+		desc:            desc,
 	}
+	p.drainCalled.Store(false)
 	return p, nil
 }
 
 func TestSever(t *testing.T) {
 	var serverTestTable = []struct {
 		name            string
 		connectionError bool
+		networkError    bool
+		hasDesc         bool
 	}{
-		{"auth_error", true},
-		{"auth_no_error", false},
+		{"auth_error", true, false, false},
+		{"no_error", false, false, false},
+		{"network_error_no_desc", false, true, false},
+		{"network_error_desc", false, true, true},
 	}
 
 	for _, tt := range serverTestTable {
 		t.Run(tt.name, func(t *testing.T) {
 			s, err := NewServer(address.Address("localhost"))
 			require.NoError(t, err)
 
-			s.pool, err = NewPool(tt.connectionError)
+			var desc *description.Server
+			descript := s.Description()
+			if tt.hasDesc {
+				desc = &descript
+				require.Nil(t, desc.LastError)
+			}
+			s.pool, err = NewPool(tt.connectionError, tt.networkError, desc)
 			s.connectionstate = connected
 
 			_, err = s.Connection(context.Background())
 
-			if tt.connectionError {
+			if tt.connectionError || tt.networkError {
 				require.Error(t, err)
 			} else {
 				require.NoError(t, err)
 			}
 
-			require.Equal(t, s.pool.(*pool).drainCalled, tt.connectionError)
+			if tt.hasDesc {
+				require.Equal(t, desc.Kind, (description.ServerKind)(description.Unknown))
+				require.NotNil(t, desc.LastError)
+			}
+			drained := s.pool.(*pool).drainCalled.Load().(bool)
+			require.Equal(t, drained, tt.connectionError || tt.networkError)
 		})
 	}
 }