Do not accidentally nil out errors

offen · Feb 7, 2024 · cb31554 · cb31554
1 parent 749ca85
commit cb31554
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 39 deletions.
diff --git a/cmd/backup/config_provider.go b/cmd/backup/config_provider.go
@@ -38,7 +38,7 @@ func loadConfig(lookup envProxy) (*Config, error) {
 
 	var c = &Config{}
 	if err := envconfig.Process("", c); err != nil {
-		return nil, fmt.Errorf("failed to process configuration values, error: %w", err)
+		return nil, fmt.Errorf("loadConfig: failed to process configuration values: %w", err)
 	}
 
 	return c, nil
@@ -59,22 +59,23 @@ func loadEnvFiles(directory string) ([]*Config, error) {
 
 	var cs = make([]*Config, 0)
 	for _, item := range items {
-		if !item.IsDir() {
-			p := filepath.Join(directory, item.Name())
-			envFile, err := godotenv.Read(p)
-			if err != nil {
-				return nil, fmt.Errorf("loadEnvFiles: error reading config file %s: %w", p, err)
-			}
-			lookup := func(key string) (string, bool) {
-				val, ok := envFile[key]
-				return val, ok
-			}
-			c, err := loadConfig(lookup)
-			if err != nil {
-				return nil, fmt.Errorf("loadEnvFiles: error loading config from file %s: %w", p, err)
-			}
-			cs = append(cs, c)
+		if item.IsDir() {
+			continue
+		}
+		p := filepath.Join(directory, item.Name())
+		envFile, err := godotenv.Read(p)
+		if err != nil {
+			return nil, fmt.Errorf("loadEnvFiles: error reading config file %s: %w", p, err)
+		}
+		lookup := func(key string) (string, bool) {
+			val, ok := envFile[key]
+			return val, ok
+		}
+		c, err := loadConfig(lookup)
+		if err != nil {
+			return nil, fmt.Errorf("loadEnvFiles: error loading config from file %s: %w", p, err)
 		}
+		cs = append(cs, c)
 	}
 
 	return cs, nil

diff --git a/cmd/backup/exec.go b/cmd/backup/exec.go
@@ -188,13 +188,18 @@ func (s *script) withLabeledCommands(step lifecyclePhase, cb func() error) func(
 	if s.cli == nil {
 		return cb
 	}
-	return func() (ret error) {
-		if err := s.runLabeledCommands(fmt.Sprintf("docker-volume-backup.%s-pre", step)); err != nil {
-			return fmt.Errorf("withLabeledCommands: %s: error running pre commands: %w", step, err)
+	return func() (err error) {
+		if err = s.runLabeledCommands(fmt.Sprintf("docker-volume-backup.%s-pre", step)); err != nil {
+			err = fmt.Errorf("withLabeledCommands: %s: error running pre commands: %w", step, err)
+			return
 		}
 		defer func() {
-			ret = s.runLabeledCommands(fmt.Sprintf("docker-volume-backup.%s-post", step))
+			derr := s.runLabeledCommands(fmt.Sprintf("docker-volume-backup.%s-post", step))
+			if err == nil && derr != nil {
+				err = derr
+			}
 		}()
-		return cb()
+		err = cb()
+		return
 	}
 }
diff --git a/cmd/backup/main.go b/cmd/backup/main.go
@@ -35,44 +35,50 @@ func (c *command) must(err error) {
 	}
 }
 
-func runScript(c *Config) (ret error) {
+func runScript(c *Config) (err error) {
 	defer func() {
-		if err := recover(); err != nil {
-			ret = fmt.Errorf("runScript: unexpected panic running script: %v", err)
+		if derr := recover(); derr != nil {
+			err = fmt.Errorf("runScript: unexpected panic running script: %v", err)
 		}
 	}()
 
 	s, err := newScript(c)
 	if err != nil {
-		return fmt.Errorf("runScript: error instantiating script: %w", err)
+		err = fmt.Errorf("runScript: error instantiating script: %w", err)
+		return
 	}
 
-	runErr := func() (ret error) {
+	runErr := func() (err error) {
 		unlock, err := s.lock("/var/lock/dockervolumebackup.lock")
 		if err != nil {
-			return fmt.Errorf("runScript: error acquiring file lock: %w", err)
+			err = fmt.Errorf("runScript: error acquiring file lock: %w", err)
+			return
 		}
 
 		defer func() {
-			err = unlock()
-			if err != nil {
-				ret = fmt.Errorf("runScript: error releasing file lock: %w", err)
+			derr := unlock()
+			if err == nil && derr != nil {
+				err = fmt.Errorf("runScript: error releasing file lock: %w", derr)
 			}
 		}()
 
 		scriptErr := func() error {
-			if err := s.withLabeledCommands(lifecyclePhaseArchive, func() (ret error) {
+			if err := s.withLabeledCommands(lifecyclePhaseArchive, func() (err error) {
 				restartContainersAndServices, err := s.stopContainersAndServices()
 				// The mechanism for restarting containers is not using hooks as it
 				// should happen as soon as possible (i.e. before uploading backups or
 				// similar).
 				defer func() {
-					ret = restartContainersAndServices()
+					derr := restartContainersAndServices()
+					if err == nil {
+						err = derr
+					}
 				}()
 				if err != nil {
-					return err
+					return
 				}
-				return s.createArchive()
+				err = s.createArchive()
+				return
 			})(); err != nil {
 				return err
 			}
@@ -88,6 +94,7 @@ func runScript(c *Config) (ret error) {
 			}
 			return nil
 		}()
+
 		if hookErr := s.runHooks(scriptErr); hookErr != nil {
 			if scriptErr != nil {
 				return fmt.Errorf(
@@ -102,7 +109,7 @@ func runScript(c *Config) (ret error) {
 			)
 		}
 		if scriptErr != nil {
-			return fmt.Errorf("runScript: error running script: %w", err)
+			return fmt.Errorf("runScript: error running script: %w", scriptErr)
 		}
 		return nil
 	}()
@@ -113,7 +120,6 @@ func runScript(c *Config) (ret error) {
 		)
 	}
 	return runErr
-
 }
 
 func (c *command) runInForeground() error {
@@ -186,12 +192,11 @@ func (c *command) runInForeground() error {
 func (c *command) runAsCommand() error {
 	config, err := loadEnvVars()
 	if err != nil {
-		return fmt.Errorf("could not load config from environment variables, error: %w", err)
+		return fmt.Errorf("runAsCommand: error loading env vars: %w", err)
 	}
-
 	err = runScript(config)
 	if err != nil {
-		return fmt.Errorf("unexpected error during backup, error: %w", err)
+		return fmt.Errorf("runAsCommand: error running script: %w", err)
 	}
 
 	return nil