Skip to content

Commit 08b8020

Browse files
matthewbordenampcode-commcncl
authored
Preflight specific exit codes (#732)
* Return dedicated preflight exit codes This introduces `cmd/preflight.Result` to classify the final preflight `build.State` and return typed CLI errors instead of a generic `fmt.Errorf` for every non-passing result. Previously `bk preflight` ended by returning plain string errors such as `preflight build failed`. `internal/errors/handler.go` treated those the same as any other generic failure, so scripts could not distinguish a completed failed build from an active failure, an incomplete build, or an unknown result. With this change, `Result.Error()` maps preflight outcomes onto dedicated error categories and `internal/errors/handler.go` assigns stable exit codes for each one: | `build.State` | Exit code | Meaning | | --- | ---: | --- | | `passed` | `0` | Successful preflight | | `failed` | `9` | Terminal preflight failure | | `canceled` | `9` | Terminal preflight failure | | `skipped` | `9` | Terminal preflight failure | | `not_run` | `9` | Terminal preflight failure | | `failing` | `10` | Active preflight failure | | `scheduled` | `11` | Incomplete preflight | | `running` | `11` | Incomplete preflight | | `blocked` | `11` | Incomplete preflight | | `canceling` | `11` | Incomplete preflight | | any other state | `12` | Unknown preflight result | This keeps `bk preflight` on the shared CLI error-handling path while giving preflight-specific prefixes, messages, and automation-friendly exit codes. Amp-Thread-ID: https://ampcode.com/threads/T-019d3d79-37bd-700b-8052-34e00ab1e618 Co-authored-by: Amp <amp@ampcode.com> * Restore rendering the summary of failures at the end of preflight * Add result kind for unknown kind error * Add additional line to preflight output To seperate cleanup from results * Show preflight build failed. * Update tests for Preflight build failed. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Ben McNicholl <ben.mcnicholl@buildkite.com>
1 parent b945534 commit 08b8020

16 files changed

Lines changed: 648 additions & 142 deletions

File tree

cmd/preflight/preflight.go

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/google/uuid"
1616
"github.com/mattn/go-isatty"
1717

18+
buildstate "github.com/buildkite/cli/v3/internal/build/state"
1819
"github.com/buildkite/cli/v3/internal/build/watch"
1920
"github.com/buildkite/cli/v3/internal/cli"
2021
bkErrors "github.com/buildkite/cli/v3/internal/errors"
@@ -35,6 +36,8 @@ type renderStatusError struct {
3536
err error
3637
}
3738

39+
var notifyContext = signal.NotifyContext
40+
3841
func (e renderStatusError) Error() string {
3942
return e.err.Error()
4043
}
@@ -81,7 +84,7 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
8184
return bkErrors.NewValidationError(fmt.Errorf("interval must be greater than 0"), "invalid polling interval")
8285
}
8386
// Resolve the pipeline to create a build against.
84-
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
87+
ctx, stop := notifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
8588
defer stop()
8689
resolvers := resolver.NewAggregateResolver(
8790
resolver.ResolveFromFlag(c.Pipeline, f.Config),
@@ -97,26 +100,23 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
97100
}
98101

99102
tty := isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd())
100-
renderer := newRenderer(os.Stdout, tty, resolvedPipeline.Name)
103+
snapshotOutput := []string{"Creating snapshot of working tree..."}
101104

102105
var opts []preflight.SnapshotOption
103106
if globals.EnableDebug() {
104107
opts = append(opts, preflight.WithDebug())
105108
}
106109

