From e130310b1f070fc7e1027281ec61da72067dfa89 Mon Sep 17 00:00:00 2001 From: abhtripathi Date: Tue, 8 Sep 2026 14:49:47 +0530 Subject: [PATCH 1/2] fix(exec): treat remote non-zero exit as command failure, not connection failure --- main.go | 9 +++ pkg/cmd/exec/exec.go | 119 ++++++++++++++++++++++++++++++++------ pkg/cmd/exec/exec_test.go | 112 +++++++++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 17 deletions(-) create mode 100644 pkg/cmd/exec/exec_test.go diff --git a/main.go b/main.go index 66b4d97e9..3183ce490 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,13 @@ package main import ( + stderrors "errors" "os" "github.com/brevdev/brev-cli/pkg/analytics" "github.com/brevdev/brev-cli/pkg/cmd" "github.com/brevdev/brev-cli/pkg/cmd/cmderrors" + "github.com/brevdev/brev-cli/pkg/cmd/exec" "github.com/brevdev/brev-cli/pkg/errors" ) @@ -16,6 +18,13 @@ func main() { command := cmd.NewDefaultBrevCommand() if err := command.Execute(); err != nil { + // A remote command exiting non-zero is not a CLI error: pass its exit + // code through so callers can branch on it, and print nothing extra. + var remoteErr exec.RemoteExitError + if stderrors.As(err, &remoteErr) { + done() + os.Exit(remoteErr.Code) //nolint:gocritic // manually call done + } analytics.CaptureCommandError() cmderrors.DisplayAndHandleError(err) done() diff --git a/pkg/cmd/exec/exec.go b/pkg/cmd/exec/exec.go index e7f0cd924..2b16fea49 100644 --- a/pkg/cmd/exec/exec.go +++ b/pkg/cmd/exec/exec.go @@ -2,6 +2,7 @@ package exec import ( "bufio" + stderrors "errors" "fmt" "os" "os/exec" @@ -52,6 +53,7 @@ type ExecStore interface { refresh.RefreshStore GetOrganizations(options *store.GetOrganizationsOptions) ([]entity.Organization, error) GetWorkspaces(organizationID string, options *store.GetWorkspacesOptions) ([]entity.Workspace, error) + GetAuthTokens() (*entity.AuthTokens, error) } func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecStore) *cobra.Command { @@ -86,6 +88,11 @@ func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecSto return breverrors.NewValidationError("command is required") } + // Heads-up only: exec can still succeed logged out if the SSH config is warm. + if isLoggedOut(store) { + fmt.Fprintf(os.Stderr, "You are logged out. Trying with your existing SSH config; you'll be prompted to log in if it fails.\n") + } + // Run on each instance var errors error for _, instanceName := range instanceNames { @@ -107,7 +114,7 @@ func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecSto } } if errors != nil { - return breverrors.WrapAndTrace(errors) + return breverrors.WrapAndTrace(flattenMultiInstanceErr(errors)) } return nil }, @@ -172,8 +179,59 @@ func parseCommand(command string) (string, error) { return command, nil } +// flattenMultiInstanceErr drops the error types from a multi-instance failure. +// One remote exit code cannot represent several instances, so the process exits +// 1 instead of picking whichever RemoteExitError happens to be first. +func flattenMultiInstanceErr(err error) error { + if err == nil { + return nil + } + return stderrors.New(err.Error()) +} + +type authTokenGetter interface { + GetAuthTokens() (*entity.AuthTokens, error) +} + +// isLoggedOut reports whether no credentials are saved. Local file read, no network. +func isLoggedOut(sstore authTokenGetter) bool { + tokens, err := sstore.GetAuthTokens() + if err != nil { + var notFound *breverrors.CredentialsFileNotFound + return stderrors.As(err, ¬Found) + } + if tokens == nil { + return true + } + return tokens.AccessToken == "" && tokens.RefreshToken == "" && strings.TrimSpace(tokens.APIKey) == "" +} + const pollTimeout = 10 * time.Minute +// sshConnectionFailedExitCode is the exit code ssh reserves for its own +// failures (unreachable host, auth rejected). Any other non-zero code is the +// remote command's own exit status, which means the connection worked. +const sshConnectionFailedExitCode = 255 + +// RemoteExitError means we connected and ran the command successfully, and the +// command itself exited non-zero. It is not a connection failure. +type RemoteExitError struct { + Code int +} + +func (e RemoteExitError) Error() string { + return fmt.Sprintf("command exited with status %d", e.Code) +} + +// exitCodeOf returns the process exit code for err, or -1 if err is not an exit error. +func exitCodeOf(err error) int { + var exitErr *exec.ExitError + if stderrors.As(err, &exitErr) { + return exitErr.ExitCode() + } + return -1 +} + func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID string, host bool, command string) error { // Determine SSH alias: use the workspace name directly (with -host suffix if needed) sshName := workspaceNameOrID @@ -192,6 +250,14 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st return nil } + // We connected fine and the command exited non-zero. Surface its exit code + // rather than treating a healthy connection as a failure. + var remoteErr RemoteExitError + if stderrors.As(err, &remoteErr) { + go trackExecAnalytics(sstore, workspaceNameOrID) + return remoteErr + } + // SSH failed — now check what's going on with the instance fmt.Fprintf(os.Stderr, "Connection failed, checking instance status...\n") @@ -228,12 +294,7 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st if err != nil { return breverrors.WrapAndTrace(err) } - err = runSSH(sshName, command) - if err != nil { - return breverrors.WrapAndTrace(err) - } - go trackExecAnalytics(sstore, workspaceNameOrID) - return nil + return runAndTrack(sstore, sshName, workspaceNameOrID, command) } if workspace.Status != "RUNNING" { @@ -261,12 +322,24 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st "could not connect to instance %q: %w\nPlease check with: brev ls", workspaceNameOrID, err)) } - err = runSSH(sshName, command) - if err != nil { - return breverrors.WrapAndTrace(err) + return runAndTrack(sstore, sshName, workspaceNameOrID, command) +} + +// runAndTrack runs the command after recovery. A non-zero exit from the remote +// command is returned as RemoteExitError, not as a connection failure. +func runAndTrack(sstore ExecStore, sshName string, workspaceNameOrID string, command string) error { + err := runSSH(sshName, command) + if err == nil { + go trackExecAnalytics(sstore, workspaceNameOrID) + return nil + } + + var remoteErr RemoteExitError + if stderrors.As(err, &remoteErr) { + go trackExecAnalytics(sstore, workspaceNameOrID) + return remoteErr } - go trackExecAnalytics(sstore, workspaceNameOrID) - return nil + return breverrors.WrapAndTrace(err) } func trackExecAnalytics(sstore ExecStore, workspaceNameOrID string) { @@ -296,18 +369,30 @@ func runSSHWithTimeout(sshAlias string, command string, connectTimeoutSecs int) // -T disables pseudo-terminal allocation (no "Pseudo-terminal will not be allocated" warning) // Only start ssh-agent if one isn't already running (avoids orphaned agent processes) agentCmd := `if [ -z "$SSH_AUTH_SOCK" ]; then eval $(ssh-agent -s) > /dev/null; fi` - cmd := fmt.Sprintf("%s && ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", agentCmd, connectTimeoutSecs, sshAlias, escapedCmd) + // `exec` replaces bash with ssh so the exit code we observe is ssh's own, + // not bash's, which keeps the 255 check below meaningful. + cmd := fmt.Sprintf("%s && exec ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", agentCmd, connectTimeoutSecs, sshAlias, escapedCmd) sshCmd := exec.Command("bash", "-c", cmd) //nolint:gosec //cmd is user input sshCmd.Stderr = os.Stderr sshCmd.Stdout = os.Stdout // Don't attach stdin - exec is non-interactive - err := sshCmd.Run() - if err != nil { - return breverrors.WrapAndTrace(err) + return classifySSHError(sshCmd.Run()) +} + +// classifySSHError maps an ssh process error to a RemoteExitError when the code +// came from the remote command, or a wrapped error when ssh itself failed. +func classifySSHError(err error) error { + if err == nil { + return nil + } + // ssh uses 255 for its own failures; any other code came from the remote + // command, which means the connection itself was fine. + if code := exitCodeOf(err); code > 0 && code != sshConnectionFailedExitCode { + return RemoteExitError{Code: code} } - return nil + return breverrors.WrapAndTrace(err) } func runSSH(sshAlias string, command string) error { diff --git a/pkg/cmd/exec/exec_test.go b/pkg/cmd/exec/exec_test.go new file mode 100644 index 000000000..487421832 --- /dev/null +++ b/pkg/cmd/exec/exec_test.go @@ -0,0 +1,112 @@ +package exec + +import ( + stderrors "errors" + "os/exec" + "strconv" + "testing" + + "github.com/brevdev/brev-cli/pkg/entity" + breverrors "github.com/brevdev/brev-cli/pkg/errors" + "github.com/hashicorp/go-multierror" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// exitErrWithCode returns a real *exec.ExitError carrying the given code. +func exitErrWithCode(t *testing.T, code int) *exec.ExitError { + t.Helper() + var exitErr *exec.ExitError + err := exec.Command("bash", "-c", "exit "+strconv.Itoa(code)).Run() + require.ErrorAs(t, err, &exitErr) + return exitErr +} + +func TestClassifySSHError(t *testing.T) { + assert.NoError(t, classifySSHError(nil)) + + // A remote command's own exit code means the connection worked. + for _, code := range []int{1, 2, 7, 127} { + var remoteErr RemoteExitError + err := classifySSHError(exitErrWithCode(t, code)) + require.True(t, stderrors.As(err, &remoteErr), "code %d should be a RemoteExitError", code) + assert.Equal(t, code, remoteErr.Code) + } + + // 255 is ssh's own failure code, so it must stay a connection error. + var remoteErr RemoteExitError + err := classifySSHError(exitErrWithCode(t, sshConnectionFailedExitCode)) + require.Error(t, err) + assert.False(t, stderrors.As(err, &remoteErr), "255 must not be treated as a remote exit") +} + +func TestExitCodeOf(t *testing.T) { + assert.Equal(t, 3, exitCodeOf(exitErrWithCode(t, 3))) + assert.Equal(t, -1, exitCodeOf(stderrors.New("not an exit error"))) + assert.Equal(t, -1, exitCodeOf(nil)) +} + +// A multi-instance run must not exit with one instance's remote code. +func TestFlattenMultiInstanceErr(t *testing.T) { + assert.NoError(t, flattenMultiInstanceErr(nil)) + + agg := multierror.Append(nil, RemoteExitError{Code: 3}, RemoteExitError{Code: 7}) + flat := flattenMultiInstanceErr(agg) + + var remoteErr RemoteExitError + require.Error(t, flat) + assert.False(t, stderrors.As(flat, &remoteErr), "no exit code may leak from a multi-instance run") + // The per-instance detail is still readable in the message. + assert.Contains(t, flat.Error(), "status 3") + assert.Contains(t, flat.Error(), "status 7") +} + +type fakeTokenStore struct { + tokens *entity.AuthTokens + err error +} + +func (f fakeTokenStore) GetAuthTokens() (*entity.AuthTokens, error) { return f.tokens, f.err } + +func TestIsLoggedOut(t *testing.T) { + cases := map[string]struct { + store fakeTokenStore + want bool + }{ + "no credentials file": {fakeTokenStore{err: &breverrors.CredentialsFileNotFound{}}, true}, + "wrapped not found": {fakeTokenStore{err: breverrors.WrapAndTrace(&breverrors.CredentialsFileNotFound{})}, true}, + "nil tokens": {fakeTokenStore{}, true}, + "empty tokens": {fakeTokenStore{tokens: &entity.AuthTokens{}}, true}, + "has access token": {fakeTokenStore{tokens: &entity.AuthTokens{AccessToken: "a"}}, false}, + "has refresh token": {fakeTokenStore{tokens: &entity.AuthTokens{RefreshToken: "r"}}, false}, + "has api key": {fakeTokenStore{tokens: &entity.AuthTokens{APIKey: "k"}}, false}, + // An unrelated read error must not claim the user is logged out. + "other error": {fakeTokenStore{err: stderrors.New("permission denied")}, false}, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.want, isLoggedOut(tc.store)) + }) + } +} + +// RemoteExitError must survive the wrapping every return path applies, because +// main.go recovers the exit code with errors.As. +func TestRemoteExitErrorSurvivesWrapping(t *testing.T) { + cases := map[string]error{ + "unwrapped": RemoteExitError{Code: 4}, + "wrapped": breverrors.WrapAndTrace(RemoteExitError{Code: 4}), + "doubleWrap": breverrors.WrapAndTrace(breverrors.WrapAndTrace(RemoteExitError{Code: 4})), + "multierror": multierror.Append(nil, RemoteExitError{Code: 4}), + "multiWrapped": multierror.Append(nil, breverrors.WrapAndTrace(RemoteExitError{Code: 4})), + } + + for name, err := range cases { + t.Run(name, func(t *testing.T) { + var remoteErr RemoteExitError + require.True(t, stderrors.As(err, &remoteErr), "errors.As must match through %s", name) + assert.Equal(t, 4, remoteErr.Code) + }) + } +} From 5047e98d97fb697ba4d5c40a349753f4fad81703 Mon Sep 17 00:00:00 2001 From: abhtripathi Date: Fri, 11 Sep 2026 12:46:50 +0530 Subject: [PATCH 2/2] refactor: move RemoteExitError to pkg/errors so main.go does not import a subcommand --- main.go | 6 ++--- pkg/cmd/exec/exec.go | 50 +++++++++++---------------------------- pkg/cmd/exec/exec_test.go | 29 +++++++++++------------ pkg/errors/errors.go | 9 +++++++ 4 files changed, 39 insertions(+), 55 deletions(-) diff --git a/main.go b/main.go index 3183ce490..8047a202d 100644 --- a/main.go +++ b/main.go @@ -7,7 +7,6 @@ import ( "github.com/brevdev/brev-cli/pkg/analytics" "github.com/brevdev/brev-cli/pkg/cmd" "github.com/brevdev/brev-cli/pkg/cmd/cmderrors" - "github.com/brevdev/brev-cli/pkg/cmd/exec" "github.com/brevdev/brev-cli/pkg/errors" ) @@ -18,9 +17,8 @@ func main() { command := cmd.NewDefaultBrevCommand() if err := command.Execute(); err != nil { - // A remote command exiting non-zero is not a CLI error: pass its exit - // code through so callers can branch on it, and print nothing extra. - var remoteErr exec.RemoteExitError + // Not a CLI error: pass the remote command's exit code straight through. + var remoteErr errors.RemoteExitError if stderrors.As(err, &remoteErr) { done() os.Exit(remoteErr.Code) //nolint:gocritic // manually call done diff --git a/pkg/cmd/exec/exec.go b/pkg/cmd/exec/exec.go index 2b16fea49..1cfa7f6b1 100644 --- a/pkg/cmd/exec/exec.go +++ b/pkg/cmd/exec/exec.go @@ -88,9 +88,9 @@ func NewCmdExec(t *terminal.Terminal, store ExecStore, noLoginStartStore ExecSto return breverrors.NewValidationError("command is required") } - // Heads-up only: exec can still succeed logged out if the SSH config is warm. - if isLoggedOut(store) { - fmt.Fprintf(os.Stderr, "You are logged out. Trying with your existing SSH config; you'll be prompted to log in if it fails.\n") + // Heads-up only: exec can still succeed without credentials if the SSH config is warm. + if hasNoSavedCredentials(store) { + fmt.Fprintf(os.Stderr, "No saved Brev credentials. Trying with your existing SSH config; you'll be prompted to log in if it fails.\n") } // Run on each instance @@ -179,9 +179,7 @@ func parseCommand(command string) (string, error) { return command, nil } -// flattenMultiInstanceErr drops the error types from a multi-instance failure. -// One remote exit code cannot represent several instances, so the process exits -// 1 instead of picking whichever RemoteExitError happens to be first. +// flattenMultiInstanceErr drops error types so one instance's exit code can't become the process's. func flattenMultiInstanceErr(err error) error { if err == nil { return nil @@ -193,8 +191,8 @@ type authTokenGetter interface { GetAuthTokens() (*entity.AuthTokens, error) } -// isLoggedOut reports whether no credentials are saved. Local file read, no network. -func isLoggedOut(sstore authTokenGetter) bool { +// hasNoSavedCredentials reports whether credentials are missing or empty; it does not validate them. +func hasNoSavedCredentials(sstore authTokenGetter) bool { tokens, err := sstore.GetAuthTokens() if err != nil { var notFound *breverrors.CredentialsFileNotFound @@ -208,21 +206,9 @@ func isLoggedOut(sstore authTokenGetter) bool { const pollTimeout = 10 * time.Minute -// sshConnectionFailedExitCode is the exit code ssh reserves for its own -// failures (unreachable host, auth rejected). Any other non-zero code is the -// remote command's own exit status, which means the connection worked. +// sshConnectionFailedExitCode is ssh's own failure code; any other code is the remote command's. const sshConnectionFailedExitCode = 255 -// RemoteExitError means we connected and ran the command successfully, and the -// command itself exited non-zero. It is not a connection failure. -type RemoteExitError struct { - Code int -} - -func (e RemoteExitError) Error() string { - return fmt.Sprintf("command exited with status %d", e.Code) -} - // exitCodeOf returns the process exit code for err, or -1 if err is not an exit error. func exitCodeOf(err error) int { var exitErr *exec.ExitError @@ -250,11 +236,9 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st return nil } - // We connected fine and the command exited non-zero. Surface its exit code - // rather than treating a healthy connection as a failure. - var remoteErr RemoteExitError + // The connection worked and the command itself failed, so skip the recovery path. + var remoteErr breverrors.RemoteExitError if stderrors.As(err, &remoteErr) { - go trackExecAnalytics(sstore, workspaceNameOrID) return remoteErr } @@ -325,8 +309,7 @@ func runExecCommand(t *terminal.Terminal, sstore ExecStore, workspaceNameOrID st return runAndTrack(sstore, sshName, workspaceNameOrID, command) } -// runAndTrack runs the command after recovery. A non-zero exit from the remote -// command is returned as RemoteExitError, not as a connection failure. +// runAndTrack runs the command after recovery. func runAndTrack(sstore ExecStore, sshName string, workspaceNameOrID string, command string) error { err := runSSH(sshName, command) if err == nil { @@ -334,9 +317,8 @@ func runAndTrack(sstore ExecStore, sshName string, workspaceNameOrID string, com return nil } - var remoteErr RemoteExitError + var remoteErr breverrors.RemoteExitError if stderrors.As(err, &remoteErr) { - go trackExecAnalytics(sstore, workspaceNameOrID) return remoteErr } return breverrors.WrapAndTrace(err) @@ -369,8 +351,7 @@ func runSSHWithTimeout(sshAlias string, command string, connectTimeoutSecs int) // -T disables pseudo-terminal allocation (no "Pseudo-terminal will not be allocated" warning) // Only start ssh-agent if one isn't already running (avoids orphaned agent processes) agentCmd := `if [ -z "$SSH_AUTH_SOCK" ]; then eval $(ssh-agent -s) > /dev/null; fi` - // `exec` replaces bash with ssh so the exit code we observe is ssh's own, - // not bash's, which keeps the 255 check below meaningful. + // exec replaces bash with ssh so the exit code we see is ssh's own, not bash's. cmd := fmt.Sprintf("%s && exec ssh -T -o ConnectTimeout=%d -o LogLevel=ERROR %s '%s'", agentCmd, connectTimeoutSecs, sshAlias, escapedCmd) sshCmd := exec.Command("bash", "-c", cmd) //nolint:gosec //cmd is user input @@ -381,16 +362,13 @@ func runSSHWithTimeout(sshAlias string, command string, connectTimeoutSecs int) return classifySSHError(sshCmd.Run()) } -// classifySSHError maps an ssh process error to a RemoteExitError when the code -// came from the remote command, or a wrapped error when ssh itself failed. +// classifySSHError separates a remote command failure from an ssh connection failure. func classifySSHError(err error) error { if err == nil { return nil } - // ssh uses 255 for its own failures; any other code came from the remote - // command, which means the connection itself was fine. if code := exitCodeOf(err); code > 0 && code != sshConnectionFailedExitCode { - return RemoteExitError{Code: code} + return breverrors.RemoteExitError{Code: code} } return breverrors.WrapAndTrace(err) } diff --git a/pkg/cmd/exec/exec_test.go b/pkg/cmd/exec/exec_test.go index 487421832..e007d2e6a 100644 --- a/pkg/cmd/exec/exec_test.go +++ b/pkg/cmd/exec/exec_test.go @@ -27,14 +27,14 @@ func TestClassifySSHError(t *testing.T) { // A remote command's own exit code means the connection worked. for _, code := range []int{1, 2, 7, 127} { - var remoteErr RemoteExitError + var remoteErr breverrors.RemoteExitError err := classifySSHError(exitErrWithCode(t, code)) require.True(t, stderrors.As(err, &remoteErr), "code %d should be a RemoteExitError", code) assert.Equal(t, code, remoteErr.Code) } // 255 is ssh's own failure code, so it must stay a connection error. - var remoteErr RemoteExitError + var remoteErr breverrors.RemoteExitError err := classifySSHError(exitErrWithCode(t, sshConnectionFailedExitCode)) require.Error(t, err) assert.False(t, stderrors.As(err, &remoteErr), "255 must not be treated as a remote exit") @@ -50,10 +50,10 @@ func TestExitCodeOf(t *testing.T) { func TestFlattenMultiInstanceErr(t *testing.T) { assert.NoError(t, flattenMultiInstanceErr(nil)) - agg := multierror.Append(nil, RemoteExitError{Code: 3}, RemoteExitError{Code: 7}) + agg := multierror.Append(nil, breverrors.RemoteExitError{Code: 3}, breverrors.RemoteExitError{Code: 7}) flat := flattenMultiInstanceErr(agg) - var remoteErr RemoteExitError + var remoteErr breverrors.RemoteExitError require.Error(t, flat) assert.False(t, stderrors.As(flat, &remoteErr), "no exit code may leak from a multi-instance run") // The per-instance detail is still readable in the message. @@ -68,7 +68,7 @@ type fakeTokenStore struct { func (f fakeTokenStore) GetAuthTokens() (*entity.AuthTokens, error) { return f.tokens, f.err } -func TestIsLoggedOut(t *testing.T) { +func TestHasNoSavedCredentials(t *testing.T) { cases := map[string]struct { store fakeTokenStore want bool @@ -80,31 +80,30 @@ func TestIsLoggedOut(t *testing.T) { "has access token": {fakeTokenStore{tokens: &entity.AuthTokens{AccessToken: "a"}}, false}, "has refresh token": {fakeTokenStore{tokens: &entity.AuthTokens{RefreshToken: "r"}}, false}, "has api key": {fakeTokenStore{tokens: &entity.AuthTokens{APIKey: "k"}}, false}, - // An unrelated read error must not claim the user is logged out. + // An unrelated read error must not be reported as missing credentials. "other error": {fakeTokenStore{err: stderrors.New("permission denied")}, false}, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - assert.Equal(t, tc.want, isLoggedOut(tc.store)) + assert.Equal(t, tc.want, hasNoSavedCredentials(tc.store)) }) } } -// RemoteExitError must survive the wrapping every return path applies, because -// main.go recovers the exit code with errors.As. +// RemoteExitError must survive wrapping, since main.go recovers the code with errors.As. func TestRemoteExitErrorSurvivesWrapping(t *testing.T) { cases := map[string]error{ - "unwrapped": RemoteExitError{Code: 4}, - "wrapped": breverrors.WrapAndTrace(RemoteExitError{Code: 4}), - "doubleWrap": breverrors.WrapAndTrace(breverrors.WrapAndTrace(RemoteExitError{Code: 4})), - "multierror": multierror.Append(nil, RemoteExitError{Code: 4}), - "multiWrapped": multierror.Append(nil, breverrors.WrapAndTrace(RemoteExitError{Code: 4})), + "unwrapped": breverrors.RemoteExitError{Code: 4}, + "wrapped": breverrors.WrapAndTrace(breverrors.RemoteExitError{Code: 4}), + "doubleWrap": breverrors.WrapAndTrace(breverrors.WrapAndTrace(breverrors.RemoteExitError{Code: 4})), + "multierror": multierror.Append(nil, breverrors.RemoteExitError{Code: 4}), + "multiWrapped": multierror.Append(nil, breverrors.WrapAndTrace(breverrors.RemoteExitError{Code: 4})), } for name, err := range cases { t.Run(name, func(t *testing.T) { - var remoteErr RemoteExitError + var remoteErr breverrors.RemoteExitError require.True(t, stderrors.As(err, &remoteErr), "errors.As must match through %s", name) assert.Equal(t, 4, remoteErr.Code) }) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 6579aacea..21d345615 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -295,3 +295,12 @@ func (e NvidiaMigrationError) Directive() string { func NewNvidiaMigrationError(msg string) *NvidiaMigrationError { return &NvidiaMigrationError{Message: msg} } + +// RemoteExitError means a command ran over SSH and exited non-zero; Code is its exit status. +type RemoteExitError struct { + Code int +} + +func (e RemoteExitError) Error() string { + return fmt.Sprintf("command exited with status %d", e.Code) +}