Improve error handling and logging

Author: eikendev

internal/commands/down.go

@@ -1,6 +1,7 @@
 package commands
 
 import (
+	"fmt"
 	"log/slog"
 
 	"github.com/eikendev/hackenv/internal/handling"
@@ -14,18 +15,31 @@ type DownCommand struct{}
 
 // Run is the function for the down command.
 func (c *DownCommand) Run(s *options.Options) error {
-	image := images.GetImageDetails(s.Type)
+	image, err := images.GetImageDetails(s.Type)
+	if err != nil {
+		slog.Error("Failed to get image details for down command", "type", s.Type, "err", err)
+		return fmt.Errorf("cannot resolve image details for %q: %w", s.Type, err)
+	}
 
-	conn := libvirt.Connect()
+	conn, err := libvirt.Connect()
+	if err != nil {
+		slog.Error("Failed to connect to libvirt for down command", "err", err)
+		return fmt.Errorf("cannot connect to libvirt: %w", err)
+	}
 	defer handling.CloseConnect(conn)
 
-	dom := libvirt.GetDomain(conn, &image, true)
-	defer handling.FreeDomain(dom)
-
-	err := dom.Destroy()
+	dom, err := libvirt.GetDomain(conn, &image, true)
 	if err != nil {
+		slog.Error("Failed to lookup domain for down command", "image", image.Name, "err", err)
+		return fmt.Errorf("cannot look up domain %q: %w", image.Name, err)
+	}
+	if dom != nil {
+		defer handling.FreeDomain(dom)
+	}
+
+	if err := dom.Destroy(); err != nil {
 		slog.Error("Cannot destroy domain", "err", err)
-		return err
+		return fmt.Errorf("cannot destroy domain %q: %w", image.Name, err)
 	}
 
 	return nil

internal/commands/fix.go

@@ -53,7 +53,8 @@ func execCommand(scripts []string, verbose bool) error {
 			fmt.Println(string(b))
 		}
 		if err != nil {
-			return err
+			slog.Error("Script execution failed", "index", i+1, "total", len(scripts), "err", err)
+			return fmt.Errorf("failed to run script %d/%d: %w", i+1, len(scripts), err)
 		}
 	}
 

internal/commands/get.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"crypto/sha256"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -28,14 +29,14 @@ func calculateFileChecksum(path string) (string, error) {
 	f, err := os.Open(path) //#nosec G304
 	if err != nil {
 		slog.Error("Failed to open file", "path", path, "err", err)
-		return "", err
+		return "", fmt.Errorf("cannot open %s: %w", path, err)
 	}
 	defer handling.Close(f)
 
 	h := sha256.New()
 	if _, err := io.Copy(h, f); err != nil {
 		slog.Error("Failed to copy file content", "path", path, "err", err)
-		return "", err
+		return "", fmt.Errorf("cannot read %s for checksum: %w", path, err)
 	}
 
 	return fmt.Sprintf("%x", h.Sum(nil)), nil
@@ -48,14 +49,14 @@ func downloadImage(path, url string) error {
 	out, err := os.Create(path) //#nosec G304
 	if err != nil {
 		slog.Error("Cannot create image file", "path", path, "err", err)
-		return err
+		return fmt.Errorf("cannot create %s: %w", path, err)
 	}
 	defer handling.Close(out)
 
 	resp, err := http.Get(url) //#nosec G107
 	if err != nil {
 		slog.Error("Cannot download image file", "url", url, "err", err)
-		return err
+		return fmt.Errorf("cannot download %s: %w", url, err)
 	}
 	if resp == nil {
 		return fmt.Errorf("received nil response")
@@ -64,7 +65,7 @@ func downloadImage(path, url string) error {
 
 	if resp.StatusCode != http.StatusOK {
 		slog.Error("Cannot download image file: bad status", "status", resp.Status)
-		return err
+		return fmt.Errorf("cannot download %s: status %s", url, resp.Status)
 	}
 
 	bar := progressbar.DefaultBytes(
@@ -75,7 +76,7 @@ func downloadImage(path, url string) error {
 	_, err = io.Copy(io.MultiWriter(out, bar), resp.Body)
 	if err != nil {
 		slog.Error("Cannot write image file", "path", path, "err", err)
-		return err
+		return fmt.Errorf("cannot write %s: %w", path, err)
 	}
 
 	slog.Info("Download successful")
@@ -85,18 +86,19 @@ func downloadImage(path, url string) error {
 func validateChecksum(localPath, checksum string) error {
 	newChecksum, err := calculateFileChecksum(localPath)
 	if err != nil {
-		return err
+		slog.Error("Failed to calculate checksum for downloaded image", "path", localPath, "err", err)
+		return fmt.Errorf("failed to calculate checksum for %s: %w", localPath, err)
 	}
 
 	if newChecksum != checksum {
-		err := os.Remove(localPath)
-		if err != nil {
-			slog.Error("Downloaded image has bad checksum and cannot be removed", "path", localPath, "expected", checksum, "actual", newChecksum, "err", err)
-			os.Exit(1)
+		removeErr := os.Remove(localPath)
+		if removeErr != nil {
+			slog.Error("Downloaded image has bad checksum and cannot be removed", "path", localPath, "expected", checksum, "actual", newChecksum, "err", removeErr)
+			return fmt.Errorf("failed to remove image with bad checksum (expected %s, actual %s): %w", checksum, newChecksum, removeErr)
 		}
 
 		slog.Error("Downloaded image has bad checksum and was removed", "path", localPath, "expected", checksum, "actual", newChecksum)
-		os.Exit(1)
+		return fmt.Errorf("detected bad checksum for downloaded image: expected %s, actual %s", checksum, newChecksum)
 	}
 
 	slog.Info("Checksum validated successfully")
@@ -105,47 +107,77 @@ func validateChecksum(localPath, checksum string) error {
 
 // Run is the function for the get command.
 func (c *GetCommand) Run(s *options.Options) error {
-	image := images.GetImageDetails(s.Type)
-	info := image.GetDownloadInfo(true)
+	image, err := images.GetImageDetails(s.Type)
+	if err != nil {
+		slog.Error("Failed to get image details for get command", "type", s.Type, "err", err)
+		return fmt.Errorf("cannot resolve image details for %q: %w", s.Type, err)
+	}
+	info, err := image.GetDownloadInfo(true)
+	if err != nil {
+		slog.Error("Failed to get download info for image", "image", image.DisplayName, "err", err)
+		return fmt.Errorf("cannot fetch download info for %s: %w", image.DisplayName, err)
+	}
 	if info == nil {
+		slog.Error("Download info is nil", "image", image.DisplayName)
 		return fmt.Errorf("failed to get download information")
 	}
 
 	slog.Info("Found image to download", "filename", info.Filename, "checksum", info.Checksum)
 
-	localPath := image.GetLocalPath(info.Version)
+	localPath, err := image.GetLocalPath(info.Version)
+	if err != nil {
+		slog.Error("Failed to resolve local image path", "image", image.DisplayName, "version", info.Version, "err", err)
+		return fmt.Errorf("cannot resolve local path for %s %s: %w", image.DisplayName, info.Version, err)
+	}
 
-	// https://stackoverflow.com/a/12518877
-	if _, err := os.Stat(localPath); err == nil {
-		// The image already exists.
+	skipDownload, err := c.shouldSkipDownload(localPath, image, info)
+	if err != nil {
+		slog.Error("Failed to determine if download should be skipped", "path", localPath, "err", err)
+		return fmt.Errorf("cannot decide whether to download %s: %w", localPath, err)
+	}
+	if skipDownload {
+		return nil
+	}
+
+	err = downloadImage(localPath, image.ArchiveURL+"/"+info.Filename)
+	if err != nil {
+		slog.Error("Failed to download image", "path", localPath, "url", image.ArchiveURL+"/"+info.Filename, "err", err)
+		return fmt.Errorf("cannot download image %s/%s: %w", image.ArchiveURL, info.Filename, err)
+	}
 
+	err = validateChecksum(localPath, info.Checksum)
+	if err != nil {
+		slog.Error("Checksum validation failed", "path", localPath, "err", err)
+		return fmt.Errorf("failed to validate checksum for %s: %w", localPath, err)
+	}
+
+	slog.Info("When using SELinux, label the image with the fix command before proceeding")
+
+	return nil
+}
+
+func (c *GetCommand) shouldSkipDownload(localPath string, image images.Image, info *images.DownloadInfo) (bool, error) {
+	// https://stackoverflow.com/a/12518877
+	_, err := os.Stat(localPath)
+	if err == nil {
 		if !c.Update && !c.Force {
 			slog.Info("An image is already installed; use --update to refresh")
-			return nil
+			return true, nil
 		}
 
 		localVersion := image.FileVersion(localPath)
 
 		if !c.Force && image.VersionComparer.Eq(info.Version, localVersion) {
 			slog.Info("Latest image is already installed; use --force to overwrite")
-			return nil
+			return true, nil
 		}
-	} else if !os.IsNotExist(err) {
-		slog.Error("Unable to get file information", "path", localPath, "err", err)
-		os.Exit(1)
+		return false, nil
 	}
 
-	err := downloadImage(localPath, image.ArchiveURL+"/"+info.Filename)
-	if err != nil {
-		return err
+	if errors.Is(err, os.ErrNotExist) {
+		return false, nil
 	}
 
-	err = validateChecksum(localPath, info.Checksum)
-	if err != nil {
-		return err
-	}
-
-	slog.Info("When using SELinux, label the image with the fix command before proceeding")
-
-	return nil
+	slog.Error("Unable to get file information", "path", localPath, "err", err)
+	return false, fmt.Errorf("cannot stat %s: %w", localPath, err)
 }

internal/commands/gui.go

@@ -27,38 +27,31 @@ type GuiCommand struct {
 
 // Run is the function for the gui command.
 func (c *GuiCommand) Run(s *options.Options) error {
-	image := images.GetImageDetails(s.Type)
+	image, err := images.GetImageDetails(s.Type)
+	if err != nil {
+		slog.Error("Failed to get image details for GUI command", "type", s.Type, "err", err)
+		return fmt.Errorf("cannot resolve image details for %q: %w", s.Type, err)
+	}
 
-	conn := libvirt.Connect()
+	conn, err := libvirt.Connect()
+	if err != nil {
+		slog.Error("Failed to connect to libvirt for GUI command", "err", err)
+		return fmt.Errorf("cannot connect to libvirt: %w", err)
+	}
 	defer handling.CloseConnect(conn)
 
 	// Check if the domain is up.
-	dom := libvirt.GetDomain(conn, &image, true)
+	dom, err := libvirt.GetDomain(conn, &image, true)
+	if err != nil {
+		slog.Error("Failed to lookup domain for GUI command", "image", image.Name, "err", err)
+		return fmt.Errorf("cannot look up domain %q: %w", image.Name, err)
+	}
 	defer handling.FreeDomain(dom)
 
-	var args []string
-
-	if virtViewerPath, err := paths.GetCmdPath(virtViewerBin); c.Viewer == virtViewerBin && err == nil {
-		args = []string{
-			virtViewerPath,
-			"--connect",
-			constants.ConnectURI,
-			image.Name,
-		}
-
-		if c.Fullscreen {
-			args = append(args, []string{"--full-screen"}...)
-		}
-
-	} else if remminaPath, err := paths.GetCmdPath(remminaBin); c.Viewer == remminaBin && err == nil {
-		args = []string{
-			remminaPath,
-			"-c",
-			"SPICE://localhost",
-		}
-	} else {
-		slog.Error("Unable to locate viewer to connect to the VM", "viewer", c.Viewer)
-		return fmt.Errorf("unable to locate %s to connect to the VM", c.Viewer)
+	args, err := c.viewerArgs(&image)
+	if err != nil {
+		slog.Error("Failed to resolve viewer arguments", "viewer", c.Viewer, "err", err)
+		return fmt.Errorf("cannot resolve viewer %q: %w", c.Viewer, err)
 	}
 
 	cwd, err := os.Getwd()
@@ -73,8 +66,46 @@ func (c *GuiCommand) Run(s *options.Options) error {
 	err = cmd.Start()
 	if err != nil {
 		slog.Error("Cannot spawn viewer process", "err", err)
+		return fmt.Errorf("cannot start viewer %q: %w", args[0], err)
 	}
 	defer handling.ReleaseProcess(cmd.Process)
 
 	return nil
 }
+
+func (c *GuiCommand) viewerArgs(image *images.Image) ([]string, error) {
+	switch c.Viewer {
+	case virtViewerBin:
+		virtViewerPath, err := paths.GetCmdPath(virtViewerBin)
+		if err != nil {
+			slog.Error("Unable to locate viewer", "viewer", c.Viewer, "err", err)
+			return nil, fmt.Errorf("unable to locate %s", c.Viewer)
+		}
+
+		args := []string{
+			virtViewerPath,
+			"--connect",
+			constants.ConnectURI,
+			image.Name,
+		}
+		if c.Fullscreen {
+			args = append(args, "--full-screen")
+		}
+		return args, nil
+	case remminaBin:
+		remminaPath, err := paths.GetCmdPath(remminaBin)
+		if err != nil {
+			slog.Error("Unable to locate viewer", "viewer", c.Viewer, "err", err)
+			return nil, fmt.Errorf("unable to locate %s", c.Viewer)
+		}
+
+		return []string{
+			remminaPath,
+			"-c",
+			"SPICE://localhost",
+		}, nil
+	default:
+		slog.Error("Unsupported viewer", "viewer", c.Viewer)
+		return nil, fmt.Errorf("cannot use viewer %s: unsupported", c.Viewer)
+	}
+}