107-
renderer.appendSnapshotLine("Creating snapshot of working tree...")
108110
result, err := preflight.Snapshot(wt.Filesystem.Root(), preflightID, opts...)
109111
if err != nil {
110112
return bkErrors.NewSnapshotError(err, "failed to create preflight snapshot",
111113
"Ensure you have uncommitted or committed changes to snapshot",
112114
"Ensure you have push access to the remote repository",
113115
)
114116
}
115-
116-
renderer.setSnapshot(result)
117-
118-
renderer.appendSnapshotLine("")
119-
renderer.appendSnapshotLine(fmt.Sprintf("Creating build on %s/%s...", resolvedPipeline.Org, resolvedPipeline.Name))
117+
snapshotOutput = append(snapshotOutput, snapshotLines(result)...)
118+
snapshotOutput = append(snapshotOutput, "")
119+
snapshotOutput = append(snapshotOutput, fmt.Sprintf("Creating build on %s/%s...", resolvedPipeline.Org, resolvedPipeline.Name))
120120
build, _, err := f.RestAPIClient.Builds.Create(ctx, resolvedPipeline.Org, resolvedPipeline.Name, buildkite.CreateBuild{
121121
Message: fmt.Sprintf("Preflight %s", preflightID),
122122
Commit: result.Commit,
@@ -129,6 +129,10 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
129129
return bkErrors.WrapAPIError(err, "creating preflight build")
130130
}
131131

132+
renderer := newRenderer(os.Stdout, tty, resolvedPipeline.Name, build.Number)
133+
for _, line := range snapshotOutput {
134+
renderer.appendSnapshotLine(line)
135+
}
132136
renderer.appendSnapshotLine(fmt.Sprintf("Build: %s", build.WebURL))
133137

134138
if !c.Watch {
@@ -139,7 +143,7 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
139143
tracker := watch.NewJobTracker()
140144

141145
finalBuild, err := watch.WatchBuild(ctx, f.RestAPIClient, resolvedPipeline.Org, resolvedPipeline.Name, build.Number, interval, func(b buildkite.Build) error {
142-
if err := renderer.renderStatus(tracker.Update(b), b); err != nil {
146+
if err := renderer.renderStatus(tracker.Update(b), b.State); err != nil {
143147
return renderStatusError{err: err}
144148
}
145149
return nil
@@ -151,6 +155,14 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
151155
}
152156
}
153157

158+
// Flush the screen so final output is not overwritten.
159+
renderer.flush()
160+
161+
buildResult := NewResult(finalBuild)
162+
failedJobs := tracker.FailedJobs()
163+
finalErr := buildResult.Error()
164+
renderer.renderFinalFailures(buildResult, failedJobs)
165+
154166
if !c.NoCleanup {
155167
fmt.Fprintf(os.Stderr, "Cleaning up remote branch %s...\n", result.Branch)
156168
if cleanupErr := preflight.Cleanup(wt.Filesystem.Root(), result.Ref, globals.EnableDebug()); cleanupErr != nil {
@@ -161,7 +173,7 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
161173
}
162174

163175
if errors.Is(err, context.Canceled) {
164-
if finalBuild.FinishedAt == nil && !watch.IsTerminalBuildState(finalBuild.State) {
176+
if finalBuild.FinishedAt == nil && !buildstate.IsTerminal(buildstate.State(finalBuild.State)) {
165177
cancelCtx, cancelStop := context.WithTimeout(context.Background(), 5*time.Second)
166178
defer cancelStop()
167179
if _, cancelErr := f.RestAPIClient.Builds.Cancel(cancelCtx, resolvedPipeline.Org, resolvedPipeline.Name, strconv.Itoa(build.Number)); cancelErr != nil {
@@ -177,7 +189,7 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
177189
fmt.Fprintf(os.Stderr, "Cancelled build #%d\n", build.Number)
178190
}
179191
}
180-
return nil
192+
return bkErrors.NewUserAbortedError(context.Canceled, "preflight canceled by user")
181193
}
182194
if err != nil {
183195
return bkErrors.NewInternalError(err, "watching build failed",
@@ -186,14 +198,5 @@ func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error
186198
)
187199
}
188200

189-
// Flush the screen so final output is not overwritten.
190-
renderer.flush()
191-
192-
renderer.renderFinalFailures(tracker.AllFailed())
193-
194-
renderer.setResult(finalBuild.State)
195-
if finalBuild.State == "passed" {
196-
return nil
197-
}
198-
return fmt.Errorf("preflight build %s", finalBuild.State)
201+
return finalErr
199202
}

cmd/preflight/preflight_test.go

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package preflight
22

33
import (
4+
"context"
45
"encoding/json"
56
"errors"
67
"net/http"
@@ -9,6 +10,7 @@ import (
910
"os/exec"
1011
"path/filepath"
1112
"strings"
13+
"sync/atomic"
1214
"testing"
1315
"time"
1416

@@ -259,8 +261,72 @@ func TestPreflightCmd_Run(t *testing.T) {
259261
if err == nil {
260262
t.Fatal("expected error, got nil")
261263
}
262-
if !strings.Contains(err.Error(), "preflight build failed") {
263-
t.Errorf("expected 'preflight build failed', got: %v", err)
264+
if !strings.Contains(err.Error(), "preflight completed with failure: build is failed") {
265+
t.Errorf("expected completed failure error, got: %v", err)
266+
}
267+
})
268+
269+
t.Run("returns user aborted error when interrupted while watching", func(t *testing.T) {
270+
t.Setenv("BUILDKITE_EXPERIMENTS", "preflight")
271+
272+
originalNotifyContext := notifyContext
273+
t.Cleanup(func() { notifyContext = originalNotifyContext })
274+
275+
watchCtx, cancelWatch := context.WithCancel(context.Background())
276+
notifyContext = func(context.Context, ...os.Signal) (context.Context, context.CancelFunc) {
277+
return watchCtx, cancelWatch
278+
}
279+
280+
var buildCancelRequests atomic.Int32
281+
var pollCount atomic.Int32
282+
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
283+
w.Header().Set("Content-Type", "application/json")
284+
switch {
285+
case r.Method == "PUT" && strings.Contains(r.URL.Path, "/builds/1/cancel"):
286+
buildCancelRequests.Add(1)
287+
json.NewEncoder(w).Encode(buildkite.Build{Number: 1, State: "canceling"})
288+
return
289+
case r.Method == "POST" && strings.Contains(r.URL.Path, "/builds"):
290+
json.NewEncoder(w).Encode(buildkite.Build{
291+
Number: 1,
292+
State: "scheduled",
293+
WebURL: "https://buildkite.com/test-org/test-pipeline/builds/1",
294+
})
295+
return
296+
case r.Method == "GET" && strings.Contains(r.URL.Path, "/builds/1"):
297+
if pollCount.Add(1) == 1 {
298+
cancelWatch()
299+
}
300+
json.NewEncoder(w).Encode(buildkite.Build{Number: 1, State: "running"})
301+
return
302+
}
303+
http.NotFound(w, r)
304+
}))
305+
defer s.Close()
306+
t.Setenv("BUILDKITE_REST_API_ENDPOINT", s.URL)
307+
308+
worktree := initTestRepo(t)
309+
t.Chdir(worktree)
310+
if err := os.WriteFile(filepath.Join(worktree, "new.txt"), []byte("hello\n"), 0o644); err != nil {
311+
t.Fatal(err)
312+
}
313+
314+
cmd := &PreflightCmd{Pipeline: "test-org/test-pipeline", Watch: true, Interval: 0.01}
315+
err := cmd.Run(nil, stubGlobals{})
316+
if err == nil {
317+
t.Fatal("expected error, got nil")
318+
}
319+
if !bkErrors.IsUserAborted(err) {
320+
t.Fatalf("expected user aborted error, got %T: %v", err, err)
321+
}
322+
if code := bkErrors.GetExitCodeForError(err); code != bkErrors.ExitCodeUserAbortedError {
323+
t.Fatalf("expected exit code %d, got %d", bkErrors.ExitCodeUserAbortedError, code)
324+
}
325+
if pollCount.Load() == 0 {
326+
t.Fatal("expected at least one build poll before interrupt")
327+
}
328+
if buildCancelRequests.Load() != 1 {
329+
t.Fatalf("expected one build cancel request, got %d", buildCancelRequests.Load())
264330
}
265331
})
266332

cmd/preflight/render.go

Lines changed: 43 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,23 @@ import (
99

1010
"github.com/buildkite/cli/v3/internal/build/watch"
1111
internalpreflight "github.com/buildkite/cli/v3/internal/preflight"
12-
buildkite "github.com/buildkite/go-buildkite/v4"
1312
)
1413

1514
const maxTTYRunningJobs = 10
1615

1716
type renderer interface {
1817
appendSnapshotLine(string)
1918
setSnapshot(*internalpreflight.SnapshotResult)
20-
renderStatus(watch.BuildStatus, buildkite.Build) error
19+
renderStatus(watch.BuildStatus, string) error
2120
flush()
22-
renderFinalFailures([]buildkite.Job)
23-
setResult(string)
21+
renderFinalFailures(Result, watch.FailedJobs)
2422
}
2523

26-
func newRenderer(stdout *os.File, tty bool, pipeline string) renderer {
24+
func newRenderer(stdout *os.File, tty bool, pipeline string, buildNumber int) renderer {
2725
if tty {
28-
return newTTYRenderer(stdout, pipeline)
26+
return newTTYRenderer(stdout, pipeline, buildNumber)
2927
}
30-
return newPlainRenderer(stdout, pipeline)
28+
return newPlainRenderer(stdout, pipeline, buildNumber)
3129
}
3230

3331
type ttyRenderer struct {
@@ -41,10 +39,11 @@ type ttyRenderer struct {
4139
resultRegion *internalpreflight.Region
4240
}
4341

44-
func newTTYRenderer(stdout *os.File, pipeline string) *ttyRenderer {
42+
func newTTYRenderer(stdout *os.File, pipeline string, buildNumber int) *ttyRenderer {
4543
screen := internalpreflight.NewScreen(stdout)
4644
return &ttyRenderer{
4745
pipeline: pipeline,
46+
buildNumber: buildNumber,
4847
screen: screen,
4948
snapshotRegion: screen.AddRegion("snapshot"),
5049
titleRegion: screen.AddRegion("title"),
@@ -62,12 +61,15 @@ func (r *ttyRenderer) setSnapshot(result *internalpreflight.SnapshotResult) {
6261
r.snapshotRegion.SetLines(snapshotLines(result))
6362
}
6463

65-
func (r *ttyRenderer) renderStatus(status watch.BuildStatus, b buildkite.Build) error {
66-
r.buildNumber = b.Number
64+
func (r *ttyRenderer) renderStatus(status watch.BuildStatus, buildState string) error {
6765
var presenter jobPresenter = ttyJobPresenter{pipeline: r.pipeline, buildNumber: r.buildNumber}
66+
title := fmt.Sprintf(" %s Watching build #%d…", spinner(), r.buildNumber)
67+
if buildState != "" {
68+
title += " (" + buildState + ")"
69+
}
6870
r.titleRegion.SetLines([]string{
6971
"",
70-
fmt.Sprintf(" %s Watching build #%d…", spinner(), b.Number),
72+
title,
7173
"",
7274
})
7375
for _, failed := range status.NewlyFailed {
@@ -94,14 +96,9 @@ func (r *ttyRenderer) flush() {
9496
r.screen.Flush()
9597
}
9698

97-
func (r *ttyRenderer) renderFinalFailures(_ []buildkite.Job) {}
98-
99-
func (r *ttyRenderer) setResult(state string) {
100-
if state == "passed" {
101-
r.resultRegion.SetLines([]string{"", "✅ Preflight passed!"})
102-
return
103-
}
104-
r.resultRegion.SetLines([]string{"", fmt.Sprintf("❌ Preflight %s", state)})
99+
func (r *ttyRenderer) renderFinalFailures(result Result, failedJobs watch.FailedJobs) {
100+
var presenter jobPresenter = ttyJobPresenter{pipeline: r.pipeline, buildNumber: r.buildNumber}
101+
r.resultRegion.SetLines(finalResultLines(result, failedJobs, presenter))
105102
}
106103

107104
type plainRenderer struct {
@@ -111,8 +108,8 @@ type plainRenderer struct {
111108
lastLine string
112109
}
113110

114-
func newPlainRenderer(stdout io.Writer, pipeline string) *plainRenderer {
115-
return &plainRenderer{stdout: stdout, pipeline: pipeline}
111+
func newPlainRenderer(stdout io.Writer, pipeline string, buildNumber int) *plainRenderer {
112+
return &plainRenderer{stdout: stdout, pipeline: pipeline, buildNumber: buildNumber}
116113
}
117114

118115
func (r *plainRenderer) appendSnapshotLine(line string) {
@@ -125,16 +122,15 @@ func (r *plainRenderer) setSnapshot(result *internalpreflight.SnapshotResult) {
125122
}
126123
}
127124

128-
func (r *plainRenderer) renderStatus(status watch.BuildStatus, b buildkite.Build) error {
129-
r.buildNumber = b.Number
125+
func (r *plainRenderer) renderStatus(status watch.BuildStatus, buildState string) error {
130126
var presenter jobPresenter = plainJobPresenter{pipeline: r.pipeline, buildNumber: r.buildNumber}
131127
for _, failed := range status.NewlyFailed {
132128
if _, err := fmt.Fprintf(r.stdout, "%s\n", presenter.Line(failed)); err != nil {
133129
return err
134130
}
135131
}
136132

137-
line := fmt.Sprintf("Build #%d %s", b.Number, b.State)
133+
line := fmt.Sprintf("Build #%d %s", r.buildNumber, buildState)
138134
if summary := status.Summary.String(); summary != "" {
139135
line += " — " + summary
140136
}
@@ -149,41 +145,40 @@ func (r *plainRenderer) renderStatus(status watch.BuildStatus, b buildkite.Build
149145

150146
func (r *plainRenderer) flush() {}
151147

152-
func (r *plainRenderer) renderFinalFailures(allFailed []buildkite.Job) {
153-
var hardFailed, softFailed []buildkite.Job
148+
func (r *plainRenderer) renderFinalFailures(result Result, failedJobs watch.FailedJobs) {
154149
var presenter jobPresenter = plainJobPresenter{pipeline: r.pipeline, buildNumber: r.buildNumber, final: true}
155-
for _, rawJob := range allFailed {
156-
job := watch.NewFormattedJob(rawJob)
157-
if job.IsSoftFailed() {
158-
softFailed = append(softFailed, rawJob)
159-
} else {
160-
hardFailed = append(hardFailed, rawJob)
161-
}
150+
for _, line := range finalResultLines(result, failedJobs, presenter) {
151+
fmt.Fprintln(r.stdout, line)
162152
}
153+
}
154+
155+
func finalResultLines(result Result, failedJobs watch.FailedJobs, presenter jobPresenter) []string {
156+
lines := []string{"", resultStatusLine(result)}
163157

164-
if len(hardFailed) > 0 {
165-
fmt.Fprintln(r.stdout)
166-
fmt.Fprintf(r.stdout, "Failed jobs (%d):\n", len(hardFailed))
167-
for _, rawJob := range hardFailed {
168-
fmt.Fprintf(r.stdout, "%s\n", presenter.Line(rawJob))
158+
if len(failedJobs.Hard) > 0 {
159+
lines = append(lines, "", fmt.Sprintf("Failed jobs (%d):", len(failedJobs.Hard)))
160+
for _, rawJob := range failedJobs.Hard {
161+
lines = append(lines, presenter.Line(rawJob))
169162
}
170163
}
171164

172-
if len(softFailed) > 0 {
173-
fmt.Fprintln(r.stdout)
174-
fmt.Fprintf(r.stdout, "Soft failed jobs (%d):\n", len(softFailed))
175-
for _, rawJob := range softFailed {
176-
fmt.Fprintf(r.stdout, "%s\n", presenter.Line(rawJob))
165+
if len(failedJobs.Soft) > 0 {
166+
lines = append(lines, "", fmt.Sprintf("Soft failed jobs (%d):", len(failedJobs.Soft)))
167+
for _, rawJob := range failedJobs.Soft {
168+
lines = append(lines, presenter.Line(rawJob))
177169
}
178170
}
171+
172+
lines = append(lines, "")
173+
174+
return lines
179175
}
180176

181-
func (r *plainRenderer) setResult(state string) {
182-
if state == "passed" {
183-
fmt.Fprintln(r.stdout, "✅ Preflight passed!")
184-
return
177+
func resultStatusLine(result Result) string {
178+
if result.kind == resultCompletedPass {
179+
return "✅ Preflight build passed."
185180
}
186-
fmt.Fprintf(r.stdout, "❌ Preflight %s\n", state)
181+
return fmt.Sprintf("❌ Preflight build %s.", result.buildState)
187182
}
188183

189184
func snapshotLines(result *internalpreflight.SnapshotResult) []string {

0 commit comments

Comments
 (0)