Skip to content

Commit

Permalink
[error] Better error message for ExitErrors (#675)
Browse files Browse the repository at this point in the history
## Summary

This PR makes three changes:

1. Handles ExitErrors specially (for non `usererr.ExitError`) by
printing the `err.Stderr` when `DEVBOX_DEBUG=1`.

2. When `DEVBOX_DEBUG=0`, we now include the command and also suggest to
the user to run with DEVBOX_DEBUG=1 for a more detailed error and
suggest reporting the bug.

3. Renames `usererr.UserExecError` to `usererr.ExitError` which more
accurately represents what that struct is doing: the error is only
wrapped if it is ExitError.

## How was it tested?

in `testdata/unfree-packages`:


BEFORE:
```
❯ DEVBOX_FEATURE_FLAKES=1 DEVBOX_FEATURE_UNIFIED_ENV=1 DEVBOX_DEBUG=0 devbox shell
Ensuring packages are installed.
Ensuring nixpkgs registry is downloaded.
Downloaded 'github:NixOS/nixpkgs/52e3e80afff4b16ccb7c52e9f0f5220552f03d04' to '/nix/store/j5a7i3dvxslg2ychfy53wdhg1m3xfrwm-source' (hash 'sha256-FqZ7b2RpoHQ/jlG6JPcCNmG/DoUPCIvyaropUDFhF3Q=').
Ensuring nixpkgs registry is downloaded: Success
Starting a devbox shell...
Error: exit status 1
```

AFTER:

<img width="1239" alt="Screenshot 2023-02-23 at 1 21 30 PM"
src="https://user-images.githubusercontent.com/676452/221033169-92c65573-a339-422d-8141-5ba9c58e0353.png">

and AFTER with DEBUG=1:


<img width="1217" alt="Screenshot 2023-02-23 at 1 15 50 PM"
src="https://user-images.githubusercontent.com/676452/221033213-ded4c288-7fec-425e-bf7e-7a75c5323f6a.png">
  • Loading branch information
savil committed Feb 24, 2023
1 parent caa8a4a commit e5b0710
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 38 deletions.
12 changes: 8 additions & 4 deletions internal/boxcli/midcobra/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
package midcobra

import (
"fmt"
"errors"
"os"
"os/exec"
"strconv"

"github.com/fatih/color"
Expand Down Expand Up @@ -59,12 +60,15 @@ func (d *DebugMiddleware) postRun(cmd *cobra.Command, args []string, runErr erro
} else {
color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "\nError: %s\n\n", runErr.Error())
}
} else {
fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", runErr)
}

st := debug.EarliestStackTrace(runErr)
debug.Log("Error: %v\nExecutionID:%s\n%+v\n", runErr, d.executionID, st)
color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "Error: %v\n\n", runErr)
var exitErr *exec.ExitError
if errors.As(runErr, &exitErr) {
debug.Log("Command stderr: %s\n", exitErr.Stderr)
}
debug.Log("\nExecutionID:%s\n%+v\n", d.executionID, st)
}

func (d *DebugMiddleware) withExecutionID(execID string) Middleware {
Expand Down
11 changes: 9 additions & 2 deletions internal/boxcli/midcobra/midcobra.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/google/uuid"
"github.com/spf13/cobra"
"go.jetpack.io/devbox/internal/boxcli/usererr"
"go.jetpack.io/devbox/internal/debug"
"go.jetpack.io/devbox/internal/ux"
)

