Skip to content

Commit e5b0710

Browse files
authored
[error] Better error message for ExitErrors (#675)
## 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">
1 parent caa8a4a commit e5b0710

File tree

5 files changed

+28
-38
lines changed

5 files changed

+28
-38
lines changed

internal/boxcli/midcobra/debug.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
package midcobra
55

66
import (
7-
"fmt"
7+
"errors"
88
"os"
9+
"os/exec"
910
"strconv"
1011

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

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

7074
func (d *DebugMiddleware) withExecutionID(execID string) Middleware {

internal/boxcli/midcobra/midcobra.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"github.com/google/uuid"
1313
"github.com/spf13/cobra"
1414
"go.jetpack.io/devbox/internal/boxcli/usererr"
15+
"go.jetpack.io/devbox/internal/debug"
16+
"go.jetpack.io/devbox/internal/ux"
1517
)
1618

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

6668
var postRunErr error
67-
var userExecErr *usererr.UserExecError
69+
var userExecErr *usererr.ExitError
6870
// If the error is from a user exec call, exclude such error from postrun hooks.
6971
if err != nil && !errors.As(err, &userExecErr) {
7072
postRunErr = err
@@ -82,11 +84,16 @@ func (ex *midcobraExecutable) Execute(ctx context.Context, args []string) int {
8284
// If the error is from the exec call, return the exit code of the exec call.
8385
// Note: order matters! Check if it is a user exec error before a generic exit error.
8486
var exitErr *exec.ExitError
85-
var userExecErr *usererr.UserExecError
87+
var userExecErr *usererr.ExitError
8688
if errors.As(err, &userExecErr) {
8789
return userExecErr.ExitCode()
8890
}
8991
if errors.As(err, &exitErr) {
92+
if !debug.IsEnabled() {
93+
ux.Ferror(ex.cmd.ErrOrStderr(), "There was an internal error. "+
94+
"Run with DEVBOX_DEBUG=1 for a detailed error message, and consider reporting it at "+
95+
"https://github.com/jetpack-io/devbox/issues\n")
96+
}
9097
return exitErr.ExitCode()
9198
}
9299
return 1 // Error exit code
Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package usererr
22

33
import (
4-
"os/exec"
5-
64
"errors"
5+
"os/exec"
76
)
87

9-
type UserExecError struct {
8+
// ExitError is an ExitError for a command run on behalf of a user
9+
type ExitError struct {
1010
err *exec.ExitError
1111
}
1212

@@ -19,22 +19,22 @@ func NewExecError(source error) error {
1919
if !errors.As(source, &exitErr) {
2020
return source
2121
}
22-
return &UserExecError{
22+
return &ExitError{
2323
err: exitErr,
2424
}
2525
}
2626

27-
func (e *UserExecError) Error() string {
27+
func (e *ExitError) Error() string {
2828
return e.err.Error()
2929
}
3030

31-
func (e *UserExecError) Is(target error) bool {
31+
func (e *ExitError) Is(target error) bool {
3232
return errors.Is(e.err, target)
3333
}
3434

35-
func (e *UserExecError) ExitCode() int {
35+
func (e *ExitError) ExitCode() int {
3636
return e.err.ExitCode()
3737
}
3838

3939
// Unwrap provides compatibility for Go 1.13 error chains.
40-
func (e *UserExecError) Unwrap() error { return e.err }
40+
func (e *ExitError) Unwrap() error { return e.err }

internal/impl/packages.go

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,7 @@ func (d *Devbox) addPackagesToProfile(mode installMode) error {
9898
if err != nil {
9999
fmt.Fprintf(d.writer, "%s: ", stepMsg)
100100
color.New(color.FgRed).Fprintf(d.writer, "Fail\n")
101-
102-
return errors.New(commandErrorMessage(cmd, err))
101+
return errors.Wrapf(err, "Command: %s", cmd)
103102
}
104103

105104
fmt.Fprintf(d.writer, "%s: ", stepMsg)
@@ -240,29 +239,9 @@ func (d *Devbox) ensureNixpkgsPrefetched() error {
240239
if err := cmd.Run(); err != nil {
241240
fmt.Fprintf(d.writer, "Ensuring nixpkgs registry is downloaded: ")
242241
color.New(color.FgRed).Fprintf(d.writer, "Fail\n")
243-
return errors.New(commandErrorMessage(cmd, err))
242+
return errors.Wrapf(err, "Command: %s", cmd)
244243
}
245244
fmt.Fprintf(d.writer, "Ensuring nixpkgs registry is downloaded: ")
246245
color.New(color.FgGreen).Fprintf(d.writer, "Success\n")
247246
return nil
248247
}
249-
250-
// Consider moving to cobra middleware where this could be generalized. There is
251-
// a complication in that its current form is useful because of the exec.Cmd. This
252-
// would be missing in the middleware, unless we pass it along by wrapping the error in
253-
// another struct.
254-
func commandErrorMessage(cmd *exec.Cmd, err error) string {
255-
var errorMsg string
256-
257-
// ExitErrors can give us more information so handle that specially.
258-
var exitErr *exec.ExitError
259-
if errors.As(err, &exitErr) {
260-
errorMsg = fmt.Sprintf(
261-
"Error running command %s. Exit status is %d. Command stderr: %s",
262-
cmd, exitErr.ExitCode(), string(exitErr.Stderr),
263-
)
264-
} else {
265-
errorMsg = fmt.Sprintf("Error running command %s. Error: %v", cmd, err)
266-
}
267-
return errorMsg
268-
}

internal/nix/nix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ func PrintDevEnv(nixShellFilePath, nixFlakesFilePath string) (*varsAndFuncs, err
155155
cmd.Env = DefaultEnv()
156156
out, err := cmd.Output()
157157
if err != nil {
158-
return nil, errors.WithStack(err)
158+
return nil, errors.Wrapf(err, "Command: %s", cmd)
159159
}
160160

161161
var vaf varsAndFuncs

0 commit comments

Comments
 (0)