Skip to content

Commit

Permalink
cmd/utils, pkg/utils: Beautify the error messages for the distro option
Browse files Browse the repository at this point in the history
  • Loading branch information
debarshiray committed Sep 2, 2022
1 parent 8ca5611 commit c07c0cc
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 14 deletions.
32 changes: 32 additions & 0 deletions src/cmd/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ func createErrorContainerNotFound(container string) error {
return errors.New(errMsg)
}

func createErrorDistroWithoutRelease(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)
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 @@ -81,6 +91,16 @@ func createErrorInvalidContainer(containerArg string) error {
return errors.New(errMsg)
}

func createErrorInvalidDistro(distro string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '--distro'\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 createErrorInvalidImageForContainerName(container string) error {
var builder strings.Builder
fmt.Fprintf(&builder, "invalid argument for '--image'\n")
Expand Down Expand Up @@ -122,6 +142,7 @@ func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI,

if err != nil {
var errContainer *utils.ContainerError
var errDistro *utils.DistroError
var errParseRelease *utils.ParseReleaseError

if errors.As(err, &errContainer) {
Expand All @@ -140,6 +161,17 @@ func resolveContainerAndImageNames(container, containerArg, distroCLI, imageCLI,
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)
return "", "", "", err
} else if errors.Is(err, utils.ErrDistroWithoutRelease) {
err := createErrorDistroWithoutRelease(errDistro.Distro)
return "", "", "", err
} else {
panicMsg := fmt.Sprintf("unexpected %T: %s", err, err)
panic(panicMsg)
}
} else if errors.As(err, &errParseRelease) {
err := createErrorInvalidRelease(errParseRelease.Hint)
return "", "", "", err
Expand Down
14 changes: 14 additions & 0 deletions src/pkg/utils/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ type ContainerError struct {
Err error
}

type DistroError struct {
Distro string
Err error
}

type ParseReleaseError struct {
Hint string
}
Expand All @@ -39,6 +44,15 @@ func (err *ContainerError) Unwrap() error {
return err.Err
}

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

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

func (err *ParseReleaseError) Error() string {
return err.Hint
}
8 changes: 6 additions & 2 deletions src/pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ var (
ErrContainerNameFromImageInvalid = errors.New("container name generated from image is invalid")

ErrContainerNameInvalid = errors.New("container name is invalid")

ErrDistroUnsupported = errors.New("distribution is unsupported")

ErrDistroWithoutRelease = errors.New("non-default distribution must specify release")
)

func init() {
Expand Down Expand Up @@ -713,11 +717,11 @@ func ResolveContainerAndImageNames(container, distroCLI, imageCLI, releaseCLI st
}

if _, ok := supportedDistros[distro]; !ok {
return "", "", "", fmt.Errorf("distribution %s is unsupported", distro)
return "", "", "", &DistroError{distro, ErrDistroUnsupported}
}

if distro != distroDefault && releaseCLI == "" && !viper.IsSet("general.release") {
return "", "", "", fmt.Errorf("release not found for non-default distribution %s", distro)
return "", "", "", &DistroError{distro, ErrDistroWithoutRelease}
}

if releaseCLI == "" {
Expand Down
12 changes: 8 additions & 4 deletions test/system/101-create.bats
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ teardown() {
run $TOOLBOX --assumeyes create --distro "$distro"

assert_failure
assert_line --index 0 "Error: distribution $distro is unsupported"
assert [ ${#lines[@]} -eq 1 ]
assert_line --index 0 "Error: invalid argument for '--distro'"
assert_line --index 1 "Distribution $distro is unsupported."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 3 ]
}

@test "create: Try to create a container based on non-existent image" {
Expand Down Expand Up @@ -158,8 +160,10 @@ teardown() {
run $TOOLBOX -y create -d "$distro"

assert_failure
assert_line --index 0 "Error: release not found for non-default distribution $distro"
assert [ ${#lines[@]} -eq 1 ]
assert_line --index 0 "Error: option '--release' is needed"
assert_line --index 1 "Distribution $distro doesn't match the host."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 3 ]
}

@test "create: Try to create a container and pass a non-existent file to the --authfile option" {
Expand Down
12 changes: 8 additions & 4 deletions test/system/104-run.bats
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ teardown() {
run $TOOLBOX --assumeyes run --distro "$distro" ls

assert_failure
assert_line --index 0 "Error: distribution $distro is unsupported"
assert [ ${#lines[@]} -eq 1 ]
assert_line --index 0 "Error: invalid argument for '--distro'"
assert_line --index 1 "Distribution $distro is unsupported."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 3 ]
}

@test "run: Try to run a command in a container based on Fedora but with wrong version" {
Expand Down Expand Up @@ -113,8 +115,10 @@ teardown() {
run $TOOLBOX run -d "$distro" ls

assert_failure
assert_line --index 0 "Error: release not found for non-default distribution $distro"
assert [ ${#lines[@]} -eq 1 ]
assert_line --index 0 "Error: option '--release' is needed"
assert_line --index 1 "Distribution $distro doesn't match the host."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 3 ]
}

@test "run: Run echo 'Hello World' inside of the default container" {
Expand Down
12 changes: 8 additions & 4 deletions test/system/105-enter.bats
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ teardown() {
run $TOOLBOX --assumeyes enter --distro "$distro"

assert_failure
assert_line --index 0 "Error: distribution $distro is unsupported"
assert [ ${#lines[@]} -eq 1 ]
assert_line --index 0 "Error: invalid argument for '--distro'"
assert_line --index 1 "Distribution $distro is unsupported."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 3 ]
}

@test "enter: Try to enter a container based on Fedora but with wrong version" {
Expand Down Expand Up @@ -119,8 +121,10 @@ teardown() {
run $TOOLBOX enter -d "$distro"

assert_failure
assert_line --index 0 "Error: release not found for non-default distribution $distro"
assert [ ${#lines[@]} -eq 1 ]
assert_line --index 0 "Error: option '--release' is needed"
assert_line --index 1 "Distribution $distro doesn't match the host."
assert_line --index 2 "Run 'toolbox --help' for usage."
assert [ ${#lines[@]} -eq 3 ]
}

# TODO: Write the test
Expand Down

0 comments on commit c07c0cc

Please sign in to comment.