Skip to content

Commit e5dc817

Browse files
authored
Update sentry to avoid logging errors from os exec commands (#449)
## Summary Update sentry to avoid logging errors from os exec commands. After #441 goes in, sentry starts to log user script errors which are not devbox errors. This is to exclude those errors. cc @devholic ## How was it tested? devbox run build ./dist/devbox shell -- exit 1 ./dist/devbox run nonexistentcmd
1 parent e61da20 commit e5dc817

File tree

4 files changed

+72
-5
lines changed

4 files changed

+72
-5
lines changed

internal/boxcli/midcobra/midcobra.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/google/uuid"
1313
"github.com/spf13/cobra"
14+
"go.jetpack.io/devbox/internal/boxcli/usererr"
1415
)
1516

1617
type Executable interface {
@@ -62,17 +63,29 @@ func (ex *midcobraExecutable) Execute(ctx context.Context, args []string) int {
6263
// Execute the cobra command:
6364
err := ex.cmd.ExecuteContext(ctx)
6465

66+
var postRunErr error
67+
var userExecErr *usererr.UserExecError
68+
// If the error is from a user exec call, exclude such error from postrun hooks.
69+
if err != nil && !errors.As(err, &userExecErr) {
70+
postRunErr = err
71+
}
72+
6573
// Run the 'post' hooks. Note that unlike the default PostRun cobra functionality these
6674
// run even if the command resulted in an error. This is useful when we still want to clean up
6775
// before the program exists or we want to log something. The error, if any, gets passed
6876
// to the post hook.
6977
for _, m := range ex.middlewares {
70-
m.postRun(ex.cmd, args, err)
78+
m.postRun(ex.cmd, args, postRunErr)
7179
}
7280

7381
if err != nil {
7482
// If the error is from the exec call, return the exit code of the exec call.
83+
// Note: order matters! Check if it is a user exec error before a generic exit error.
7584
var exitErr *exec.ExitError
85+
var userExecErr *usererr.UserExecError
86+
if errors.As(err, &userExecErr) {
87+
return userExecErr.ExitCode()
88+
}
7689
if errors.As(err, &exitErr) {
7790
return exitErr.ExitCode()
7891
}

internal/boxcli/usererr/execerr.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package usererr
2+
3+
import (
4+
"os/exec"
5+
6+
"errors"
7+
)
8+
9+
type UserExecError struct {
10+
err *exec.ExitError
11+
}
12+
13+
func NewExecError(source error) error {
14+
if source == nil {
15+
return nil
16+
}
17+
var exitErr *exec.ExitError
18+
// If the error is not from the exec call, return the original error.
19+
if !errors.As(source, &exitErr) {
20+
return source
21+
}
22+
return &UserExecError{
23+
err: exitErr,
24+
}
25+
}
26+
27+
func (e *UserExecError) Error() string {
28+
return e.err.Error()
29+
}
30+
31+
func (e *UserExecError) Is(target error) bool {
32+
return errors.Is(e.err, target)
33+
}
34+
35+
func (e *UserExecError) ExitCode() int {
36+
return e.err.ExitCode()
37+
}
38+
39+
// Unwrap provides compatibility for Go 1.13 error chains.
40+
func (e *UserExecError) Unwrap() error { return e.err }

internal/nix/nix.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212

1313
"github.com/pkg/errors"
14+
"go.jetpack.io/devbox/internal/boxcli/usererr"
1415
"go.jetpack.io/devbox/internal/debug"
1516
)
1617

@@ -42,7 +43,7 @@ func Exec(path string, command []string, env []string) error {
4243
cmd.Stdout = os.Stdout
4344
cmd.Stderr = os.Stderr
4445
cmd.Env = append(DefaultEnv(), env...)
45-
return errors.WithStack(cmd.Run())
46+
return errors.WithStack(usererr.NewExecError(cmd.Run()))
4647
}
4748

4849
func PkgInfo(nixpkgsCommit, pkg string) (*Info, bool) {

internal/nix/shell.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/alessio/shellescape"
1818
"github.com/pkg/errors"
1919
"go.jetpack.io/devbox/internal/boxcli/featureflag"
20+
"go.jetpack.io/devbox/internal/boxcli/usererr"
2021
"go.jetpack.io/devbox/internal/debug"
2122
)
2223

@@ -210,7 +211,13 @@ func (s *Shell) Run(nixShellFilePath string) error {
210211

211212
debug.Log("Executing nix develop command: %v", cmd.Args)
212213
debug.Log("Executing nix develop command with env: %v", cmd.Environ())
213-
return errors.WithStack(cmd.Run())
214+
215+
err := cmd.Run()
216+
if err != nil && s.ScriptCommand != "" {
217+
// Report error as exec error when executing shell -- <cmd> script.
218+
err = usererr.NewExecError(err)
219+
}
220+
return errors.WithStack(err)
214221
}
215222

216223
// Launch a fallback shell if we couldn't find the path to the user's
@@ -237,7 +244,12 @@ func (s *Shell) Run(nixShellFilePath string) error {
237244
cmd.Stderr = os.Stderr
238245

239246
debug.Log("Executing nix-shell command: %v", cmd.Args)
240-
return errors.WithStack(cmd.Run())
247+
err := cmd.Run()
248+
if err != nil && s.ScriptCommand != "" {
249+
// Report error as exec error when executing shell -- <cmd> script.
250+
err = usererr.NewExecError(err)
251+
}
252+
return errors.WithStack(err)
241253
}
242254

243255
// execCommand is a command that replaces the current shell with s. This is what
@@ -302,7 +314,8 @@ func (s *Shell) RunInShell() error {
302314
cmd.Stdout = os.Stdout
303315
cmd.Stderr = os.Stderr
304316
debug.Log("Executing command from inside devbox shell: %v", cmd.Args)
305-
return errors.WithStack(cmd.Run())
317+
318+
return errors.WithStack(usererr.NewExecError(cmd.Run()))
306319
}
307320

308321
func (s *Shell) shellRCOverrides(shellrc string) (extraEnv []string, extraArgs []string) {

0 commit comments

Comments
 (0)