Skip to content

Fix handling of empty config file#595

Merged
optik-aper merged 4 commits into
vultr:masterfrom
optik-aper:config-file-fixes
May 5, 2026
Merged

Fix handling of empty config file#595
optik-aper merged 4 commits into
vultr:masterfrom
optik-aper:config-file-fixes

Conversation

@optik-aper

@optik-aper optik-aper commented May 5, 2026

Copy link
Copy Markdown
Contributor

Description

Better handles missing or empty yaml config file (vultr-cli.yaml) which should be more consistent across platforms

Related Issues

Checklist:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you linted your code locally prior to submission?
  • Have you successfully ran tests with your changes locally?

The master branch of viper has a fix so that the ReadInConfig function
emits the ConfigFileNotFoundError but that currently isn't released, so
this just does a string compare.  In the future, the type should be
used.
@optik-aper optik-aper self-assigned this May 5, 2026
@optik-aper optik-aper added the bug Something isn't working label May 5, 2026
@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Fragile Error Check

Using strings.Contains(err.Error(), "no such file or directory") to detect a missing config file is fragile and platform-dependent. On Windows, the OS error message differs (e.g., "The system cannot find the file specified"), so the suppression won't work and the error message will still be printed for legitimately missing config files. A more robust approach would be to use os.IsNotExist(err) or check for viper.ConfigFileNotFoundError (e.g., var cnf viper.ConfigFileNotFoundError; errors.As(err, &cnf)).

if !strings.Contains(err.Error(), "no such file or directory") {
	fmt.Printf("Error reading in config file (%s) : %v", viper.ConfigFileUsed(), err)
}

@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use proper OS error checking for missing files

Checking for a specific string in the error message is fragile and
platform-dependent (e.g., it won't match Windows error messages or non-English
locales). Use os.IsNotExist(err) to properly and safely check if the error indicates
a missing file.

cmd/root.go [136-138]

-if !strings.Contains(err.Error(), "no such file or directory") {
+if !os.IsNotExist(err) {
     fmt.Printf("Error reading in config file (%s) : %v", viper.ConfigFileUsed(), err)
 }
Suggestion importance[1-10]: 8

__

Why: Checking err.Error() for a specific string is fragile and platform-dependent. Using os.IsNotExist(err) correctly handles missing file errors across different operating systems and locales, fixing a potential cross-platform bug.

Medium

@optik-aper optik-aper merged commit 6959db7 into vultr:master May 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant