From 84582bbb887bd8df6436064cd3b69d244c996e15 Mon Sep 17 00:00:00 2001
From: Martin Habovstiak <martin.habovstiak@gmail.com>
Date: Wed, 4 Aug 2021 11:11:44 +0200
Subject: [PATCH] Improve error handling of config file

This makes two changes to handling of errors when configuration file
could not be loaded:

1. Only NotFound errors are considered OK - access errors and other FS
   issues are now treated as fatal.
2. Failing to load config file specified explicitly via `--configfile`
   option is alway a fatal error.

Rationale: If the configfile was specified explicitly then it indicates
the user really wishes to load it. While the user could want it to be
optionally loaded for extra configuration options, this can be
accomplished using an empty file.

If the config file was not specified explicitly then its' path was
computed from loop directory. If the file is inaccessible due to
permissions or other FS errors it's nearly certain other following
operations will fail as well. Failing early with a clear message is thus
beneficial.

This still leaves room for uncaught user error (e.g. mistakenly naming
config file inside loop dir as `loop.conf` instead of `loopd.conf`) but
it's greatly reduced and such error should be easier to identify.

(Indirectly) closes #412
---
 loopd/run.go     | 21 ++++++++++++---------
 release_notes.md |  8 ++++++++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/loopd/run.go b/loopd/run.go
index 00bafa3..7e0cee3 100644
--- a/loopd/run.go
+++ b/loopd/run.go
@@ -149,13 +149,16 @@ func Run(rpcCfg RPCConfig) error {
 
 	// Parse ini file.
 	loopDir := lncfg.CleanAndExpandPath(config.LoopDir)
-	configFile := getConfigPath(config, loopDir)
+	configFile, explicitConfig := getConfigPath(config, loopDir)
 
 	if err := flags.IniParse(configFile, &config); err != nil {
-		// If it's a parsing related error, then we'll return
-		// immediately, otherwise we can proceed as possibly the config
-		// file doesn't exist which is OK.
-		if _, ok := err.(*flags.IniError); ok {
+		// File not existing is OK as long as it wasn't specified
+		// explicitly.  All other errors (parsing, EACCESS...) indicate
+		// misconfiguration and need to be reported. In case of
+		// non-not-found FS errors there's high likelihood that other
+		// operations in data directory would also fail so we treat it
+		// as early detection of a problem.
+		if explicitConfig || !os.IsNotExist(err) {
 			return err
 		}
 	}
@@ -248,22 +251,22 @@ func Run(rpcCfg RPCConfig) error {
 
 // getConfigPath gets our config path based on the values that are set in our
 // config.
-func getConfigPath(cfg Config, loopDir string) string {
+func getConfigPath(cfg Config, loopDir string) (string, bool) {
 	// If the config file path provided by the user is set, then we just
 	// use this value.
 	if cfg.ConfigFile != defaultConfigFile {
-		return lncfg.CleanAndExpandPath(cfg.ConfigFile)
+		return lncfg.CleanAndExpandPath(cfg.ConfigFile), true
 	}
 
 	// If the user has set a loop directory that is different to the default
 	// we will use this loop directory as the location of our config file.
 	// We do not namespace by network, because this is a custom loop dir.
 	if loopDir != LoopDirBase {
-		return filepath.Join(loopDir, defaultConfigFilename)
+		return filepath.Join(loopDir, defaultConfigFilename), false
 	}
 
 	// Otherwise, we are using our default loop directory, and the user did
 	// not set a config file path. We use our default loop dir, namespaced
 	// by network.
-	return filepath.Join(loopDir, cfg.Network, defaultConfigFilename)
+	return filepath.Join(loopDir, cfg.Network, defaultConfigFilename), false
 }
diff --git a/release_notes.md b/release_notes.md
index c71d135..cc82360 100644
--- a/release_notes.md
+++ b/release_notes.md
@@ -18,6 +18,14 @@ This file tracks release notes for the loop client.
 
 #### Breaking Changes
 
+* Failing to load configuration file specified by `--configfile` for any
+  reason is now hard error. If you've used `--configfile` to mean "optional
+  extra configuration" you will need to create an empty file. This was done in
+  [#413](https://github.com/lightninglabs/loop/pull/413) to improve error
+  reporting and avoid confusion. Similarly, failure to load any configuration
+  file for reason other than NotFound is hard error, though this is not strictly
+  breaking because such scenario would be already broken later.
+
 #### Bug Fixes
 
 #### Maintenance