refactor: refactor config loading and strengthen error handling in tests

- Refactor config loading logic to avoid code duplication and improve error handling
- Return detailed error messages when config file reading or parsing fails
- Return nil for config on errors rather than partially initialized objects
- Add tests for missing config files, invalid YAML content, and default config loading failures
- Improve coverage for error scenarios and error messages in config tests

Signed-off-by: appleboy <[email protected]>

Author: appleboy

config/config.go

@@ -318,8 +318,6 @@ func setDefault() {
 
 // LoadConf load config from file and read in environment variables that match
 func LoadConf(confPath ...string) (*ConfYaml, error) {
-	conf := &ConfYaml{}
-
 	// load default values
 	setDefault()
 
@@ -328,31 +326,45 @@ func LoadConf(confPath ...string) (*ConfYaml, error) {
 	viper.SetEnvPrefix("gorush") // will be uppercased automatically
 	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
 
+	// If config path is provided, load from file
 	if len(confPath) > 0 && confPath[0] != "" {
 		content, err := os.ReadFile(confPath[0])
 		if err != nil {
-			return conf, err
+			return nil, fmt.Errorf("failed to read config file %s: %w", confPath[0], err)
 		}
 
 		if err := viper.ReadConfig(bytes.NewBuffer(content)); err != nil {
-			return conf, err
-		}
-	} else {
-		// Search config in home directory with name ".gorush" (without extension).
-		viper.AddConfigPath("/etc/gorush/")
-		viper.AddConfigPath("$HOME/.gorush")
-		viper.AddConfigPath(".")
-		viper.SetConfigName("config")
-
-		// If a config file is found, read it in.
-		if err := viper.ReadInConfig(); err == nil {
-			fmt.Println("Using config file:", viper.ConfigFileUsed())
-		} else if err := viper.ReadConfig(bytes.NewBuffer(defaultConf)); err != nil {
-			// load default config
-			return conf, err
+			return nil, fmt.Errorf("failed to parse config file %s: %w", confPath[0], err)
 		}
+
+		// Early return after successfully loading config from file
+		return loadConfigFromViper()
+	}
+
+	// Search config in home directory with name ".gorush" (without extension).
+	viper.AddConfigPath("/etc/gorush/")
+	viper.AddConfigPath("$HOME/.gorush")
+	viper.AddConfigPath(".")
+	viper.SetConfigName("config")
+
+	// If a config file is found, read it in.
+	if err := viper.ReadInConfig(); err == nil {
+		fmt.Println("Using config file:", viper.ConfigFileUsed())
+		return loadConfigFromViper()
 	}
 
+	// Try to load default config as fallback
+	if err := viper.ReadConfig(bytes.NewBuffer(defaultConf)); err != nil {
+		return nil, fmt.Errorf("failed to load default config and no config file found: %w", err)
+	}
+
+	return loadConfigFromViper()
+}
+
+// loadConfigFromViper extracts config loading logic to avoid code duplication
+func loadConfigFromViper() (*ConfYaml, error) {
+	conf := &ConfYaml{}
+
 	// Core
 	conf.Core.Address = viper.GetString("core.address")
 	conf.Core.Port = viper.GetString("core.port")

config/config_test.go

@@ -12,10 +12,53 @@ import (
 
 // Test file is missing
 func TestMissingFile(t *testing.T) {
-	filename := "test"
-	_, err := LoadConf(filename)
+	filename := "nonexistent_file.yml"
+	conf, err := LoadConf(filename)
 
+	assert.Nil(t, conf)
 	assert.NotNil(t, err)
+	// Check that the error message includes the filename and describes the issue
+	assert.Contains(t, err.Error(), "failed to read config file")
+	assert.Contains(t, err.Error(), filename)
+}
+
+// Test invalid YAML content
+func TestInvalidYAMLFile(t *testing.T) {
+	// Create a temporary file with invalid YAML
+	tmpFile := "test_invalid.yml"
+	content := []byte("invalid: yaml: content: [unclosed")
+
+	// Write invalid content to a temporary file
+	err := os.WriteFile(tmpFile, content, 0o644)
+	assert.NoError(t, err)
+	defer os.Remove(tmpFile) // Clean up
+
+	conf, err := LoadConf(tmpFile)
+
+	assert.Nil(t, conf)
+	assert.NotNil(t, err)
+	// Check that the error message includes the filename and describes parsing failure
+	assert.Contains(t, err.Error(), "failed to parse config file")
+	assert.Contains(t, err.Error(), tmpFile)
+}
+
+// Test improved error messages for default config loading
+func TestDefaultConfigLoadFailure(t *testing.T) {
+	// Backup original defaultConf
+	originalDefaultConf := defaultConf
+	defer func() {
+		defaultConf = originalDefaultConf
+	}()
+
+	// Set invalid default config
+	defaultConf = []byte(`invalid: yaml: [unclosed`)
+
+	conf, err := LoadConf()
+
+	assert.Nil(t, conf)
+	assert.NotNil(t, err)
+	// Check that the error message describes the default config loading failure
+	assert.Contains(t, err.Error(), "failed to load default config and no config file found")
 }
 
 func TestEmptyConfig(t *testing.T) {