ConfigFileNotFoundError is not returned when SetConfigFile is called and there is no config file

[jamestiotio]

Preflight Checklist

• I have searched the issue tracker for an issue that matches the one I want to file, without success.
• I am not looking for support or already pursued the available support channels without success.
• I have checked the troubleshooting guide for my problem, without success.

Viper Version

1.14.0

Go Version

1.19.2

Config Source

Files

Format

YAML

Repl.it link

https://replit.com/@jamestiotio/ViperConfigFileNotFoundError

Code reproducing the issue

package main

import (
	"fmt"
	"os"
	"reflect"

	"github.com/spf13/viper"
)

type Configuration struct {
	Text string `mapstructure:"text"`
}

func main() {
	var config Configuration

	viper.SetConfigFile("./config.yml")

	if err := viper.ReadInConfig(); err != nil {
		fmt.Printf("Error reading config file: %s\n", err)
		fmt.Printf("Error type: %s\n", reflect.TypeOf(err))
		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
			fmt.Println("If config file does not exist, this should be printed.")
		} else {
			fmt.Println("This is printed instead.")
		}
		os.Exit(1)
	}

	if err := viper.Unmarshal(&config); err != nil {
		fmt.Printf("Error decoding config file: %s\n", err)
		os.Exit(1)
	}

	fmt.Println(config.Text)
}

Expected Behavior

The program should print:

If config file does not exist, this should be printed.

Actual Behavior

The program prints:

This is printed instead.

Steps To Reproduce

Set up a directory similar to the one shown on the Repl above and then compile and run the program.

Additional Information

On the main README file of Viper, there is an example to handle the specific case where no config file is found. The example provides the following code snippet:

if err := viper.ReadInConfig(); err != nil {
	if _, ok := err.(viper.ConfigFileNotFoundError); ok {
		// Config file not found; ignore error if desired
	} else {
		// Config file was found but another error was produced
	}
}

// Config file found and successfully parsed

However, it seems that the example is wrong if SetConfigFile is used.

In the Repl provided above, the config file is named not_config.yml. In the source code, we point Viper to a non-existent file: viper.SetConfigFile("./config.yml"). Hence, Viper should not be able to find a config file.

If the config file does not exist, the program should print If config file does not exist, this should be printed.. Instead, the program prints This is printed instead., which is in the other branch. Using reflect.TypeOf(err) shows that the error type is *fs.PathError (which is not the same as viper.ConfigFileNotFoundError).

Renaming the not_config.yml file to config.yml will make the program print Hello World!, which is the text string in the config file itself, indicating that the normal happy path is working properly. However, the error path does not seem to work properly.

If this is expected behaviour, then I would suggest renaming the error name to another, more appropriate name (instead of ConfigFileNotFoundError) and updating the README file accordingly to indicate when exactly the error type is returned. As shown in the Repl, this case satisfies the condition of a non-existent config file, and yet ConfigFileNotFoundError is not returned.

[github-actions]

👋 Thanks for reporting!

A maintainer will take a look at your issue shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news,
either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

[chaosreload]

Also face this problem in go1.18.10;
I printed the '' and 'ok' in the statement ', ok := err.(viper.ConfigFileNotFoundError)' :

v1, v2 := err.(viper.ConfigFileNotFoundError)
log.Printf("error: %v, %v\n", v1, v2)

and got this:

2023/05/06 17:31:47 error: Config File "" Not Found in "", false

It seems that viper.ConfigFileNotFoundError doesn't process the name and location correctly with viper.SetConfigFile().

[sagikazarmark]

This is the expected behavior. ConfigFileNotFoundError is returned when a search for a config file yields no results.

When SetConfigFile is used, there is no config file search happening and it returns the file reading error directly.

Semantically, it would probably not be correct to return ConfigFileNotFoundError for both issues. I understand, however, that this is confusing, so I'm open to suggestions.

One potential solution I can imagine is creating a separate error and making both errors implement a common interface, so it's possible to catch both errors in one.

Unfortunately, the solution proposed in #1540 is not quite correct either. Not found error is not the only one afero may yield when reading a file.

[jamestiotio]

Hi @sagikazarmark,

Thank you for your response.

This is the expected behavior. ConfigFileNotFoundError is returned when a search for a config file yields no results.

When SetConfigFile is used, there is no config file search happening and it returns the file reading error directly.

One change that we could implement would be to modify the ReadInConfig function to first check if the file exists or not and raise ConfigFileNotFoundError if the file does not exist. However, if you feel that we should not add this check since it is not present by design or since it introduces a backward-incompatible change, then we should inform the user about this.

Semantically, it would probably not be correct to return ConfigFileNotFoundError for both issues. I understand, however, that this is confusing, so I'm open to suggestions.

One potential solution I can imagine is creating a separate error and making both errors implement a common interface, so it's possible to catch both errors in one.

Sounds good, this solution seems acceptable. In addition to that, we should also update the documentation and the README about this. This way, this matter can be resolved smoothly and we can avoid future users from raising this same issue or asking the same question.

I can raise a PR to add these changes if you are okay with my proposed suggestions.

[sagikazarmark]

@jamestiotio

Sounds good, this solution seems acceptable.

Excellent. Happy to accept PRs for this.

In addition to that, we should also update the documentation and the README about this.

PRs welcome as well.

One change that we could implement would be to modify the ReadInConfig function to first check if the file exists or not and raise ConfigFileNotFoundError if the file does not exist.

I'm a bit reluctant to go down this path because this way "file does not exist" errors would be indistinguishable from "couldn't figure out what file to load" errors.

(An additional piece of information: I'm writing a file search library using io/fs that would replace the current file searching logic and Viper will eventually wrap the error emitted by that library to keep backwards compatibility)