refactor: extract logFatal helper for config error handling

Replace 14 repeated fmt.Fprintln(os.Stderr, "FATAL:", ...)/os.Exit(1)
patterns with a single logFatal function. This reduces duplication
and makes all fatal config paths testable via SetLogFatal.

Signed-off-by: Deluan <deluan@navidrome.org>

conf/configuration.go

@@ -258,10 +258,10 @@ type searchOptions struct {
 	FullString bool
 }
 
-// fatalFunc is called for fatal config errors. Defaults to printing + os.Exit(1).
+// logFatal prints a fatal error message to stderr and exits.
 // Overridden in tests to allow testing fatal paths.
-var fatalFunc = func(msg string) {
+var logFatal = func(args ...any) {
-	_, _ = fmt.Fprintln(os.Stderr, "FATAL:", msg)
+	_, _ = fmt.Fprintln(os.Stderr, append([]any{"FATAL:"}, args...)...)
 	os.Exit(1)
 }
 
@@ -274,8 +274,7 @@ func LoadFromFile(confFile string) {
 	viper.SetConfigFile(confFile)
 	err := viper.ReadInConfig()
 	if err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error reading config file:", err)
+		logFatal("Error reading config file:", err)
-		os.Exit(1)
 	}
 	Load(true)
 }
@@ -292,14 +291,12 @@ func Load(noConfigDump bool) {
 
 	err := viper.Unmarshal(&Server)
 	if err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error parsing config:", err)
+		logFatal("Error parsing config:", err)
-		os.Exit(1)
 	}
 
 	err = os.MkdirAll(Server.DataFolder, os.ModePerm)
 	if err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating data path:", err)
+		logFatal("Error creating data path:", err)
-		os.Exit(1)
 	}
 
 	if Server.CacheFolder == "" {
@@ -307,14 +304,12 @@ func Load(noConfigDump bool) {
 	}
 	err = os.MkdirAll(Server.CacheFolder, os.ModePerm)
 	if err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating cache path:", err)
+		logFatal("Error creating cache path:", err)
-		os.Exit(1)
 	}
 
 	err = os.MkdirAll(filepath.Join(Server.DataFolder, consts.ArtworkFolder), os.ModePerm)
 	if err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating artwork path:", err)
+		logFatal("Error creating artwork path:", err)
-		os.Exit(1)
 	}
 
 	if Server.Plugins.Enabled {
@@ -323,8 +318,7 @@ func Load(noConfigDump bool) {
 		}
 		err = os.MkdirAll(Server.Plugins.Folder, 0700)
 		if err != nil {
-			_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating plugins path:", err)
+			logFatal("Error creating plugins path:", err)
-			os.Exit(1)
 		}
 	}
 
