Remove unused ConnectionOption error. (#859)

Author: matthewdale

x/mongo/driver/topology/connection.go

@@ -72,11 +72,8 @@ type connection struct {
 }
 
 // newConnection handles the creation of a connection. It does not connect the connection.
-func newConnection(addr address.Address, opts ...ConnectionOption) (*connection, error) {
-	cfg, err := newConnectionConfig(opts...)
-	if err != nil {
-		return nil, err
-	}
+func newConnection(addr address.Address, opts ...ConnectionOption) *connection {
+	cfg := newConnectionConfig(opts...)
 
 	id := fmt.Sprintf("%s[-%d]", addr, nextConnectionID())
 
@@ -98,7 +95,7 @@ func newConnection(addr address.Address, opts ...ConnectionOption) (*connection,
 	}
 	atomic.StoreInt64(&c.connected, initialized)
 
-	return c, nil
+	return c
 }
 
 // setGenerationNumber sets the connection's generation number if a callback has been provided to do so in connection

x/mongo/driver/topology/connection_errors_test.go

@@ -25,16 +25,15 @@ func TestConnectionErrors(t *testing.T) {
 		t.Run("dial error", func(t *testing.T) {
 			dialError := errors.New("foo")
 
-			conn, err := newConnection(address.Address(""), WithDialer(func(Dialer) Dialer {
+			conn := newConnection(address.Address(""), WithDialer(func(Dialer) Dialer {
 				return DialerFunc(func(context.Context, string, string) (net.Conn, error) { return nil, dialError })
 			}))
-			assert.Nil(t, err, "newConnection error: %v", err)
 
-			err = conn.connect(context.Background())
+			err := conn.connect(context.Background())
 			assert.True(t, errors.Is(err, dialError), "expected error %v, got %v", dialError, err)
 		})
 		t.Run("handshake error", func(t *testing.T) {
-			conn, err := newConnection(address.Address(""),
+			conn := newConnection(address.Address(""),
 				WithHandshaker(func(Handshaker) Handshaker {
 					return auth.Handshaker(nil, &auth.HandshakeOptions{})
 				}),
@@ -44,12 +43,11 @@ func TestConnectionErrors(t *testing.T) {
 					})
 				}),
 			)
-			assert.Nil(t, err, "newConnection error: %v", err)
 			defer conn.close()
 
 			ctx, cancel := context.WithCancel(context.Background())
 			cancel()
-			err = conn.connect(ctx)
+			err := conn.connect(ctx)
 			assert.True(t, errors.Is(err, context.Canceled), "expected error %v, got %v", context.Canceled, err)
 		})
 		t.Run("write error", func(t *testing.T) {

x/mongo/driver/topology/connection_options.go

@@ -58,156 +58,137 @@ type connectionConfig struct {
 	getGenerationFn          generationNumberFn
 }
 
-func newConnectionConfig(opts ...ConnectionOption) (*connectionConfig, error) {
+func newConnectionConfig(opts ...ConnectionOption) *connectionConfig {
 	cfg := &connectionConfig{
 		connectTimeout:      30 * time.Second,
 		dialer:              nil,
 		tlsConnectionSource: defaultTLSConnectionSource,
 	}
 
 	for _, opt := range opts {
-		err := opt(cfg)
-		if err != nil {
-			return nil, err
-		}
+		opt(cfg)
 	}
 
 	if cfg.dialer == nil {
 		cfg.dialer = &net.Dialer{}
 	}
 
-	return cfg, nil
+	return cfg
 }
 
 // ConnectionOption is used to configure a connection.
-type ConnectionOption func(*connectionConfig) error
+type ConnectionOption func(*connectionConfig)
 
 func withTLSConnectionSource(fn func(tlsConnectionSource) tlsConnectionSource) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.tlsConnectionSource = fn(c.tlsConnectionSource)
-		return nil
 	}
 }
 
 // WithCompressors sets the compressors that can be used for communication.
 func WithCompressors(fn func([]string) []string) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.compressors = fn(c.compressors)
-		return nil
 	}
 }
 
 // WithConnectTimeout configures the maximum amount of time a dial will wait for a
 // Connect to complete. The default is 30 seconds.
 func WithConnectTimeout(fn func(time.Duration) time.Duration) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.connectTimeout = fn(c.connectTimeout)
-		return nil
 	}
 }
 
 // WithDialer configures the Dialer to use when making a new connection to MongoDB.
 func WithDialer(fn func(Dialer) Dialer) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.dialer = fn(c.dialer)
-		return nil
 	}
 }
 
 // WithHandshaker configures the Handshaker that wll be used to initialize newly
 // dialed connections.
 func WithHandshaker(fn func(Handshaker) Handshaker) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.handshaker = fn(c.handshaker)
