Merge 172aeb2a6f38dbf89463599124afb20df8bcfdc5 into 3867fab4da6ea7142bfc6374c94b34a708e03b4e

cmd/root.go

@@ -2,10 +2,8 @@ package cmd
 
 import (
 	"context"
-	"os"
 	"os/signal"
 	"strings"
-	"syscall"
 	"time"
 
 	"github.com/go-chi/chi/v5/middleware"
@@ -107,12 +105,7 @@ func runNavidrome(ctx context.Context) {
 
 // mainContext returns a context that is cancelled when the process receives a signal to exit.
 func mainContext(ctx context.Context) (context.Context, context.CancelFunc) {
-	return signal.NotifyContext(ctx,
-		os.Interrupt,
-		syscall.SIGHUP,
-		syscall.SIGTERM,
-		syscall.SIGABRT,
-	)
+	return signal.NotifyContext(ctx, shutdownSignals...)
 }
 
 // startServer starts the Navidrome web server, adding all the necessary routers.

cmd/signaller_nounix.go

@@ -4,8 +4,14 @@ package cmd
 
 import (
 	"context"
+	"os"
+	"syscall"
 )
 
+// SIGHUP is kept as a shutdown signal here, as on Windows it is delivered when the console
+// window is closed, and there is no log rotation convention based on it.
+var shutdownSignals = []os.Signal{os.Interrupt, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGABRT}
+
 // Windows and Plan9 don't support SIGUSR1, so we don't need to start a signaler
 func startSignaller(ctx context.Context) func() error {
 	return func() error {

cmd/signaller_unix.go

@@ -9,32 +9,59 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/log"
+	"github.com/navidrome/navidrome/model"
 )
 
 const triggerScanSignal = syscall.SIGUSR1
 
+// shutdownSignals does not include SIGHUP: as expected from a daemon, the server handles it
+// by reopening its log file (see handleSignal), so log rotation tools can use it.
+var shutdownSignals = []os.Signal{os.Interrupt, syscall.SIGTERM, syscall.SIGABRT}
+
 func startSignaller(ctx context.Context) func() error {
 	log.Info(ctx, "Starting signaler")
 	scanner := CreateScanner(ctx)
 
 	return func() error {
 		var sigChan = make(chan os.Signal, 1)
-		signal.Notify(sigChan, triggerScanSignal)
+		signal.Notify(sigChan, triggerScanSignal, syscall.SIGHUP)
+		defer signal.Stop(sigChan)
 
 		for {
 			select {
 			case sig := <-sigChan:
-				log.Info(ctx, "Received signal, triggering a new scan", "signal", sig)
-				start := time.Now()
-				_, err := scanner.ScanAll(ctx, false)
-				if err != nil {
-					log.Error(ctx, "Error scanning", err)
-				}
-				log.Info(ctx, "Triggered scan complete", "elapsed", time.Since(start))
+				handleSignal(ctx, sig, scanner)
 			case <-ctx.Done():
 				return nil
 			}
 		}
 	}
 }
+
+func handleSignal(ctx context.Context, sig os.Signal, scanner model.Scanner) {
+	switch sig {
+	case syscall.SIGHUP:
+		reopenLogFile(ctx, sig)
+	case triggerScanSignal:
+		log.Info(ctx, "Received signal, triggering a new scan", "signal", sig)
+		start := time.Now()
+		_, err := scanner.ScanAll(ctx, false)
+		if err != nil {
+			log.Error(ctx, "Error scanning", err)
+		}
+		log.Info(ctx, "Triggered scan complete", "elapsed", time.Since(start))
+	}
+}
+
+func reopenLogFile(ctx context.Context, sig os.Signal) {
+	if conf.Server.LogFile == "" {
+		log.Debug(ctx, "Received signal, but no log file configured. Ignoring", "signal", sig)
+		return
+	}
+	log.Info(ctx, "Received signal, reopening log file", "signal", sig, "logFile", conf.Server.LogFile)
+	if err := log.SetOutputFile(conf.Server.LogFile); err != nil {
+		log.Error(ctx, "Error reopening log file", "logFile", conf.Server.LogFile, err)
+	}
+}

conf/configuration.go

@@ -389,16 +389,10 @@ func Load(noConfigDump bool) {
 		Server.DbPath = filepath.Join(Server.DataFolder.String(), consts.DefaultDbPath)
 	}
 
-	out := os.Stderr
 	if Server.LogFile != "" {
-		if mkErr := os.MkdirAll(filepath.Dir(Server.LogFile), os.ModePerm); mkErr != nil {
-			logFatal(fmt.Sprintf("Error creating log file directory: %s", mkErr.Error()))
-		}
-		out, err = os.OpenFile(Server.LogFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
-		if err != nil {
+		if err := log.SetOutputFile(Server.LogFile); err != nil {
 			logFatal(fmt.Sprintf("Error opening log file %s: %s", Server.LogFile, err.Error()))
 		}
-		log.SetOutput(out)
 	} else if os.Getenv("ND_SYSTEMD_PRIORITY_LOGGING") != "" && os.Getenv("JOURNAL_STREAM") != "" {
 		// When running under systemd, prepend syslog priority prefixes so
 		// journald assigns the correct severity to each log line.
@@ -461,7 +455,7 @@ func Load(noConfigDump bool) {
 		if Server.EnableLogRedacting {
 			prettyConf = log.Redact(prettyConf)
 		}
-		_, _ = fmt.Fprintln(out, prettyConf)
+		_, _ = fmt.Fprintln(log.Output(), prettyConf)
 	}
 
 	if !Server.EnableExternalServices {

conf/configuration_test.go

@@ -323,7 +323,7 @@ var _ = Describe("Configuration", func() {
 			viper.SetDefault("logfile", filepath.Join(invalidPath, "log.txt"))
 			Expect(func() {
 				conf.Load(true)
-			}).To(PanicWith(ContainSubstring("Error creating log file directory")))
+			}).To(PanicWith(ContainSubstring("creating log file directory")))
 		})
 
 		It("is called when BaseURL is invalid", func() {

log/log.go

@@ -8,6 +8,7 @@ import (
 	"iter"
 	"net/http"
 	"os"
+	"path/filepath"
 	"runtime"
 	"sort"
 	"strings"
@@ -152,6 +153,41 @@ func SetOutput(w io.Writer) {
 	defaultLogger.SetOutput(w)
 }
 
+// Output returns the current writer used by the default logger.
+func Output() io.Writer {
+	loggerMu.RLock()
+	defer loggerMu.RUnlock()
+	return defaultLogger.Out
+}
+
+var (
+	outputFileMu sync.Mutex
+	outputFile   *os.File
+)
+
+// SetOutputFile opens (or creates) the given file in append mode and sets it as the log
+// output, closing the previously opened log file, if any. Calling it again with the same
+// path reopens the file, allowing external log rotation tools to signal the process
+// (e.g. with SIGHUP) after rotating the log file.
+func SetOutputFile(path string) error {
+	if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil {
+		return fmt.Errorf("creating log file directory: %w", err)
+	}
+	outputFileMu.Lock()
+	defer outputFileMu.Unlock()
+	f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+	if err != nil {
+		return err
+	}
+	SetOutput(f)
+	prev := outputFile
+	outputFile = f
+	if prev != nil {
+		_ = prev.Close()
+	}
+	return nil
+}
+
 // EnableJournalFormat wraps the current logger formatter with syslog
 // priority prefixes for systemd-journald. Only call this when output
 // goes to stderr and JOURNAL_STREAM is set.

log/log_test.go

@@ -6,6 +6,8 @@ import (
 	"errors"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -292,4 +294,39 @@ var _ = Describe("Logger", func() {
 			Expect(got).To(ContainSubstring(`"User-Agent":["Finamp/1.0"]`))
 		})
 	})
+
+	Describe("SetOutputFile", func() {
+		var path string
+
+		BeforeEach(func() {
+			path = filepath.Join(GinkgoT().TempDir(), "logs", "navidrome.log")
+			Expect(SetOutputFile(path)).To(Succeed())
+		})
+
+		It("creates the log file directory and writes log messages to the file", func() {
+			Error("first message")
+
+			content, err := os.ReadFile(path)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(content)).To(ContainSubstring("first message"))
+		})
+
+		It("recreates the log file after it has been rotated away", func() {
+			Error("first message")
+			rotated := path + ".1"
+			Expect(os.Rename(path, rotated)).To(Succeed())
+
+			Expect(SetOutputFile(path)).To(Succeed())
+			Error("second message")
+
+			rotatedContent, err := os.ReadFile(rotated)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(rotatedContent)).To(ContainSubstring("first message"))
+			Expect(string(rotatedContent)).ToNot(ContainSubstring("second message"))
+
+			content, err := os.ReadFile(path)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(content)).To(ContainSubstring("second message"))
+		})
+	})
 })