fix: address review comments on error swallowing and spinner handling

Signed-off-by: Enrique Lacal <enrique.lacal@kaleido.io>

Author: EnriqueL8

cmd/init_cardano.go

@@ -49,8 +49,11 @@ var initCardanoCmd = &cobra.Command{
 			return err
 		}
 		if err := stackManager.InitStack(&initOptions); err != nil {
-			// Try to clean up, but don't mask the original error
-			_ = stackManager.RemoveStack()
+			// Try to clean up, but log rather than return any error to not mask the original error
+			if verr := stackManager.RemoveStack(); verr != nil {
+				l := log.LoggerFromContext(ctx)
+				l.Info(fmt.Sprintf("Error whilst removing the stack: %s", verr.Error()))
+			}
 			return err
 		}
 		fmt.Printf("Stack '%s' created!\nTo start your new stack run:\n\n%s start %s\n", initOptions.StackName, rootCmd.Use, initOptions.StackName)

cmd/init_fabric.go

@@ -54,8 +54,11 @@ var initFabricCmd = &cobra.Command{
 			return err
 		}
 		if err := stackManager.InitStack(&initOptions); err != nil {
-			// Try to clean up, but don't mask the original error
-			_ = stackManager.RemoveStack()
+			// Try to clean up, but log rather than return any error to not mask the original error
+			if verr := stackManager.RemoveStack(); verr != nil {
+				l := log.LoggerFromContext(ctx)
+				l.Info(fmt.Sprintf("Error whilst removing the stack: %s", verr.Error()))
+			}
 			return err
 		}
 		fmt.Printf("Stack '%s' created!\nTo start your new stack run:\n\n%s start %s\n", initOptions.StackName, rootCmd.Use, initOptions.StackName)

cmd/init_tezos.go

@@ -50,8 +50,11 @@ var initTezosCmd = &cobra.Command{
 			return err
 		}
 		if err := stackManager.InitStack(&initOptions); err != nil {
-			// Try to clean up, but don't mask the original error
-			_ = stackManager.RemoveStack()
+			// Try to clean up, but log rather than return any error to not mask the original error
+			if verr := stackManager.RemoveStack(); verr != nil {
+				l := log.LoggerFromContext(ctx)
+				l.Info(fmt.Sprintf("Error whilst removing the stack: %s", verr.Error()))
+			}
 			return err
 		}
 		fmt.Printf("Stack '%s' created!\nTo start your new stack run:\n\n%s start %s\n", initOptions.StackName, rootCmd.Use, initOptions.StackName)

cmd/pull.go

@@ -63,18 +63,9 @@ Pull the images for a stack .
 		if err := stackManager.LoadStack(stackName); err != nil {
 			return err
 		}
-		if spin != nil {
-			spin.Start()
-		}
-		err = stackManager.PullStack(&pullOptions)
-		if spin != nil {
-			spin.Stop()
-		}
-		// Throw an error after stopping the spin, this will prevent the user's terminal from having the spinner as overlay
-		if err != nil {
-			return err
-		}
-		return nil
+		return withSpinner(spin, func() error {
+			return stackManager.PullStack(&pullOptions)
+		})
 	},
 }
 

cmd/root.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/briandowns/spinner"
 	"github.com/mattn/go-isatty"
 	"github.com/spf13/cobra"
 
@@ -42,6 +43,16 @@ var logger log.Logger = &log.StdoutLogger{
 // name of the executable, this is for the help messages
 var ExecutableName string = os.Args[0]
 
+// withSpinner runs fn with the spinner (if configured) running, and stops it
+// before returning so an error is not overlaid by the spinner in the user's terminal
+func withSpinner(spin *spinner.Spinner, fn func() error) error {
+	if spin != nil {
+		spin.Start()
+		defer spin.Stop()
+	}
+	return fn()
+}
+
 func GetFireflyASCIIArt() string {
 	s := ""
 	s += "\u001b[33m    _______           ________     \u001b[0m\n"   // yellow

cmd/start.go

@@ -71,14 +71,11 @@ This command will start a stack and run it in the background.
 			fmt.Println("this will take a few seconds longer since this is the first time you're running this stack...")
 		}
 
-		if spin != nil {
-			spin.Start()
-		}
-		messages, err := stackManager.StartStack(&startOptions)
-		if spin != nil {
-			spin.Stop()
-		}
-		if err != nil {
+		var messages []string
+		if err := withSpinner(spin, func() (err error) {
+			messages, err = stackManager.StartStack(&startOptions)
+			return err
+		}); err != nil {
 			return err
 		}
 		fmt.Print("\n\n")