ReadConfig(in io.Reader) does not Actually ReadConfig

dp1140a commented 3 months ago

Preflight Checklist

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

Viper Version

1.18.2

Go Version

1.22

Config Source

Files

Format

TOML

Repl.it link

No response

Code reproducing the issue

/**
testdata/good_config.toml is a good toml file.
Viper sees it initially
**/
f, err := os.Open("testdata/good_config.toml")
if err != nil {
   assert.Fail(suite.T(), err.Error())
}
err = viper.ReadConfig(f)
if err != nil {
  assert.Fail(suite.T(), err.Error())
}

Expected Behavior

should read the config. When I print the config I expect the contents of the initial file. but I get {}

Actual Behavior

Return an empty config

Steps To Reproduce

No response

Additional Information

Did some initial debugging on this.

shot1 At this point all is well.

shot2 Here is where things start to hinky

shot3 Still dont have a file tto read? Ok says Viper Ill try to find the config file . . . But wait!

shot4 Here you can see that configPath has never been set. It still has not set the configFile or pconfigPath despite asking for a file reader that has the name and path of the file. The reult is that it cant find the config file and fails.

BUT!!! This code works:

file := "testdata/good_config.toml"
viper.SetConfigName(strings.TrimSuffix(filepath.Base(file), filepath.Ext(file)))
viper.SetConfigType("toml")
viper.AddConfigPath(filepath.Dir(file))
f, err := os.Open(file)
if err != nil {
   assert.Fail(suite.T(), err.Error())
}
err = viper.ReadConfig(f)
if err != nil {
  assert.Fail(suite.T(), err.Error())
}

So why would I go through all that trouble to useReadConfig(f) when I could just go through the same trouble and use ReadInConfig()?

Seems like ReadConfig(f) should just ReadConfig from f

github-actions[bot] commented 3 months ago

👋 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! ❤️

mazenharake commented 3 months ago

This seems to come up as a consequence of not specifying the config type. If you read in a file it still needs to be interpreted in some way and since the config type has not been set then getConfigType() resorts to trying to find the file instead and inferring the config type from the file extension. This is (probably) why your working example is successful.

See https://github.com/spf13/viper/blob/master/viper.go#L2218

I'll admit it is quirky behavior... perhaps adding a default case here https://github.com/spf13/viper/blob/master/viper.go#L1804 and retur an error would be more suitable since the function assumes that a config type has been set either explicitly using SetConfigType or by using SetConfigFile()+ReadInConfig()

Either way, using SetConfigType should resolve this issue?

sagikazarmark commented 3 months ago

I believe @mazenharake is correct.

sagikazarmark commented 1 month ago

Added a check that returns an error when no config type is set. In the future it might make more sense to introduce an option to specify the type.

spf13 / viper