diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index f2e569c609..b0e1a5121b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -5,6 +5,7 @@ ### Notable Changes ### CLI +* Ctrl+C in an interactive prompt now prints `cancelled` and exits 130 instead of `Error: user aborted` / `Error: ^C` and exit 1. Applies to all `huh`-based prompts (e.g. `databricks aitools`, `databricks apps init`) and the bubbletea-based prompts in `libs/cmdio`. ### Bundles * The error reported when a direct-only resource (catalogs, external locations, vector search endpoints) is used with the terraform engine now also suggests setting `bundle.engine: direct` in `databricks.yml`, in addition to the `DATABRICKS_BUNDLE_ENGINE` environment variable ([#5295](https://github.com/databricks/cli/pull/5295)). diff --git a/cmd/root/root.go b/cmd/root/root.go index 6b6de2a9ba..3e4c87c8d7 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -8,9 +8,11 @@ import ( "os" "runtime" "runtime/debug" + "strconv" "strings" "time" + "github.com/charmbracelet/huh" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/cmdctx" @@ -22,6 +24,32 @@ import ( "github.com/spf13/cobra" ) +// ExitInterrupted is the exit code main.go uses when the user cancelled an +// interactive prompt with Ctrl+C. Matches the POSIX 128+SIGINT convention so +// shell scripts can distinguish a user cancel from a genuine command failure. +const ExitInterrupted = 130 + +// IsInterrupted reports whether err indicates the user cancelled an +// interactive prompt with Ctrl+C. Covers both cmdio's TUI prompts and the +// huh library used by aitools. main.go reads this to pick an exit code. +func IsInterrupted(err error) bool { + return errors.Is(err, cmdio.ErrInterrupted) || errors.Is(err, huh.ErrUserAborted) +} + +// ExitCodeFor maps the result of Execute to a process exit code. +// 0 = success, ExitInterrupted (130) = user Ctrl+C, 1 = any other error. +// Single source of truth shared between Execute's telemetry and main.go. +func ExitCodeFor(err error) int { + switch { + case err == nil: + return 0 + case IsInterrupted(err): + return ExitInterrupted + default: + return 1 + } +} + func New(ctx context.Context) *cobra.Command { cmd := &cobra.Command{ Use: "databricks", @@ -144,7 +172,14 @@ Stack Trace: // Run the command cmd, err = cmd.ExecuteContextC(ctx) - if err != nil && !errors.Is(err, ErrAlreadyPrinted) { + interrupted := IsInterrupted(err) + switch { + case err == nil, errors.Is(err, ErrAlreadyPrinted): + // ErrAlreadyPrinted wins over interrupted: a subcommand that + // printed its own cancel message should not be overridden. + case interrupted: + fmt.Fprintln(cmd.ErrOrStderr(), "cancelled") + default: if cmdctx.HasConfigUsed(cmd.Context()) { cfg := cmdctx.ConfigUsed(cmd.Context()) err = auth.EnrichAuthError(cmd.Context(), cfg, err) @@ -152,30 +187,27 @@ Stack Trace: fmt.Fprintf(cmd.ErrOrStderr(), "Error: %s\n", err.Error()) } + exitCode := ExitCodeFor(err) + // Log exit status and error // We only log if logger initialization succeeded and is stored in command // context if logger, ok := log.FromContext(cmd.Context()); ok { - if err == nil { - logger.Info("completed execution", - slog.String("exit_code", "0")) - } else if errors.Is(err, ErrAlreadyPrinted) { - logger.Debug("failed execution", - slog.String("exit_code", "1"), - ) - } else { + switch { + case err == nil: + logger.Info("completed execution", slog.String("exit_code", strconv.Itoa(exitCode))) + case interrupted: + logger.Info("cancelled execution", slog.String("exit_code", strconv.Itoa(exitCode))) + case errors.Is(err, ErrAlreadyPrinted): + logger.Debug("failed execution", slog.String("exit_code", strconv.Itoa(exitCode))) + default: logger.Info("failed execution", - slog.String("exit_code", "1"), + slog.String("exit_code", strconv.Itoa(exitCode)), slog.String("error", err.Error()), ) } } - exitCode := 0 - if err != nil { - exitCode = 1 - } - commandStr := commandString(cmd) ctx = cmd.Context() diff --git a/cmd/root/root_test.go b/cmd/root/root_test.go index 44a0707398..bb97e0a552 100644 --- a/cmd/root/root_test.go +++ b/cmd/root/root_test.go @@ -2,9 +2,13 @@ package root import ( "bytes" + "errors" + "fmt" "testing" + "github.com/charmbracelet/huh" "github.com/databricks/cli/libs/cmdctx" + "github.com/databricks/cli/libs/cmdio" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" @@ -77,6 +81,77 @@ func TestExecuteNoEnrichmentWithoutConfigUsed(t *testing.T) { assert.NotContains(t, output, "Next steps:") } +func TestIsInterrupted(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {"nil", nil, false}, + {"random error", errors.New("boom"), false}, + {"cmdio interrupt", cmdio.ErrInterrupted, true}, + {"huh aborted", huh.ErrUserAborted, true}, + {"wrapped cmdio interrupt", fmt.Errorf("prompt: %w", cmdio.ErrInterrupted), true}, + {"wrapped huh aborted", fmt.Errorf("form: %w", huh.ErrUserAborted), true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, IsInterrupted(tc.err)) + }) + } +} + +func TestExecuteInterruptPrintsCancelled(t *testing.T) { + tests := []struct { + name string + err error + }{ + {"cmdio interrupt", cmdio.ErrInterrupted}, + {"huh aborted", huh.ErrUserAborted}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + cmd := &cobra.Command{ + Use: "test", + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, args []string) error { return tc.err }, + } + cmd.SetErr(stderr) + + err := Execute(t.Context(), cmd) + require.Error(t, err) + assert.True(t, IsInterrupted(err)) + + output := stderr.String() + assert.Equal(t, "cancelled\n", output) + assert.NotContains(t, output, "Error:") + }) + } +} + +func TestExitCodeFor(t *testing.T) { + tests := []struct { + name string + err error + want int + }{ + {"nil", nil, 0}, + {"random error", errors.New("boom"), 1}, + {"already printed", ErrAlreadyPrinted, 1}, + {"cmdio interrupt", cmdio.ErrInterrupted, ExitInterrupted}, + {"huh aborted", huh.ErrUserAborted, ExitInterrupted}, + {"wrapped cmdio interrupt", fmt.Errorf("prompt: %w", cmdio.ErrInterrupted), ExitInterrupted}, + {"wrapped huh aborted", fmt.Errorf("form: %w", huh.ErrUserAborted), ExitInterrupted}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, ExitCodeFor(tc.err)) + }) + } +} + func TestExecuteErrAlreadyPrintedNotEnriched(t *testing.T) { ctx := t.Context() stderr := &bytes.Buffer{} diff --git a/libs/cmdio/io.go b/libs/cmdio/io.go index a25062fcd3..054ad16c58 100644 --- a/libs/cmdio/io.go +++ b/libs/cmdio/io.go @@ -11,9 +11,11 @@ import ( "github.com/databricks/cli/libs/flags" ) -// errCtrlC is returned when the user cancels a TUI prompt with Ctrl+C. The -// "^C" string matches the historical wire format; goldens depend on it. -var errCtrlC = errors.New("^C") +// ErrInterrupted is returned when the user cancels a TUI prompt with Ctrl+C. +// The "^C" string matches the historical wire format; goldens depend on it. +// Callers above the cobra layer (cmd/root) recognise this sentinel to print +// "cancelled" and exit with code 130 instead of an error stack. +var ErrInterrupted = errors.New("^C") // runTUI runs a tea.Program through cmdIO's tea program slot so spinners and // pagers can't fight a prompt for the terminal. Blocks until the model quits. @@ -21,7 +23,7 @@ func (c *cmdIO) runTUI(m tea.Model) (tea.Model, error) { p := tea.NewProgram(m, tea.WithInput(c.in), tea.WithOutput(c.err), - // Ctrl+C is delivered as a key event so the model can return errCtrlC. + // Ctrl+C is delivered as a key event so the model can return ErrInterrupted. tea.WithoutSignalHandler(), ) c.acquireTeaProgram(p) diff --git a/libs/cmdio/prompt.go b/libs/cmdio/prompt.go index 8c8ca1a3a3..ca6faf4282 100644 --- a/libs/cmdio/prompt.go +++ b/libs/cmdio/prompt.go @@ -239,7 +239,7 @@ func (c *cmdIO) runPromptModel(m *promptModel) (string, error) { pm := final.(*promptModel) switch { case pm.cancelled: - return "", errCtrlC + return "", ErrInterrupted case pm.deleted: return "", io.EOF } diff --git a/libs/cmdio/select.go b/libs/cmdio/select.go index 2d481139ba..b861eb93bc 100644 --- a/libs/cmdio/select.go +++ b/libs/cmdio/select.go @@ -521,7 +521,7 @@ func (c *cmdIO) runSelectModel(m *selectModel) (int, error) { } sm := final.(*selectModel) if sm.cancelled { - return 0, errCtrlC + return 0, ErrInterrupted } return sm.originalIndex(), nil } diff --git a/main.go b/main.go index e81dde6946..ccb886f8d9 100644 --- a/main.go +++ b/main.go @@ -15,7 +15,5 @@ import ( func main() { ctx := context.Background() err := root.Execute(ctx, cmd.New(ctx)) - if err != nil { - os.Exit(1) - } + os.Exit(root.ExitCodeFor(err)) }