From 4a1aa4652e27a1265574cd0b95b3cfd0ec5c12b0 Mon Sep 17 00:00:00 2001 From: Debarshi Ray Date: Thu, 24 Jun 2021 17:34:15 +0200 Subject: [PATCH] Ensure that all unknown error messages are limited to the debug logs This builds upon commit eedfdda535f762dc, which added more information to the error messages presented to the user by including the errors thrown by the lower layers of the code. However, if the errors are being thrown by external modules, or are coming from functions that are too many layers below, then it is difficult to guarantee their contents. They might be duplicating information added by the upper layers, or in extreme cases might even contain JSON blobs, simply because it made sense for the API contracts of the functions generating the errors. Therefore, it's better to put them in the debug logs to retain control over what gets displayed to the user under normal (ie., non-debug) operation. https://github.com/containers/toolbox/pull/809 --- src/cmd/create.go | 19 ++++++++++++++++--- src/cmd/list.go | 6 ++++-- src/cmd/root.go | 33 ++++++++++++++++++++++++--------- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/cmd/create.go b/src/cmd/create.go index f99342c..e3245b4 100644 --- a/src/cmd/create.go +++ b/src/cmd/create.go @@ -518,7 +518,11 @@ func getDBusSystemSocket() (string, error) { path := addressSplit[1] pathEvaled, err := filepath.EvalSymlinks(path) if err != nil { - return "", fmt.Errorf("failed to resolve the path to the D-Bus system socket: %w", err) + logrus.Debugf("Resolving path to the D-Bus system socket: failed to evaluate symbolic links in %s: %s", + path, + err) + + return "", errors.New("failed to resolve the path to the D-Bus system socket") } return pathEvaled, nil @@ -586,7 +590,11 @@ func getServiceSocket(serviceName string, unitName string) (string, error) { connection, err := dbus.SystemBus() if err != nil { - return "", fmt.Errorf("failed to connect to the D-Bus system instance: %w", err) + logrus.Debugf("Resolving path to the %s socket: failed to connect to the D-Bus system instance: %s", + serviceName, + err) + + return "", errors.New("failed to connect to the D-Bus system instance") } unitNameEscaped := systemdPathBusEscape(unitName) @@ -597,7 +605,12 @@ func getServiceSocket(serviceName string, unitName string) (string, error) { var result map[string]dbus.Variant err = call.Store(&result) if err != nil { - return "", fmt.Errorf("failed to get the properties of %s: %w", unitName, err) + logrus.Debugf("Resolving path to the %s socket: failed to get the properties of %s: %s", + serviceName, + unitName, + err) + + return "", fmt.Errorf("failed to get the properties of %s", unitName) } listenVariant, listenFound := result["Listen"] diff --git a/src/cmd/list.go b/src/cmd/list.go index e9f676a..8893e5d 100644 --- a/src/cmd/list.go +++ b/src/cmd/list.go @@ -133,7 +133,8 @@ func getContainers() ([]toolboxContainer, error) { args := []string{"--all", "--sort", "names"} containers, err := podman.GetContainers(args...) if err != nil { - return nil, fmt.Errorf("failed to get containers: %w", err) + logrus.Debugf("Fetching all containers failed: %s", err) + return nil, errors.New("failed to get containers") } var toolboxContainers []toolboxContainer @@ -195,7 +196,8 @@ func getImages() ([]toolboxImage, error) { args := []string{"--sort", "repository"} images, err := podman.GetImages(args...) if err != nil { - return nil, fmt.Errorf("failed to get images: %w", err) + logrus.Debugf("Fetching all images failed: %s", err) + return nil, errors.New("failed to get images") } var toolboxImages []toolboxImage diff --git a/src/cmd/root.go b/src/cmd/root.go index fc11e5d..ab58d82 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -203,7 +203,8 @@ func migrate() error { configDir, err := os.UserConfigDir() if err != nil { - return fmt.Errorf("failed to get the user config directory: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to get the user config directory: %s", err) + return errors.New("failed to get the user config directory") } toolboxConfigDir := configDir + "/toolbox" @@ -212,14 +213,19 @@ func migrate() error { podmanVersion, err := podman.GetVersion() if err != nil { - return fmt.Errorf("failed to get the Podman version: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to get the Podman version: %s", err) + return errors.New("failed to get the Podman version") } logrus.Debugf("Current Podman version is %s", podmanVersion) err = os.MkdirAll(toolboxConfigDir, 0775) if err != nil { - return fmt.Errorf("failed to create configuration directory: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to create configuration directory %s: %s", + toolboxConfigDir, + err) + + return errors.New("failed to create configuration directory") } toolboxRuntimeDirectory, err := utils.GetRuntimeDirectory(currentUser) @@ -231,7 +237,8 @@ func migrate() error { migrateLockFile, err := os.Create(migrateLock) if err != nil { - return fmt.Errorf("failed to create migration lock file: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to create migration lock file %s: %s", migrateLock, err) + return errors.New("failed to create migration lock file") } defer migrateLockFile.Close() @@ -239,13 +246,15 @@ func migrate() error { migrateLockFD := migrateLockFile.Fd() migrateLockFDInt := int(migrateLockFD) if err := syscall.Flock(migrateLockFDInt, syscall.LOCK_EX); err != nil { - return fmt.Errorf("failed to acquire migration lock: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to acquire migration lock on %s: %s", migrateLock, err) + return errors.New("failed to acquire migration lock") } stampBytes, err := ioutil.ReadFile(stampPath) if err != nil { if !os.IsNotExist(err) { - return fmt.Errorf("failed to read migration stamp file: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to read migration stamp file %s: %s", stampPath, err) + return errors.New("failed to read migration stamp file") } } else { stampString := string(stampBytes) @@ -268,7 +277,8 @@ func migrate() error { } if err = podman.SystemMigrate(""); err != nil { - return fmt.Errorf("failed to migrate containers: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to migrate containers: %s", err) + return errors.New("failed to migrate containers") } logrus.Debugf("Migration to Podman version %s was ok", podmanVersion) @@ -277,7 +287,11 @@ func migrate() error { podmanVersionBytes := []byte(podmanVersion + "\n") err = ioutil.WriteFile(stampPath, podmanVersionBytes, 0664) if err != nil { - return fmt.Errorf("failed to update Podman version in migration stamp file: %w", err) + logrus.Debugf("Migrating to newer Podman: failed to update Podman version in migration stamp file %s: %s", + stampPath, + err) + + return errors.New("failed to update Podman version in migration stamp file") } return nil @@ -359,7 +373,8 @@ func setUpLoggers() error { func validateSubIDFile(path string) (bool, error) { file, err := os.Open(path) if err != nil { - return false, fmt.Errorf("failed to open %s: %w", path, err) + logrus.Debugf("Validating sub-ID file: failed to open %s: %s", path, err) + return false, fmt.Errorf("failed to open %s", path) } scanner := bufio.NewScanner(file)