-		return nil
 	}
 }
 
 // WithIdleTimeout configures the maximum idle time to allow for a connection.
 func WithIdleTimeout(fn func(time.Duration) time.Duration) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.idleTimeout = fn(c.idleTimeout)
-		return nil
 	}
 }
 
 // WithReadTimeout configures the maximum read time for a connection.
 func WithReadTimeout(fn func(time.Duration) time.Duration) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.readTimeout = fn(c.readTimeout)
-		return nil
 	}
 }
 
 // WithWriteTimeout configures the maximum write time for a connection.
 func WithWriteTimeout(fn func(time.Duration) time.Duration) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.writeTimeout = fn(c.writeTimeout)
-		return nil
 	}
 }
 
 // WithTLSConfig configures the TLS options for a connection.
 func WithTLSConfig(fn func(*tls.Config) *tls.Config) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.tlsConfig = fn(c.tlsConfig)
-		return nil
 	}
 }
 
 // WithMonitor configures a event for command monitoring.
 func WithMonitor(fn func(*event.CommandMonitor) *event.CommandMonitor) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.cmdMonitor = fn(c.cmdMonitor)
-		return nil
 	}
 }
 
 // WithZlibLevel sets the zLib compression level.
 func WithZlibLevel(fn func(*int) *int) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.zlibLevel = fn(c.zlibLevel)
-		return nil
 	}
 }
 
 // WithZstdLevel sets the zstd compression level.
 func WithZstdLevel(fn func(*int) *int) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.zstdLevel = fn(c.zstdLevel)
-		return nil
 	}
 }
 
 // WithOCSPCache specifies a cache to use for OCSP verification.
 func WithOCSPCache(fn func(ocsp.Cache) ocsp.Cache) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.ocspCache = fn(c.ocspCache)
-		return nil
 	}
 }
 
 // WithDisableOCSPEndpointCheck specifies whether or the driver should perform non-stapled OCSP verification. If set
 // to true, the driver will only check stapled responses and will continue the connection without reaching out to
 // OCSP responders.
 func WithDisableOCSPEndpointCheck(fn func(bool) bool) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.disableOCSPEndpointCheck = fn(c.disableOCSPEndpointCheck)
-		return nil
 	}
 }
 
 // WithConnectionLoadBalanced specifies whether or not the connection is to a server behind a load balancer.
 func WithConnectionLoadBalanced(fn func(bool) bool) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.loadBalanced = fn(c.loadBalanced)