type Executable interface {
Expand Down Expand Up @@ -64,7 +66,7 @@ func (ex *midcobraExecutable) Execute(ctx context.Context, args []string) int {
err := ex.cmd.ExecuteContext(ctx)

var postRunErr error
var userExecErr *usererr.UserExecError
var userExecErr *usererr.ExitError
// If the error is from a user exec call, exclude such error from postrun hooks.
if err != nil && !errors.As(err, &userExecErr) {
postRunErr = err
Expand All @@ -82,11 +84,16 @@ func (ex *midcobraExecutable) Execute(ctx context.Context, args []string) int {
// If the error is from the exec call, return the exit code of the exec call.
// Note: order matters! Check if it is a user exec error before a generic exit error.
var exitErr *exec.ExitError
var userExecErr *usererr.UserExecError
var userExecErr *usererr.ExitError
if errors.As(err, &userExecErr) {
return userExecErr.ExitCode()
}
if errors.As(err, &exitErr) {
if !debug.IsEnabled() {
ux.Ferror(ex.cmd.ErrOrStderr(), "There was an internal error. "+
"Run with DEVBOX_DEBUG=1 for a detailed error message, and consider reporting it at "+
"https://github.com/jetpack-io/devbox/issues\n")
}
return exitErr.ExitCode()
}
return 1 // Error exit code
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package usererr

import (
"os/exec"

"errors"
"os/exec"
)

type UserExecError struct {
// ExitError is an ExitError for a command run on behalf of a user
type ExitError struct {
err *exec.ExitError
}

Expand All @@ -19,22 +19,22 @@ func NewExecError(source error) error {
if !errors.As(source, &exitErr) {
return source
}
return &UserExecError{
return &ExitError{
err: exitErr,
}
}

func (e *UserExecError) Error() string {
func (e *ExitError) Error() string {
return e.err.Error()
}

func (e *UserExecError) Is(target error) bool {
func (e *ExitError) Is(target error) bool {
return errors.Is(e.err, target)
}

func (e *UserExecError) ExitCode() int {
func (e *ExitError) ExitCode() int {
return e.err.ExitCode()
}

// Unwrap provides compatibility for Go 1.13 error chains.
func (e *UserExecError) Unwrap() error { return e.err }
func (e *ExitError) Unwrap() error { return e.err }
25 changes: 2 additions & 23 deletions internal/impl/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ func (d *Devbox) addPackagesToProfile(mode installMode) error {
if err != nil {
fmt.Fprintf(d.writer, "%s: ", stepMsg)
color.New(color.FgRed).Fprintf(d.writer, "Fail\n")

return errors.New(commandErrorMessage(cmd, err))
return errors.Wrapf(err, "Command: %s", cmd)
}

fmt.Fprintf(d.writer, "%s: ", stepMsg)
Expand Down Expand Up @@ -240,29 +239,9 @@ func (d *Devbox) ensureNixpkgsPrefetched() error {
if err := cmd.Run(); err != nil {
fmt.Fprintf(d.writer, "Ensuring nixpkgs registry is downloaded: ")
color.New(color.FgRed).Fprintf(d.writer, "Fail\n")
return errors.New(commandErrorMessage(cmd, err))
return errors.Wrapf(err, "Command: %s", cmd)
}
fmt.Fprintf(d.writer, "Ensuring nixpkgs registry is downloaded: ")
color.New(color.FgGreen).Fprintf(d.writer, "Success\n")
return nil
}

// Consider moving to cobra middleware where this could be generalized. There is
// a complication in that its current form is useful because of the exec.Cmd. This
// would be missing in the middleware, unless we pass it along by wrapping the error in
// another struct.
func commandErrorMessage(cmd *exec.Cmd, err error) string {
var errorMsg string

// ExitErrors can give us more information so handle that specially.
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
errorMsg = fmt.Sprintf(
"Error running command %s. Exit status is %d. Command stderr: %s",
cmd, exitErr.ExitCode(), string(exitErr.Stderr),
)
} else {
errorMsg = fmt.Sprintf("Error running command %s. Error: %v", cmd, err)
}
return errorMsg
}
2 changes: 1 addition & 1 deletion internal/nix/nix.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func PrintDevEnv(nixShellFilePath, nixFlakesFilePath string) (*varsAndFuncs, err
cmd.Env = DefaultEnv()
out, err := cmd.Output()
if err != nil {
return nil, errors.WithStack(err)
return nil, errors.Wrapf(err, "Command: %s", cmd)
}

var vaf varsAndFuncs
Expand Down

0 comments on commit e5b0710

Please sign in to comment.