feat: throw-panic-with-multiple-custom-clients-and-conflicting-options

Author: Zaimwa9

client.go

@@ -71,9 +71,14 @@ func NewClient(apiKey string, options ...Option) *Client {
 		config: defaultConfig(),
 	}
 
+	customClientCount := 0
 	for _, opt := range options {
 		name := getOptionQualifiedName(opt)
 		if isClientOption(name) {
+			customClientCount = customClientCount + 1
+			if customClientCount > 1 {
+				panic("Only one client option can be provided")
+			}
 			opt(c)
 		}
 	}
@@ -82,9 +87,13 @@ func NewClient(apiKey string, options ...Option) *Client {
 	if c.client == nil {
 		if c.httpClient != nil {
 			c.client = resty.NewWithClient(c.httpClient)
+			c.config.userProvidedClient = true
+
 		} else {
 			c.client = resty.New()
 		}
+	} else {
+		c.config.userProvidedClient = true
 	}
 
 	c.client.SetHeaders(map[string]string{

client_test.go

@@ -1098,36 +1098,20 @@ func TestCustomRestyClientIsUsed(t *testing.T) {
 	assert.True(t, hasCustomHeader, "Expected custom resty header")
 }
 
-func TestRestyClientOverridesHTTPClient(t *testing.T) {
-	ctx := context.Background()
-
-	var customHeader string
-	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
-		customHeader = req.Header.Get("X-Test-Client")
-		rw.Header().Set("Content-Type", "application/json")
-		rw.WriteHeader(http.StatusOK)
-		_, err := io.WriteString(rw, fixtures.FlagsJson)
-		assert.NoError(t, err)
-	}))
-	defer server.Close()
-
+func TestRestyClientOverridesHTTPClientShouldPanic(t *testing.T) {
 	httpClient := &http.Client{
 		Transport: roundTripperWithHeader("X-Test-Client", "http"),
 	}
 
 	restyClient := resty.New().
 		SetHeader("X-Test-Client", "resty")
 
-	client := flagsmith.NewClient(fixtures.EnvironmentAPIKey,
-		flagsmith.WithHTTPClient(httpClient),
-		flagsmith.WithRestyClient(restyClient),
-		flagsmith.WithBaseURL(server.URL+"/api/v1/"))
-
-	flags, err := client.GetFlags(ctx, nil)
-
-	assert.NoError(t, err)
-	assert.Equal(t, 1, len(flags.AllFlags()))
-	assert.Equal(t, "resty", customHeader, "Expected resty header")
+	assert.Panics(t, func() {
+		_ = flagsmith.NewClient(fixtures.EnvironmentAPIKey,
+			flagsmith.WithHTTPClient(httpClient),
+			flagsmith.WithRestyClient(restyClient),
+			flagsmith.WithBaseURL("http://example.com/api/v1/"))
+	}, "Expected panic when both HTTP and Resty clients are provided")
 }
 
 func TestDefaultRestyClientIsUsed(t *testing.T) {
@@ -1157,3 +1141,39 @@ func TestDefaultRestyClientIsUsed(t *testing.T) {
 	assert.True(t, serverCalled, "Expected server to be")
 	assert.Equal(t, 1, len(flags.AllFlags()))
 }
+
+func TestCustomClientOptionsShoudPanic(t *testing.T) {
+	restyClient := resty.New()
+
+	testCases := []struct {
+		name   string
+		option flagsmith.Option
+	}{
+		{
+			name:   "WithRequestTimeout",
+			option: flagsmith.WithRequestTimeout(5 * time.Second),
+		},
+		{
+			name:   "WithRetries",
+			option: flagsmith.WithRetries(3, time.Second),
+		},
+		{
+			name:   "WithCustomHeaders",
+			option: flagsmith.WithCustomHeaders(map[string]string{"X-Custom": "value"}),
+		},
+		{
+			name:   "WithProxy",
+			option: flagsmith.WithProxy("http://proxy.example.com"),
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			assert.Panics(t, func() {
+				_ = flagsmith.NewClient(fixtures.EnvironmentAPIKey,
+					flagsmith.WithRestyClient(restyClient),
+					test.option)
+			}, "Expected panic when using %s with custom resty client", test.name)
+		})
+	}
+}

config.go

@@ -27,6 +27,7 @@ type config struct {
 	realtimeBaseUrl    string
 	useRealtime        bool
 	polling            bool
+	userProvidedClient bool
 }
 
 // defaultConfig returns default configuration.
@@ -36,5 +37,6 @@ func defaultConfig() config {
 		timeout:            DefaultTimeout,
 		envRefreshInterval: time.Second * 60,
 		realtimeBaseUrl:    DefaultRealtimeBaseUrl,
+		userProvidedClient: false,
 	}
 }

options.go

@@ -65,6 +65,9 @@ func WithRemoteEvaluation() Option {
 
 func WithRequestTimeout(timeout time.Duration) Option {
 	return func(c *Client) {
+		if c.config.userProvidedClient {
+			panic("options modifying the client can not be used with a custom client")
+		}
 		c.client.SetTimeout(timeout)
 	}
 }
@@ -89,13 +92,19 @@ func WithAnalytics(ctx context.Context) Option {
 
 func WithRetries(count int, waitTime time.Duration) Option {
 	return func(c *Client) {
+		if c.config.userProvidedClient {
+			panic("options modifying the client can not be used with a custom client")
+		}
 		c.client.SetRetryCount(count)
 		c.client.SetRetryWaitTime(waitTime)
 	}
 }
 
 func WithCustomHeaders(headers map[string]string) Option {
 	return func(c *Client) {
+		if c.config.userProvidedClient {
+			panic("options modifying the client can not be used with a custom client")
+		}
 		c.client.SetHeaders(headers)
 	}
 }
@@ -124,6 +133,9 @@ func WithSlogLogger(logger *slog.Logger) Option {
 // The proxyURL argument is a string representing the URL of the proxy server to use, e.g. "http://proxy.example.com:8080".
 func WithProxy(proxyURL string) Option {
 	return func(c *Client) {
+		if c.config.userProvidedClient {
+			panic("options modifying the client can not be used with a custom client")
+		}
 		c.client.SetProxy(proxyURL)
 	}
 }