Skip to content

Commit cb76342

Browse files
committed
fix: only pipe output to stdout on full success
Address drewmalin's review: don't send names to stdout when there are partial failures, since the next piped command would act on them even without pipefail. Also send admin delete message to stderr instead of stdout, and remove unused ExitCodeError type.
1 parent 4336440 commit cb76342

5 files changed

Lines changed: 15 additions & 49 deletions

File tree

main.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package main
22

33
import (
4-
stderrors "errors"
54
"os"
65

76
"github.com/brevdev/brev-cli/pkg/cmd"
@@ -17,11 +16,6 @@ func main() {
1716
if err := command.Execute(); err != nil {
1817
cmderrors.DisplayAndHandleError(err)
1918
done()
20-
exitCode := 1
21-
var exitErr errors.ExitCodeError
22-
if stderrors.As(err, &exitErr) {
23-
exitCode = exitErr.ExitCode
24-
}
25-
os.Exit(exitCode) //nolint:gocritic // manually call done
19+
os.Exit(1) //nolint:gocritic // manually call done
2620
}
2721
}

pkg/cmd/delete/delete.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package delete
33
import (
44
_ "embed"
55
"fmt"
6+
"os"
67
"strings"
78

89
"github.com/brevdev/brev-cli/pkg/cmd/completions"
@@ -52,15 +53,15 @@ func NewCmdDelete(t *terminal.Terminal, loginDeleteStore DeleteStore, noLoginDel
5253
deletedNames = append(deletedNames, workspace)
5354
}
5455
}
55-
// Output names for piping to next command
56+
if allError != nil {
57+
return breverrors.WrapAndTrace(allError)
58+
}
59+
// Only output names for piping if all succeeded
5660
if piped {
5761
for _, name := range deletedNames {
5862
fmt.Println(name)
5963
}
6064
}
61-
if allError != nil {
62-
return breverrors.WrapAndTrace(allError)
63-
}
6465
return nil
6566
},
6667
}
@@ -106,7 +107,7 @@ func handleAdminUser(err error, deleteStore DeleteStore, piped bool) error {
106107
return breverrors.WrapAndTrace(err)
107108
}
108109
if !piped {
109-
fmt.Println("attempting to delete an instance you don't own as admin")
110+
fmt.Fprintln(os.Stderr, "attempting to delete an instance you don't own as admin")
110111
}
111112
return nil
112113
}

pkg/cmd/start/start.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -701,19 +701,15 @@ func runBatchStart(t *terminal.Terminal, names []string, org, setupScript, setup
701701
startedNames = append(startedNames, instanceName)
702702
}
703703
}
704-
// Always output successful names for piping to next command
704+
if errs != nil {
705+
return breverrors.WrapAndTrace(errs)
706+
}
707+
// Only output names for piping if all succeeded
705708
if piped {
706709
for _, n := range startedNames {
707710
fmt.Println(n)
708711
}
709712
}
710-
if errs != nil {
711-
exitCode := 1 // all failed
712-
if len(startedNames) > 0 {
713-
exitCode = 2 // partial failure
714-
}
715-
return breverrors.NewExitCodeError(errs, exitCode)
716-
}
717713
return nil
718714
}
719715

pkg/cmd/stop/stop.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,15 @@ func NewCmdStop(t *terminal.Terminal, loginStopStore StopStore, noLoginStopStore
6161
stoppedNames = append(stoppedNames, name)
6262
}
6363
}
64-
// Output names for piping to next command
64+
if allErr != nil {
65+
return breverrors.WrapAndTrace(allErr)
66+
}
67+
// Only output names for piping if all succeeded
6568
if piped {
6669
for _, name := range stoppedNames {
6770
fmt.Println(name)
6871
}
6972
}
70-
if allErr != nil {
71-
exitCode := 1 // all failed
72-
if len(stoppedNames) > 0 {
73-
exitCode = 2 // partial failure
74-
}
75-
return breverrors.NewExitCodeError(allErr, exitCode)
76-
}
7773
}
7874
return nil
7975
},

pkg/errors/errors.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -123,27 +123,6 @@ func (v ValidationError) Error() string {
123123
return v.Message
124124
}
125125

126-
// ExitCodeError wraps an error with a specific process exit code.
127-
// Use this to distinguish between failure modes (e.g., partial vs full failure).
128-
type ExitCodeError struct {
129-
Err error
130-
ExitCode int
131-
}
132-
133-
func NewExitCodeError(err error, exitCode int) ExitCodeError {
134-
return ExitCodeError{Err: err, ExitCode: exitCode}
135-
}
136-
137-
var _ error = ExitCodeError{}
138-
139-
func (e ExitCodeError) Error() string {
140-
return e.Err.Error()
141-
}
142-
143-
func (e ExitCodeError) Unwrap() error {
144-
return e.Err
145-
}
146-
147126
type DeclineToLoginError struct{}
148127

149128
func (d *DeclineToLoginError) Error() string { return "declined to login" }

0 commit comments

Comments
 (0)