From ec29dc0e228fde3d06abd8a79aa55df30625602b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= Date: Wed, 15 Jul 2026 17:16:53 +0800 Subject: [PATCH] fix(logging): make file logger close terminal --- internal/app/root.go | 16 +++++++--------- internal/app/test_home_test.go | 25 +++++++++++++++++++++++-- internal/logging/logger.go | 14 ++++++++++++++ internal/logging/logger_test.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/internal/app/root.go b/internal/app/root.go index 46954b5e..2e4ed661 100644 --- a/internal/app/root.go +++ b/internal/app/root.go @@ -748,20 +748,18 @@ func configureLogLevel(flags *GlobalFlags) { // Initialize file logger — writes to ~/.dws/logs/dws.log at DEBUG level // regardless of stderr level. All slog calls are captured for diagnostics. - logger := replaceFileLogger(defaultConfigDir()) + logger := logging.Setup(defaultConfigDir()) fileHandler := slog.NewJSONHandler(logger.Writer(), &slog.HandlerOptions{Level: slog.LevelDebug}) + defaultLogger := slog.New(logging.NewMultiHandler(stderrHandler, fileHandler)) - slog.SetDefault(slog.New(logging.NewMultiHandler(stderrHandler, fileHandler))) -} - -func replaceFileLogger(configDir string) *logging.FileLogger { fileLoggerMu.Lock() defer fileLoggerMu.Unlock() - if fileLogger != nil { - _ = fileLogger.Close() + previous := fileLogger + fileLogger = logger + slog.SetDefault(defaultLogger) + if previous != nil { + _ = previous.Close() } - fileLogger = logging.Setup(configDir) - return fileLogger } // FileLoggerInstance returns the package-level file logger, or nil if not initialized. diff --git a/internal/app/test_home_test.go b/internal/app/test_home_test.go index 119e4e67..5a61785e 100644 --- a/internal/app/test_home_test.go +++ b/internal/app/test_home_test.go @@ -15,6 +15,7 @@ package app import ( "os" + "path/filepath" "runtime" "testing" ) @@ -35,10 +36,30 @@ func TestConfigureLogLevelReleasesPreviousFileLogger(t *testing.T) { t.Setenv("DWS_CONFIG_DIR", firstConfig) configureLogLevel(&GlobalFlags{}) + firstLogger := FileLoggerInstance() t.Setenv("DWS_CONFIG_DIR", secondConfig) configureLogLevel(&GlobalFlags{}) - if err := os.RemoveAll(firstConfig); err != nil { - t.Fatalf("remove previous logger directory: %v", err) + firstLogPath := filepath.Join(firstConfig, "logs", "dws.log") + if err := os.Remove(firstLogPath); err != nil { + t.Fatalf("remove previous logger file: %v", err) + } + firstLogger.Info("late write to replaced logger") + if _, err := os.Stat(firstLogPath); !os.IsNotExist(err) { + t.Fatalf("replaced logger recreated its log file: %v", err) + } + + previousSameDirLogger := FileLoggerInstance() + configureLogLevel(&GlobalFlags{}) + currentLogger := FileLoggerInstance() + CloseFileLogger() + secondLogPath := filepath.Join(secondConfig, "logs", "dws.log") + if err := os.Remove(secondLogPath); err != nil { + t.Fatalf("remove current logger file: %v", err) + } + previousSameDirLogger.Info("late write to same-directory replaced logger") + currentLogger.Info("late write to closed current logger") + if _, err := os.Stat(secondLogPath); !os.IsNotExist(err) { + t.Fatalf("closed logger recreated same-directory log file: %v", err) } } diff --git a/internal/logging/logger.go b/internal/logging/logger.go index eef07b25..ec82f7b4 100644 --- a/internal/logging/logger.go +++ b/internal/logging/logger.go @@ -120,6 +120,7 @@ type rotatingWriter struct { mu sync.Mutex file *os.File written int64 + closed bool } func newRotatingWriter(path string, maxSize int64, maxBackups int) *rotatingWriter { @@ -133,6 +134,9 @@ func newRotatingWriter(path string, maxSize int64, maxBackups int) *rotatingWrit func (w *rotatingWriter) open() error { w.mu.Lock() defer w.mu.Unlock() + if w.closed { + return os.ErrClosed + } f, err := os.OpenFile(w.path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, config.FilePerm) if err != nil { @@ -151,6 +155,9 @@ func (w *rotatingWriter) open() error { func (w *rotatingWriter) Write(p []byte) (int, error) { w.mu.Lock() defer w.mu.Unlock() + if w.closed { + return 0, os.ErrClosed + } // Attempt to recover if file was lost (e.g. rotation failed due to full disk). if w.file == nil { @@ -175,6 +182,9 @@ func (w *rotatingWriter) Write(p []byte) (int, error) { // reopenLocked tries to reopen the log file. Caller must hold w.mu. func (w *rotatingWriter) reopenLocked() error { + if w.closed { + return os.ErrClosed + } f, err := os.OpenFile(w.path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, config.FilePerm) if err != nil { return err @@ -197,6 +207,10 @@ func (w *rotatingWriter) close() error { w.mu.Lock() defer w.mu.Unlock() + if w.closed { + return nil + } + w.closed = true if w.file == nil { return nil } diff --git a/internal/logging/logger_test.go b/internal/logging/logger_test.go index 18e05489..326061e4 100644 --- a/internal/logging/logger_test.go +++ b/internal/logging/logger_test.go @@ -14,6 +14,7 @@ package logging import ( + "errors" "os" "path/filepath" "strings" @@ -121,3 +122,30 @@ func TestCloseOnNilLogger(t *testing.T) { t.Errorf("Close on nil should not error: %v", err) } } + +func TestFileLoggerCloseIsTerminal(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + fl := Setup(dir) + logPath := filepath.Join(dir, logSubDir, logFileName) + fl.Logger.Info("before close") + if err := fl.Close(); err != nil { + t.Fatalf("close logger: %v", err) + } + if err := os.Remove(logPath); err != nil { + t.Fatalf("remove closed log file: %v", err) + } + + n, err := fl.Writer().Write([]byte("late write")) + if n != 0 || !errors.Is(err, os.ErrClosed) { + t.Fatalf("write after close = (%d, %v), want (0, os.ErrClosed)", n, err) + } + fl.Logger.Info("late structured write") + if _, err := os.Stat(logPath); !os.IsNotExist(err) { + t.Fatalf("closed logger recreated log file: %v", err) + } + if err := fl.Close(); err != nil { + t.Fatalf("second close: %v", err) + } +}