-		return nil
 	}
 }
 
 func withGenerationNumberFn(fn func(generationNumberFn) generationNumberFn) ConnectionOption {
-	return func(c *connectionConfig) error {
+	return func(c *connectionConfig) {
 		c.getGenerationFn = fn(c.getGenerationFn)
-		return nil
 	}
 }

x/mongo/driver/topology/connection_test.go

@@ -50,16 +50,8 @@ var _ driver.Handshaker = &testHandshaker{}
 func TestConnection(t *testing.T) {
 	t.Run("connection", func(t *testing.T) {
 		t.Run("newConnection", func(t *testing.T) {
-			t.Run("config error", func(t *testing.T) {
-				want := errors.New("config error")
-				_, got := newConnection(address.Address(""), ConnectionOption(func(*connectionConfig) error { return want }))
-				if !cmp.Equal(got, want, cmp.Comparer(compareErrors)) {
-					t.Errorf("errors do not match. got %v; want %v", got, want)
-				}
-			})
 			t.Run("no default idle timeout", func(t *testing.T) {
-				conn, err := newConnection(address.Address(""))
-				assert.Nil(t, err, "newConnection error: %v", err)
+				conn := newConnection(address.Address(""))
 				wantTimeout := time.Duration(0)
 				assert.Equal(t, wantTimeout, conn.idleTimeout, "expected idle timeout %v, got %v", wantTimeout,
 					conn.idleTimeout)
@@ -69,13 +61,10 @@ func TestConnection(t *testing.T) {
 			t.Run("dialer error", func(t *testing.T) {
 				err := errors.New("dialer error")
 				var want error = ConnectionError{Wrapped: err, init: true}
-				conn, got := newConnection(address.Address(""), WithDialer(func(Dialer) Dialer {
+				conn := newConnection(address.Address(""), WithDialer(func(Dialer) Dialer {
 					return DialerFunc(func(context.Context, string, string) (net.Conn, error) { return nil, err })
 				}))
-				if got != nil {
-					t.Errorf("newConnection shouldn't error. got %v; want nil", got)
-				}
-				got = conn.connect(context.Background())
+				got := conn.connect(context.Background())
 				if !cmp.Equal(got, want, cmp.Comparer(compareErrors)) {
 					t.Errorf("errors do not match. got %v; want %v", got, want)
 				}
@@ -85,7 +74,7 @@ func TestConnection(t *testing.T) {
 			t.Run("handshaker error", func(t *testing.T) {
 				err := errors.New("handshaker error")
 				var want error = ConnectionError{Wrapped: err, init: true}
-				conn, got := newConnection(address.Address(""),
+				conn := newConnection(address.Address(""),
 					WithHandshaker(func(Handshaker) Handshaker {
 						return &testHandshaker{
 							finishHandshake: func(context.Context, driver.Connection) error {
@@ -99,10 +88,7 @@ func TestConnection(t *testing.T) {
 						})
 					}),
 				)
-				if got != nil {
-					t.Errorf("newConnection shouldn't error. got %v; want nil", got)
-				}
-				got = conn.connect(context.Background())
+				got := conn.connect(context.Background())
 				if !cmp.Equal(got, want, cmp.Comparer(compareErrors)) {
 					t.Errorf("errors do not match. got %v; want %v", got, want)
 				}
@@ -117,7 +103,7 @@ func TestConnection(t *testing.T) {
 				t.Run("connect succeeds", func(t *testing.T) {
 					// In the case where connect finishes successfully, it unpins the CancelFunc.
 
-					conn, err := newConnection(address.Address(""),
+					conn := newConnection(address.Address(""),
 						WithDialer(func(Dialer) Dialer {
 							return DialerFunc(func(context.Context, string, string) (net.Conn, error) {
 								return &net.TCPConn{}, nil
@@ -127,9 +113,8 @@ func TestConnection(t *testing.T) {
 							return &testHandshaker{}
 						}),
 					)
-					assert.Nil(t, err, "newConnection error: %v", err)
 
-					err = conn.connect(context.Background())
+					err := conn.connect(context.Background())
 					assert.Nil(t, err, "error establishing connection: %v", err)
 					assert.Nil(t, conn.cancelConnectContext, "cancellation function was not cleared")
 				})
@@ -140,7 +125,7 @@ func TestConnection(t *testing.T) {
 					// Create a connection that will block in connect until doneChan is closed. This prevents
 					// connect from succeeding and unpinning the CancelFunc.
 					doneChan := make(chan struct{})
-					conn, err := newConnection(address.Address(""),
+					conn := newConnection(address.Address(""),
 						WithDialer(func(Dialer) Dialer {
 							return DialerFunc(func(context.Context, string, string) (net.Conn, error) {
 								<-doneChan
@@ -151,7 +136,6 @@ func TestConnection(t *testing.T) {
 							return &testHandshaker{}
 						}),
 					)
-					assert.Nil(t, err, "newConnection error: %v", err)
 
 					// Call connect in a goroutine because it will block.
 					var wg sync.WaitGroup
@@ -170,8 +154,7 @@ func TestConnection(t *testing.T) {
 			})
 			t.Run("tls", func(t *testing.T) {
 				t.Run("connection source is set to default if unspecified", func(t *testing.T) {
-					conn, err := newConnection(address.Address(""))
-					assert.Nil(t, err, "newConnection error: %v", err)
+					conn := newConnection(address.Address(""))
 					assert.NotNil(t, conn.config.tlsConnectionSource, "expected tlsConnectionSource to be set but was not")
 				})
 				t.Run("server name", func(t *testing.T) {
@@ -208,8 +191,7 @@ func TestConnection(t *testing.T) {
 									return testTLSConnectionSource
 								}),
 							}
-							conn, err := newConnection(tc.addr, connOpts...)
-							assert.Nil(t, err, "newConnection error: %v", err)
+							conn := newConnection(tc.addr, connOpts...)
 
 							_ = conn.connect(context.Background())
 							assert.NotNil(t, sentCfg, "expected TLS config to be set, but was not")
@@ -250,8 +232,7 @@ func TestConnection(t *testing.T) {
 								return tc.connectTimeout
 							}),
 						}
-						conn, err := newConnection("", connOpts...)
-						assert.Nil(t, err, "newConnection error: %v", err)
+						conn := newConnection("", connOpts...)
 
 						ctx, cancel := context.WithTimeout(context.Background(), tc.contextTimeout)
 						defer cancel()
@@ -291,8 +272,7 @@ func TestConnection(t *testing.T) {
 								return hangingTLSConnectionSource
 							}),
 						}
-						conn, err := newConnection("", connOpts...)
-						assert.Nil(t, err, "newConnection error: %v", err)
+						conn := newConnection("", connOpts...)
 
 						ctx, cancel := context.WithTimeout(context.Background(), tc.contextTimeout)
 						defer cancel()
@@ -336,11 +316,10 @@ func TestConnection(t *testing.T) {
 								return handshaker
 							}),
 						}
-						conn, err := newConnection("", connOpts...)
-						assert.Nil(t, err, "newConnection error: %v", err)
+						conn := newConnection("", connOpts...)
 
 						bgCtx := context.Background()
-						err = conn.connect(bgCtx)
+						err := conn.connect(bgCtx)
 						assert.Nil(t, err, "connect error: %v", err)
 
 						assertNoContextTimeout := func(t *testing.T, ctx context.Context) {
@@ -697,7 +676,7 @@ func TestConnection(t *testing.T) {
 		})
 		t.Run("close", func(t *testing.T) {
 			t.Run("can close a connection that failed handshaking", func(t *testing.T) {
-				conn, err := newConnection(address.Address(""),
+				conn := newConnection(address.Address(""),
 					WithHandshaker(func(Handshaker) Handshaker {
 						return &testHandshaker{
 							finishHandshake: func(context.Context, driver.Connection) error {
@@ -711,9 +690,8 @@ func TestConnection(t *testing.T) {
 						})
 					}),
 				)
-				assert.Nil(t, err, "newConnection error: %v", err)
 
-				err = conn.connect(context.Background())
+				err := conn.connect(context.Background())
 				assert.NotNil(t, err, "expected handshake error from connect, got nil")
 				connState := atomic.LoadInt64(&conn.connected)
 				assert.Equal(t, disconnected, connState, "expected connection state %v, got %v", disconnected, connState)