Merge 134ee02d29 into 8a9993eff0

internal/logging/logging.go

@@ -104,18 +104,20 @@ type LogHolder struct {
 	logger atomic.Pointer[slog.Logger]
 }
 
-// Logger returns the logger for the LogHolder. If nil, returns slog.Default().
+// Logger returns the logger for the LogHolder. When no logger has been set,
+// it returns slog.Default() so log records are not silently dropped.
 func (l *LogHolder) Logger() *slog.Logger {
 	if lg := l.logger.Load(); lg != nil {
 		return lg
 	}
-	return slog.New(slog.DiscardHandler) // Should never be reached
+	return slog.Default()
 }
 
-// SetLogger sets the logger for the LogHolder. If nil, sets the default logger.
+// SetLogger sets the logger for the LogHolder. A nil handler clears the logger,
+// so Logger() falls back to slog.Default() again.
 func (l *LogHolder) SetLogger(newHandler slog.Handler) {
 	if newHandler == nil {
-		l.logger.Store(slog.New(slog.DiscardHandler)) // Assume nil as discarding logs
+		l.logger.Store(nil)
 		return
 	}
 	l.logger.Store(slog.New(newHandler))

internal/logging/logging_test.go

@@ -43,14 +43,75 @@ func TestLogHolder_Logger(t *testing.T) {
 		assert.Contains(t, buf.String(), "test message")
 	})
 
-	t.Run("should return discard - defaultlogger when no handler is set", func(t *testing.T) {
+	t.Run("should return the slog default logger when no handler is set", func(t *testing.T) {
 		holder := &LogHolder{}
 		logger := holder.Logger()
 
-		assert.Equal(t, slog.Handler(slog.DiscardHandler), logger.Handler())
+		assert.Equal(t, slog.Default(), logger)
 	})
 }
 
+// TestLogHolder_DefaultHandler covers where a LogHolder sends records for each
+// way of (not) configuring it, against a capture handler installed with
+// slog.SetDefault.
+func TestLogHolder_DefaultHandler(t *testing.T) {
+	tests := []struct {
+		name        string
+		setup       func(holder *LogHolder, ownHandler slog.Handler)
+		wantDefault bool // record lands in the slog default buffer
+		wantOwn     bool // record lands in the handler set on the holder
+	}{
+		{
+			name:        "fresh holder logs through the slog default",
+			setup:       func(_ *LogHolder, _ slog.Handler) {},
+			wantDefault: true,
+		},
+		{
+			name: "handler set on the holder wins over the slog default",
+			setup: func(holder *LogHolder, ownHandler slog.Handler) {
+				holder.SetLogger(ownHandler)
+			},
+			wantOwn: true,
+		},
+		{
+			name: "nil handler falls back to the slog default",
+			setup: func(holder *LogHolder, ownHandler slog.Handler) {
+				holder.SetLogger(ownHandler)
+				holder.SetLogger(nil)
+			},
+			wantDefault: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			defaultBuf := &bytes.Buffer{}
+			previous := slog.Default()
+			slog.SetDefault(slog.New(slog.NewTextHandler(defaultBuf, &slog.HandlerOptions{Level: slog.LevelDebug})))
+			t.Cleanup(func() { slog.SetDefault(previous) })
+
+			ownBuf := &bytes.Buffer{}
+			ownHandler := slog.NewTextHandler(ownBuf, &slog.HandlerOptions{Level: slog.LevelDebug})
+
+			holder := &LogHolder{}
+			tt.setup(holder, ownHandler)
+
+			holder.Logger().Info("test message")
+
+			if tt.wantDefault {
+				assert.Contains(t, defaultBuf.String(), "test message")
+			} else {
+				assert.Empty(t, defaultBuf.String())
+			}
+			if tt.wantOwn {
+				assert.Contains(t, ownBuf.String(), "test message")
+			} else {
+				assert.Empty(t, ownBuf.String())
+			}
+		})
+	}
+}
+
 func TestLogHolder_SetLogger(t *testing.T) {
 	t.Run("sets logger with valid handler", func(t *testing.T) {
 		holder := &LogHolder{}
@@ -66,15 +127,17 @@ func TestLogHolder_SetLogger(t *testing.T) {
 		assert.Equal(t, handler, logger.Handler())
 	})
 
-	t.Run("sets discard logger with nil handler", func(t *testing.T) {
+	t.Run("clears the logger with nil handler", func(t *testing.T) {
 		holder := &LogHolder{}
+		buf := &bytes.Buffer{}
+		holder.SetLogger(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug}))
 
 		holder.SetLogger(nil)
 		logger := holder.Logger()
 
 		assert.NotNil(t, logger)
 
-		assert.Equal(t, slog.Handler(slog.DiscardHandler), logger.Handler())
+		assert.Equal(t, slog.Default(), logger)
 	})
 
 	t.Run("can replace existing logger", func(t *testing.T) {