fix

Author: nikola-jokic

common_client.go

@@ -120,9 +120,7 @@ func (o *httpClientOption) newRetryableHTTPClient() (*retryablehttp.Client, erro
 		retryClient.HTTPClient.Timeout = o.timeout
 	}
 
-	if retryClient.Logger == nil {
-		retryClient.Logger = o.logger
-	}
+	retryClient.Logger = o.logger
 
 	transport, ok := retryClient.HTTPClient.Transport.(*http.Transport)
 	if !ok {

common_client_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
 	"testing"
 	"time"
 
@@ -158,17 +159,29 @@ func TestUserAgent(t *testing.T) {
 }
 
 func TestWithLogger(t *testing.T) {
+	newJSONHandler := func() slog.Handler {
+		return slog.NewJSONHandler(
+			os.Stdout,
+			&slog.HandlerOptions{
+				AddSource: true,
+				Level:     slog.LevelError,
+			},
+		)
+	}
 	t.Run("WithLogger(nil) sets a discard logger on raw httpClientOption", func(t *testing.T) {
 		opts := httpClientOption{}
 		WithLogger(nil)(&opts)
 		require.NotNil(t, opts.logger, "WithLogger(nil) should set a discard logger, not leave it nil")
+		assert.Equal(t, slog.DiscardHandler, opts.logger.Handler(), "WithLogger(nil) should set a discard logger handler")
 	})
 
 	t.Run("WithLogger(customLogger) assigns the provided logger", func(t *testing.T) {
-		customLogger := slog.New(slog.DiscardHandler)
+		handler := newJSONHandler()
+		customLogger := slog.New(handler)
 		opts := httpClientOption{}
 		WithLogger(customLogger)(&opts)
 		require.Equal(t, customLogger, opts.logger, "WithLogger should assign the provided logger")
+		assert.Equal(t, handler, opts.logger.Handler(), "WithLogger should set the provided logger handler")
 	})
 
 	t.Run("WithLogger(nil) propagates discard logger to retryable HTTP client", func(t *testing.T) {
@@ -177,59 +190,18 @@ func TestWithLogger(t *testing.T) {
 		client, err := opts.newRetryableHTTPClient()
 		require.NoError(t, err)
 		assert.NotNil(t, client.Logger, "retryable client should have logger set from WithLogger(nil)")
+		assert.Equal(t, slog.DiscardHandler, opts.logger.Handler(), "retryable client logger should be a discard logger from WithLogger(nil)")
 	})
 
 	t.Run("WithLogger(customLogger) propagates custom logger to retryable HTTP client", func(t *testing.T) {
-		customLogger := slog.New(slog.DiscardHandler)
+		handler := newJSONHandler()
+		customLogger := slog.New(handler)
 		opts := httpClientOption{}
 		WithLogger(customLogger)(&opts)
 		client, err := opts.newRetryableHTTPClient()
 		require.NoError(t, err)
 		assert.NotNil(t, client.Logger, "retryable client should have logger set")
-	})
-
-	t.Run("WithLogger(customLogger) propagates to custom retryable HTTP client", func(t *testing.T) {
-		providedLogger := slog.New(slog.DiscardHandler)
-		customRetryClient := retryablehttp.NewClient()
-		customRetryClient.Logger = nil
-
-		opts := httpClientOption{}
-		WithRetryableHTTPClint(customRetryClient)(&opts)
-		WithLogger(providedLogger)(&opts)
-
-		retrievedRetryClient, err := opts.newRetryableHTTPClient()
-		require.NoError(t, err)
-		require.Equal(t, customRetryClient, retrievedRetryClient, "should use the provided custom retryable client")
-		assert.Equal(t, providedLogger, retrievedRetryClient.Logger, "custom retryable client logger should be set from WithLogger")
-	})
-
-	t.Run("WithLogger(nil) with custom retryable HTTP client is order-invariant", func(t *testing.T) {
-		customRetryClientA := retryablehttp.NewClient()
-		customRetryClientA.Logger = nil
-		optsA := httpClientOption{}
-		WithLogger(nil)(&optsA)
-		WithRetryableHTTPClint(customRetryClientA)(&optsA)
-
-		retrievedA, err := optsA.newRetryableHTTPClient()
-		require.NoError(t, err)
-		require.Equal(t, customRetryClientA, retrievedA)
-		require.NotNil(t, optsA.logger, "WithLogger(nil) should assign discard logger")
-		assert.Equal(t, optsA.logger, retrievedA.Logger, "custom retryable client should receive logger from options")
-
-		customRetryClientB := retryablehttp.NewClient()
-		customRetryClientB.Logger = nil
-		optsB := httpClientOption{}
-		WithRetryableHTTPClint(customRetryClientB)(&optsB)
-		WithLogger(nil)(&optsB)
-
-		retrievedB, err := optsB.newRetryableHTTPClient()
-		require.NoError(t, err)
-		require.Equal(t, customRetryClientB, retrievedB)
-		require.NotNil(t, optsB.logger, "WithLogger(nil) should assign discard logger")
-		assert.Equal(t, optsB.logger, retrievedB.Logger, "custom retryable client should receive logger from options")
-
-		assert.NotNil(t, retrievedA.Logger)
-		assert.NotNil(t, retrievedB.Logger)
+		assert.Equal(t, handler, client.Logger.(*slog.Logger).Handler(), "retryable client logger should be the custom logger from WithLogger")
 	})
 }