From b1b8f814c63902e3f0a7a7f126194b32728e45a8 Mon Sep 17 00:00:00 2001 From: deefdragon Date: Fri, 21 Nov 2025 14:35:12 -0600 Subject: [PATCH 1/3] Refactor WatchConfig to panic inline instead of os.Exit() in a goroutine on init fail Fixes #2095 I Chose to also add panics to the errors for the getConfigFile and watcher.Add as either of those failing was also returning, tho not exiting. This increases the consistency of this area of teh code. I am unsure if it would be better to just remove the panics and rely on the user not getting updates as expected. Unless there is some manner for listening to errors that occur in viper (an error channel etc) I think this is the best bet. --- viper.go | 114 +++++++++++++++++++++++++------------------------------ 1 file changed, 52 insertions(+), 62 deletions(-) diff --git a/viper.go b/viper.go index 2d5158cbd..3a40252d6 100644 --- a/viper.go +++ b/viper.go @@ -33,7 +33,6 @@ import ( "slices" "strconv" "strings" - "sync" "time" "github.com/fsnotify/fsnotify" @@ -280,76 +279,67 @@ func WatchConfig() { v.WatchConfig() } // WatchConfig starts watching a config file for changes. func (v *Viper) WatchConfig() { - initWG := sync.WaitGroup{} - initWG.Add(1) + + watcher, err := fsnotify.NewWatcher() + if err != nil { + v.logger.Error(fmt.Sprintf("failed to create watcher: %s", err)) + panic(err) + } + // we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way + filename, err := v.getConfigFile() + if err != nil { + v.logger.Error(fmt.Sprintf("get config file: %s", err)) + watcher.Close() + panic(err) + } + + configFile := filepath.Clean(filename) + configDir, _ := filepath.Split(configFile) + realConfigFile, _ := filepath.EvalSymlinks(filename) + + err = watcher.Add(configDir) + if err != nil { + v.logger.Error(fmt.Sprintf("failed to add watcher: %s", err)) + watcher.Close() + panic(err) + } + go func() { - watcher, err := fsnotify.NewWatcher() - if err != nil { - v.logger.Error(fmt.Sprintf("failed to create watcher: %s", err)) - os.Exit(1) - } defer watcher.Close() - // we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way - filename, err := v.getConfigFile() - if err != nil { - v.logger.Error(fmt.Sprintf("get config file: %s", err)) - initWG.Done() - return - } - - configFile := filepath.Clean(filename) - configDir, _ := filepath.Split(configFile) - realConfigFile, _ := filepath.EvalSymlinks(filename) - - eventsWG := sync.WaitGroup{} - eventsWG.Add(1) - go func() { - for { - select { - case event, ok := <-watcher.Events: - if !ok { // 'Events' channel is closed - eventsWG.Done() - return + for { + select { + case event, ok := <-watcher.Events: + if !ok { // 'Events' channel is closed + return + } + currentConfigFile, _ := filepath.EvalSymlinks(filename) + // we only care about the config file with the following cases: + // 1 - if the config file was modified or created + // 2 - if the real path to the config file changed (eg: k8s ConfigMap replacement) + if (filepath.Clean(event.Name) == configFile && + (event.Has(fsnotify.Write) || event.Has(fsnotify.Create))) || + (currentConfigFile != "" && currentConfigFile != realConfigFile) { + realConfigFile = currentConfigFile + err := v.ReadInConfig() + if err != nil { + v.logger.Error(fmt.Sprintf("read config file: %s", err)) } - currentConfigFile, _ := filepath.EvalSymlinks(filename) - // we only care about the config file with the following cases: - // 1 - if the config file was modified or created - // 2 - if the real path to the config file changed (eg: k8s ConfigMap replacement) - if (filepath.Clean(event.Name) == configFile && - (event.Has(fsnotify.Write) || event.Has(fsnotify.Create))) || - (currentConfigFile != "" && currentConfigFile != realConfigFile) { - realConfigFile = currentConfigFile - err := v.ReadInConfig() - if err != nil { - v.logger.Error(fmt.Sprintf("read config file: %s", err)) - } - if v.onConfigChange != nil { - v.onConfigChange(event) - } - } else if filepath.Clean(event.Name) == configFile && event.Has(fsnotify.Remove) { - eventsWG.Done() - return + if v.onConfigChange != nil { + v.onConfigChange(event) } - - case err, ok := <-watcher.Errors: - if ok { // 'Errors' channel is not closed - v.logger.Error(fmt.Sprintf("watcher error: %s", err)) - } - eventsWG.Done() + } else if filepath.Clean(event.Name) == configFile && event.Has(fsnotify.Remove) { return } + + case err, ok := <-watcher.Errors: + if ok { // 'Errors' channel is not closed + v.logger.Error(fmt.Sprintf("watcher error: %s", err)) + } + return } - }() - err = watcher.Add(configDir) - if err != nil { - v.logger.Error(fmt.Sprintf("failed to add watcher: %s", err)) - initWG.Done() - return } - initWG.Done() // done initializing the watch in this go routine, so the parent routine can move on... - eventsWG.Wait() // now, wait for event loop to end in this go-routine... }() - initWG.Wait() // make sure that the go routine above fully ended before returning + } // SetConfigFile explicitly defines the path, name and extension of the config file. From ed1917e53ac6a2a68bc3af37773f2f6be0d2125d Mon Sep 17 00:00:00 2001 From: deefdragon Date: Sat, 22 Nov 2025 04:26:29 -0600 Subject: [PATCH 2/3] Make errors consistent between the logged error and the error in the panic --- viper.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/viper.go b/viper.go index 3a40252d6..41cec9644 100644 --- a/viper.go +++ b/viper.go @@ -282,13 +282,15 @@ func (v *Viper) WatchConfig() { watcher, err := fsnotify.NewWatcher() if err != nil { - v.logger.Error(fmt.Sprintf("failed to create watcher: %s", err)) + err = fmt.Errorf("failed to create watcher: %s", err) + v.logger.Error(err.Error()) panic(err) } // we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way filename, err := v.getConfigFile() if err != nil { - v.logger.Error(fmt.Sprintf("get config file: %s", err)) + err = fmt.Errorf("failed to get config file: %s", err) + v.logger.Error(err.Error()) watcher.Close() panic(err) } @@ -299,7 +301,8 @@ func (v *Viper) WatchConfig() { err = watcher.Add(configDir) if err != nil { - v.logger.Error(fmt.Sprintf("failed to add watcher: %s", err)) + err = fmt.Errorf("failed to add config dir to watcher: %s", err) + v.logger.Error(err.Error()) watcher.Close() panic(err) } From f86aae5c5eec6109fc53d6b4059a0955e9eea279 Mon Sep 17 00:00:00 2001 From: deefdragon Date: Sat, 22 Nov 2025 05:08:53 -0600 Subject: [PATCH 3/3] Update comment for WatchConfig to denote it may panic --- viper.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/viper.go b/viper.go index 41cec9644..e47003142 100644 --- a/viper.go +++ b/viper.go @@ -275,9 +275,11 @@ func (v *Viper) OnConfigChange(run func(in fsnotify.Event)) { } // WatchConfig starts watching a config file for changes. +// If there is an error watching the config file, WatchConfig will panic. func WatchConfig() { v.WatchConfig() } // WatchConfig starts watching a config file for changes. +// If there is an error watching the config file, WatchConfig will panic. func (v *Viper) WatchConfig() { watcher, err := fsnotify.NewWatcher()