@@ -336,8 +330,7 @@ func Load(noConfigDump bool) {
 	if Server.Backup.Path != "" {
 		err = os.MkdirAll(Server.Backup.Path, os.ModePerm)
 		if err != nil {
-			_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error creating backup path:", err)
+			logFatal("Error creating backup path:", err)
-			os.Exit(1)
 		}
 	}
 
@@ -345,8 +338,7 @@ func Load(noConfigDump bool) {
 	if Server.LogFile != "" {
 		out, err = os.OpenFile(Server.LogFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
 		if err != nil {
-			_, _ = fmt.Fprintf(os.Stderr, "FATAL: Error opening log file %s: %s\n", Server.LogFile, err.Error())
+			logFatal(fmt.Sprintf("Error opening log file %s: %s", Server.LogFile, err.Error()))
-			os.Exit(1)
 		}
 		log.SetOutput(out)
 	} else if os.Getenv("ND_SYSTEMD_PRIORITY_LOGGING") != "" && os.Getenv("JOURNAL_STREAM") != "" {
@@ -378,8 +370,7 @@ func Load(noConfigDump bool) {
 	if Server.BaseURL != "" {
 		u, err := url.Parse(Server.BaseURL)
 		if err != nil {
-			_, _ = fmt.Fprintln(os.Stderr, "FATAL: Invalid BaseURL:", err)
+			logFatal("Invalid BaseURL:", err)
-			os.Exit(1)
 		}
 		Server.BasePath = u.Path
 		u.Path = ""
@@ -487,7 +478,7 @@ func remapEnvVarKeysFromConfig() {
 		displayCanonical := toPascalCase(canonicalKey)
 
 		if viper.InConfig(canonicalKey) {
-			fatalFunc(fmt.Sprintf(
+			logFatal(fmt.Sprintf(
 				"Config file contains both '%s' and '%s'. Remove the ND_-prefixed version. "+
 					"The 'ND_' prefix is only needed for environment variables, not config file keys.",
 				displayNDKey, displayCanonical,
@@ -520,18 +511,15 @@ func parseIniFileConfiguration() {
 		var iniConfig map[string]any
 		err := viper.Unmarshal(&iniConfig)
 		if err != nil {
-			_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error parsing config:", err)
+			logFatal("Error parsing config:", err)
-			os.Exit(1)
 		}
 		cfg, ok := iniConfig["default"].(map[string]any)
 		if !ok {
-			_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error parsing config: missing [default] section:", iniConfig)
+			logFatal("Error parsing config: missing [default] section:", iniConfig)
-			os.Exit(1)
 		}
 		err = viper.MergeConfigMap(cfg)
 		if err != nil {
-			_, _ = fmt.Fprintln(os.Stderr, "FATAL: Error parsing config:", err)
+			logFatal("Error parsing config:", err)
-			os.Exit(1)
 		}
 	}
 }
@@ -872,8 +860,7 @@ func InitConfig(cfgFile string, loadEnvVars bool) {
 
 	err := viper.ReadInConfig()
 	if viper.ConfigFileUsed() != "" && err != nil {
-		_, _ = fmt.Fprintln(os.Stderr, "FATAL: Navidrome could not open config file: ", err)
+		logFatal("Navidrome could not open config file:", err)
-		os.Exit(1)
 	}
 }
 

conf/configuration_test.go

@@ -2,6 +2,7 @@ package conf_test
 
 import (
 	"fmt"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -24,6 +25,11 @@ var _ = Describe("Configuration", func() {
 		viper.SetDefault("datafolder", GinkgoT().TempDir())
 		viper.SetDefault("loglevel", "error")
 		conf.ResetConf()
+
+		// Panic instead of exiting on fatal errors to allow testing error conditions
+		DeferCleanup(conf.SetLogFatal(func(args ...any) {
+			panic(fmt.Sprint(args...))
+		}))
 	})
 
 	Describe("ParseLanguages", func() {
@@ -139,11 +145,6 @@ var _ = Describe("Configuration", func() {
 		})
 
 		It("exits with fatal error when both ND_ and canonical key exist", func() {
-			cleanup := conf.SetFatalFunc(func(msg string) {
-				panic(msg)
-			})
-			defer cleanup()
-
 			filename := filepath.Join("testdata", "cfg_nd_conflict.toml")
 			conf.InitConfig(filename, false)
 
@@ -164,6 +165,60 @@ var _ = Describe("Configuration", func() {
 		})
 	})
 
+	Describe("logFatal", func() {
+		var invalidPath string
+		BeforeEach(func() {
+			viper.Reset()
+			conf.SetViperDefaults()
+			viper.SetDefault("loglevel", "error")
+			conf.ResetConf()
+
+			// Create a file so that any path under it is invalid on all OSes
+			f, err := os.CreateTemp(GinkgoT().TempDir(), "blocker")
+			Expect(err).ToNot(HaveOccurred())
+			f.Close()
+			invalidPath = filepath.Join(f.Name(), "subdir")
+		})
+
+		It("is called when LoadFromFile gets an invalid config file", func() {
+			Expect(func() {
+				conf.LoadFromFile(filepath.Join(invalidPath, "file.toml"))
+			}).To(PanicWith(ContainSubstring("Error reading config file")))
+		})
+
+		It("is called when DataFolder is not writable", func() {
+			viper.SetDefault("datafolder", invalidPath)
+			Expect(func() {
+				conf.Load(true)
+			}).To(PanicWith(ContainSubstring("Error creating data path")))
+		})
+
+		It("is called when CacheFolder is not writable", func() {
+			viper.SetDefault("datafolder", GinkgoT().TempDir())
+			viper.SetDefault("cachefolder", invalidPath)
+			Expect(func() {
+				conf.Load(true)
+			}).To(PanicWith(ContainSubstring("Error creating cache path")))
+		})
+
+		It("is called when LogFile path is not writable", func() {
+			viper.SetDefault("datafolder", GinkgoT().TempDir())
+			viper.SetDefault("logfile", filepath.Join(invalidPath, "log.txt"))
+			Expect(func() {
+				conf.Load(true)
+			}).To(PanicWith(ContainSubstring("Error opening log file")))
+		})
+
+		It("is called when BaseURL is invalid", func() {
+			viper.SetDefault("datafolder", GinkgoT().TempDir())
+			viper.SetDefault("baseurl", "://invalid")
+			Expect(func() {
+				conf.Load(true)
+			}).To(PanicWith(ContainSubstring("Invalid BaseURL")))
+		})
+
+	})
+
 	DescribeTable("should load configuration from",
 		func(format string) {
 			filename := filepath.Join("testdata", "cfg."+format)

conf/export_test.go

@@ -14,8 +14,8 @@ var NormalizeSearchBackend = normalizeSearchBackend
 
 var ToPascalCase = toPascalCase
 
-func SetFatalFunc(f func(string)) func() {
+func SetLogFatal(f func(...any)) func() {
-	old := fatalFunc
+	old := logFatal
-	fatalFunc = f
+	logFatal = f
-	return func() { fatalFunc = old }
+	return func() { logFatal = old }
 }