Skip to content

[WIP] Improve the error messages when reading the configuration files #1106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[WIP] Improve the error messages when reading the configuration files #1106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42aedcc
cmd/utils: Rename some functions
debarshiray Sep 2, 2022
63b8bc5
Improve the error messages when reading the configuration files
debarshiray Aug 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func preRun(cmd *cobra.Command, args []string) error {
return err
}

if err := utils.SetUpConfiguration(); err != nil {
if err := setUpConfiguration(); err != nil {
return err
}

Expand Down
184 changes: 174 additions & 10 deletions src/cmd/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,49 @@ func askForConfirmation(prompt string) bool {
return retVal
}

func createErrorConfiguration(file string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "failed to read file %s\n", file)
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorConfigurationIsDirectory(file string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "failed to read file %s\n", file)
fmt.Fprintf(&builder, "Configuration file must be a regular file.\n")
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorConfigurationPermission(file string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "failed to read file %s\n", file)
fmt.Fprintf(&builder, "Configuration file must be readable.\n")
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorConfigurationSyntax(file string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "failed to parse file %s\n", file)
fmt.Fprintf(&builder, "Configuration file must be valid TOML.\n")
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorConfigurationUserConfigDir() error {
return errors.New("neither $XDG_CONFIG_HOME nor $HOME are defined")
}

func createErrorContainerNotFound(container string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "container %s not found\n", container)
Expand All @@ -71,7 +114,7 @@ func createErrorContainerNotFound(container string) error {
return errors.New(errMsg)
}

func createErrorDistroWithoutRelease(distro string) error {
func createErrorDistroWithoutReleaseForCLI(distro string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "option '--release' is needed\n")
fmt.Fprintf(&builder, "Distribution %s doesn't match the host.\n", distro)
Expand All @@ -81,6 +124,16 @@ func createErrorDistroWithoutRelease(distro string) error {
return errors.New(errMsg)
}

func createErrorDistroWithoutReleaseForConfig(distro string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "option 'release' is needed in configuration file\n")
fmt.Fprintf(&builder, "Distribution %s doesn't match the host.\n", distro)
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorInvalidContainer(containerArg string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '%s'\n", containerArg)
Expand All @@ -91,7 +144,7 @@ func createErrorInvalidContainer(containerArg string) error {
return errors.New(errMsg)
}

func createErrorInvalidDistro(distro string) error {
func createErrorInvalidDistroForCLI(distro string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '--distro'\n")
fmt.Fprintf(&builder, "Distribution %s is unsupported.\n", distro)
Expand All @@ -101,7 +154,17 @@ func createErrorInvalidDistro(distro string) error {
return errors.New(errMsg)
}

func createErrorInvalidImageForContainerName(container string) error {
func createErrorInvalidDistroForConfig(distro string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for 'distro' in configuration file\n")
fmt.Fprintf(&builder, "Distribution %s is unsupported.\n", distro)
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorInvalidImageForContainerNameForCLI(container string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '--image'\n")
fmt.Fprintf(&builder, "Container name %s generated from image is invalid.\n", container)
Expand All @@ -112,7 +175,18 @@ func createErrorInvalidImageForContainerName(container string) error {
return errors.New(errMsg)
}

func createErrorInvalidImageWithoutBasename() error {
func createErrorInvalidImageForContainerNameForConfig() error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for 'image' in configuration file\n")
fmt.Fprintf(&builder, "Image gives an invalid container name.\n")
fmt.Fprintf(&builder, "Container names must match '%s'.\n", utils.ContainerNameRegexp)
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorInvalidImageWithoutBasenameForCLI() error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '--image'\n")
fmt.Fprintf(&builder, "Images must have basenames.\n")
Expand All @@ -122,7 +196,17 @@ func createErrorInvalidImageWithoutBasename() error {
return errors.New(errMsg)
}

func createErrorInvalidRelease(hint string) error {
func createErrorInvalidImageWithoutBasenameForConfig() error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for 'image' in configuration file\n")
fmt.Fprintf(&builder, "Images must have basenames.\n")
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func createErrorInvalidReleaseForCLI(hint string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '--release'\n")
fmt.Fprintf(&builder, "%s\n", hint)
Expand All @@ -132,6 +216,16 @@ func createErrorInvalidRelease(hint string) error {
return errors.New(errMsg)
}

func createErrorInvalidReleaseForConfig(hint string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for 'release'\n")
fmt.Fprintf(&builder, "%s\n", hint)
fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)

errMsg := builder.String()
return errors.New(errMsg)
}

func getUsageForCommonCommands() string {
var builder strings.Builder
fmt.Fprintf(&builder, "create Create a new toolbox container\n")
Expand Down Expand Up @@ -166,33 +260,33 @@ func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI,
err := createErrorInvalidContainer(containerArg)
return "", "", "", err
} else if errors.Is(err, utils.ErrContainerNameFromImageInvalid) {
err := createErrorInvalidImageForContainerName(errContainer.Container)
err := createErrorInvalidImageForContainerNameForCLI(errContainer.Container)
return "", "", "", err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errDistro) {
if errors.Is(err, utils.ErrDistroUnsupported) {
err := createErrorInvalidDistro(errDistro.Distro)
err := createErrorInvalidDistroForCLI(errDistro.Distro)
return "", "", "", err
} else if errors.Is(err, utils.ErrDistroWithoutRelease) {
err := createErrorDistroWithoutRelease(errDistro.Distro)
err := createErrorDistroWithoutReleaseForCLI(errDistro.Distro)
return "", "", "", err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errImage) {
if errors.Is(err, utils.ErrImageWithoutBasename) {
err := createErrorInvalidImageWithoutBasename()
err := createErrorInvalidImageWithoutBasenameForCLI()
return "", "", "", err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errParseRelease) {
err := createErrorInvalidRelease(errParseRelease.Hint)
err := createErrorInvalidReleaseForCLI(errParseRelease.Hint)
return "", "", "", err
} else {
return "", "", "", err
Expand All @@ -202,6 +296,76 @@ func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI,
return container, image, release, nil
}

func setUpConfiguration() error {
if err := utils.SetUpConfiguration(); err != nil {
if errors.Is(err, utils.ErrContainerNameInvalid) {
panicMsg := fmt.Sprintf("invalid container when reading configuration: %s", err)
panic(panicMsg)
}

var errConfiguration *utils.ConfigurationError
var errContainer *utils.ContainerError
var errDistro *utils.DistroError
var errImage *utils.ImageError
var errParseRelease *utils.ParseReleaseError

if errors.As(err, &errConfiguration) {
EISDIR := errors.New("is a directory")

if errors.Is(err, EISDIR) {
err := createErrorConfigurationIsDirectory(errConfiguration.File)
return err
} else if errors.Is(err, os.ErrPermission) {
err := createErrorConfigurationPermission(errConfiguration.File)
return err
} else if errors.Is(err, utils.ErrConfigurationSyntax) {
err := createErrorConfigurationSyntax(errConfiguration.File)
return err
} else if errors.Is(err, utils.ErrConfigurationUserConfigDir) {
err := createErrorConfigurationUserConfigDir()
return err
} else {
err := createErrorConfiguration(errConfiguration.File)
return err
}
} else if errors.As(err, &errContainer) {
if errors.Is(err, utils.ErrContainerNameFromImageInvalid) {
err := createErrorInvalidImageForContainerNameForConfig()
return err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errDistro) {
if errors.Is(err, utils.ErrDistroUnsupported) {
err := createErrorInvalidDistroForConfig(errDistro.Distro)
return err
} else if errors.Is(err, utils.ErrDistroWithoutRelease) {
err := createErrorDistroWithoutReleaseForConfig(errDistro.Distro)
return err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errImage) {
if errors.Is(err, utils.ErrImageWithoutBasename) {
err := createErrorInvalidImageWithoutBasenameForConfig()
return err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errParseRelease) {
err := createErrorInvalidReleaseForConfig(errParseRelease.Hint)
return err
} else {
return err
}
}

return nil
}

// showManual tries to open the specified manual page using man on stdout
func showManual(manual string) error {
manBinary, err := exec.LookPath("man")
Expand Down
14 changes: 14 additions & 0 deletions src/pkg/utils/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import (
"fmt"
)

type ConfigurationError struct {
File string
Err error
}

type ContainerError struct {
Container string
Image string
Expand All @@ -40,6 +45,15 @@ type ParseReleaseError struct {
Hint string
}

func (err *ConfigurationError) Error() string {
errMsg := fmt.Sprintf("%s: %s", err.File, err.Err)
return errMsg
}

func (err *ConfigurationError) Unwrap() error {
return err.Err
}

func (err *ContainerError) Error() string {
errMsg := fmt.Sprintf("%s: %s", err.Container, err.Err)
return errMsg
Expand Down
13 changes: 8 additions & 5 deletions src/pkg/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ var (
var (
ContainerNameDefault string

ErrConfigurationSyntax = errors.New("failed to parse")

ErrConfigurationUserConfigDir = errors.New("neither $XDG_CONFIG_HOME nor $HOME are defined")

ErrContainerNameFromImageInvalid = errors.New("container name generated from image is invalid")

ErrContainerNameInvalid = errors.New("container name is invalid")
Expand Down Expand Up @@ -558,7 +562,7 @@ func SetUpConfiguration() error {
userConfigDir, err := os.UserConfigDir()
if err != nil {
logrus.Debugf("Setting up configuration: failed to get the user config directory: %s", err)
return errors.New("failed to get the user config directory")
return &ConfigurationError{"", ErrConfigurationUserConfigDir}
}

userConfigPath := userConfigDir + "/containers/toolbox.conf"
Expand All @@ -580,18 +584,17 @@ func SetUpConfiguration() error {
continue
} else if errors.As(err, &errConfigParse) {
logrus.Debugf("Setting up configuration: failed to parse file %s: %s", configFile, err)
return fmt.Errorf("failed to parse file %s", configFile)
return &ConfigurationError{configFile, ErrConfigurationSyntax}
} else {
logrus.Debugf("Setting up configuration: failed to read file %s: %s", configFile, err)
return fmt.Errorf("failed to read file %s", configFile)
return &ConfigurationError{configFile, err}
}
}
}

container, _, _, err := ResolveContainerAndImageNames("", "", "", "")
if err != nil {
logrus.Debugf("Setting up configuration: failed to resolve container name: %s", err)
return errors.New("failed to resolve container name")
return err
}

ContainerNameDefault = container
Expand Down