Merge pull request #320 from overmindtech/improve-fatal-errors

Improve fatal errors; improve display of messages around commands

cmd/tea.go

@@ -50,7 +50,7 @@ type FinalReportingModel interface {
 	FinalReport() string
 }
 
-func CmdWrapper(action string, requiredScopes []string, commandModel func([]string) tea.Model) func(cmd *cobra.Command, args []string) {
+func CmdWrapper(action string, requiredScopes []string, commandModel func(args []string, execCommandFunc ExecCommandFunc) tea.Model) func(cmd *cobra.Command, args []string) {
 	return func(cmd *cobra.Command, args []string) {
 		// set up a context for the command
 		ctx, cancel := context.WithCancel(context.Background())
@@ -95,7 +95,7 @@ func CmdWrapper(action string, requiredScopes []string, commandModel func([]stri
 				return err
 			}
 
-			p := tea.NewProgram(cmdModel{
+			m := cmdModel{
 				action:         action,
 				ctx:            ctx,
 				cancel:         cancel,
@@ -104,14 +104,15 @@ func CmdWrapper(action string, requiredScopes []string, commandModel func([]stri
 				requiredScopes: requiredScopes,
 				apiKey:         viper.GetString("api-key"),
 				tasks:          map[string]tea.Model{},
-				cmd:            commandModel(args),
-			})
+			}
+			m.cmd = commandModel(args, m.NewExecCommand)
+			p := tea.NewProgram(&m)
 			result, err := p.Run()
 			if err != nil {
 				return fmt.Errorf("could not start program: %w", err)
 			}
 
-			cmd, ok := result.(cmdModel)
+			cmd, ok := result.(*cmdModel)
 			if ok {
 				frm, ok := cmd.cmd.(FinalReportingModel)
 				if ok {

cmd/tea_ensuretoken.go

@@ -96,7 +96,7 @@ func (m ensureTokenModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	case displayAuthorizationInstructionsMsg:
 		m.config = msg.config
 		m.deviceCode = msg.deviceCode
-
+		m.status = taskStatusDone // avoid console flickering to allow click to be registered
 		m.title = "Manual device authorization."
 		beginAuthMessage := `# Authenticate with a browser
 
@@ -133,26 +133,23 @@ Then enter the code:
 	case tokenLoadedMsg:
 		m.status = taskStatusDone
 		m.title = "Using stored token"
+		m.deviceMessage = ""
 		return m, m.tokenAvailable(msg.token)
 	case tokenReceivedMsg:
 		m.status = taskStatusDone
 		m.title = "Authentication successful, using API key"
+		m.deviceMessage = ""
 		return m, m.tokenAvailable(msg.token)
 	case tokenStoredMsg:
 		m.status = taskStatusDone
 		m.title = fmt.Sprintf("Authentication successful, token stored locally (%v)", msg.file)
+		m.deviceMessage = ""
 		return m, m.tokenAvailable(msg.token)
 	case otherError:
 		if msg.id == m.spinner.ID() {
 			m.errors = append(m.errors, fmt.Sprintf("Note: %v", msg.err))
 		}
 		return m, nil
-	case fatalError:
-		if msg.id == m.spinner.ID() {
-			m.status = taskStatusError
-			m.title = markdownToString(fmt.Sprintf("Ensuring Token Error: %v", msg.err))
-		}
-		return m, nil
 	default:
 		var taskCmd tea.Cmd
 		m.taskModel, taskCmd = m.taskModel.Update(msg)
@@ -165,7 +162,7 @@ func (m ensureTokenModel) View() string {
 	if len(m.errors) > 0 {
 		view += fmt.Sprintf("\n%v\n", strings.Join(m.errors, "\n"))
 	}
-	if m.deviceMessage != "" && !(m.status == taskStatusDone || m.status == taskStatusError) {
+	if m.deviceMessage != "" {
 		view += fmt.Sprintf("\n%v\n", m.deviceMessage)
 	}
 	return view
@@ -272,32 +269,41 @@ func (m ensureTokenModel) awaitTokenCmd() tea.Msg {
 		defer cancel()
 	}
 
-	// while the RFC requires the oauth2 library to use 5 as the default,
-	// Auth0 should be able to handle more. Hence we re-implement the
+	// while the RFC requires the oauth2 library to use 5 as the default, Auth0
+	// should be able to handle more. Hence we re-implement the
 	m.deviceCode.Interval = 1
 
 	var token *oauth2.Token
 	var err error
 	for {
+		log.Trace("attempting to get token from auth0")
 		// reset the deviceCode's expiry to at most 1.5 seconds
 		m.deviceCode.Expiry = time.Now().Add(1500 * time.Millisecond)
 
 		token, err = m.config.DeviceAccessToken(ctx, m.deviceCode)
 		if err == nil {
 			// we got a token, continue below. kthxbye
+			log.Trace("we got a token from auth0")
 			break
 		}
 
-		if errors.Is(err, context.DeadlineExceeded) {
+		// See https://github.com/golang/oauth2/issues/635,
+		// https://github.com/golang/oauth2/pull/636,
+		// https://go-review.googlesource.com/c/oauth2/+/476316
+		if errors.Is(err, context.DeadlineExceeded) || strings.HasSuffix(err.Error(), "context deadline exceeded") {
 			// the context has expired, we need to retry
+			log.WithError(err).Trace("context.DeadlineExceeded - waiting for a second")
+			time.Sleep(time.Second)
 			continue
 		}
 
 		// re-implement DeviceAccessToken's logic, but faster
-		e, ok := err.(*oauth2.RetrieveError) // nolint:errorlint // we depend on DeviceAccessToken() returning an non-wrapped error
-		if !ok {
+		e, isRetrieveError := err.(*oauth2.RetrieveError) // nolint:errorlint // we depend on DeviceAccessToken() returning an non-wrapped error
+		if !isRetrieveError {
+			log.WithError(err).Trace("error authorizing token")
 			return fatalError{id: m.spinner.ID(), err: fmt.Errorf("error authorizing token: %w", err)}
 		}
+
 		switch e.ErrorCode {
 		case "slow_down":
 			// // https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
@@ -310,7 +316,6 @@ func (m ensureTokenModel) awaitTokenCmd() tea.Msg {
 		default:
 			return fatalError{id: m.spinner.ID(), err: fmt.Errorf("error authorizing token (%v): %w", e.ErrorCode, err)}
 		}
-
 	}
 
 	span := trace.SpanFromContext(m.ctx)

cmd/tea_execcommand.go

@@ -0,0 +1,66 @@
+package cmd
+
+import (
+	"fmt"
+	"io"
+	"os/exec"
+
+	tea "github.com/charmbracelet/bubbletea"
+)
+
+type ExecCommandFunc func(cmd *exec.Cmd) tea.ExecCommand
+
+// NewExecCommand returns a new ExecCommand that will print the last view from
+// the parent cmdModel after bubbletea has released the terminal, but before the
+// command is run.
+func (m *cmdModel) NewExecCommand(c *exec.Cmd) tea.ExecCommand {
+	return NewExecCommand(m, c)
+}
+
+func NewExecCommand(parent *cmdModel, c *exec.Cmd) *cliExecCommandModel {
+	return &cliExecCommandModel{
+		parent: parent,
+		Cmd:    c,
+	}
+}
+
+// osExecCommand is a layer over an exec.Cmd that satisfies the ExecCommand
+// interface. It prints the last view from
+// the parent cmdModel after bubbletea has released the terminal, but before the
+// command is run.
+type cliExecCommandModel struct {
+	parent *cmdModel
+	*exec.Cmd
+}
+
+func (c cliExecCommandModel) Run() error {
+	_, err := c.Stdout.Write([]byte(c.parent.frozenView))
+	if err != nil {
+		return fmt.Errorf("failed to write view to stdout: %w", err)
+	}
+	return c.Cmd.Run()
+}
+
+// SetStdin sets stdin on underlying exec.Cmd to the given io.Reader.
+func (c *cliExecCommandModel) SetStdin(r io.Reader) {
+	// If unset, have the command use the same input as the terminal.
+	if c.Stdin == nil {
+		c.Stdin = r
+	}
+}
+
+// SetStdout sets stdout on underlying exec.Cmd to the given io.Writer.
+func (c *cliExecCommandModel) SetStdout(w io.Writer) {
+	// If unset, have the command use the same output as the terminal.
+	if c.Stdout == nil {
+		c.Stdout = w
+	}
+}
+
+// SetStderr sets stderr on the underlying exec.Cmd to the given io.Writer.
+func (c *cliExecCommandModel) SetStderr(w io.Writer) {
+	// If unset, use stderr for the command's stderr
+	if c.Stderr == nil {
+		c.Stderr = w
+	}
+}

cmd/tea_initialisesources.go

@@ -156,17 +156,12 @@ func (m initialiseSourcesModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		if msg.id == m.spinner.ID() {
 			m.errors = append(m.errors, fmt.Sprintf("Note: %v", msg.err))
 		}
-	case fatalError:
-		if msg.id == m.spinner.ID() {
-			m.status = taskStatusError
-			m.title = markdownToString(fmt.Sprintf("> error while configuring AWS access: %v", msg.err))
-		}
-	default:
-		var taskCmd tea.Cmd
-		m.taskModel, taskCmd = m.taskModel.Update(msg)
-		cmds = append(cmds, taskCmd)
 	}
 
+	var taskCmd tea.Cmd
+	m.taskModel, taskCmd = m.taskModel.Update(msg)
+	cmds = append(cmds, taskCmd)
+
 	// process the form if it is not yet done
 	if m.awsConfigForm != nil && !m.awsConfigFormDone {
 		switch m.awsConfigForm.State {

cmd/tea_plan.go

@@ -22,19 +22,21 @@ type runPlanModel struct {
 	args     []string
 	planFile string
 
-	revlinkTask revlinkWarmupModel
+	execCommandFunc ExecCommandFunc
+	revlinkTask     revlinkWarmupModel
 	taskModel
 }
 type runPlanNowMsg struct{}
 type runPlanFinishedMsg struct{}
 
-func NewRunPlanModel(args []string, planFile string) runPlanModel {
+func NewRunPlanModel(args []string, planFile string, execCommandFunc ExecCommandFunc) runPlanModel {
 	return runPlanModel{
 		args:     args,
 		planFile: planFile,
 
-		revlinkTask: NewRevlinkWarmupModel(),
-		taskModel:   NewTaskModel("Planning Changes"),
+		revlinkTask:     NewRevlinkWarmupModel(),
+		execCommandFunc: execCommandFunc,
+		taskModel:       NewTaskModel("Planning Changes"),
 	}
 }
 
@@ -77,22 +79,26 @@ func (m runPlanModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			c = exec.CommandContext(m.ctx, "bash", "-c", "for i in $(seq 100); do echo fake terraform plan progress line $i of 100; done; sleep 1")
 		}
 
-		_, span := tracing.Tracer().Start(m.ctx, "terraform plan", trace.WithAttributes(
+		_, span := tracing.Tracer().Start(m.ctx, "terraform plan", trace.WithAttributes( // nolint:spancheck // will be ended in the tea.Exec cleanup func
 			attribute.String("command", strings.Join(m.args, " ")),
 		))
-		cmds = append(cmds, tea.ExecProcess(
-			c,
-			func(err error) tea.Msg {
-				defer span.End()
-
-				if err != nil {
-					return fatalError{err: fmt.Errorf("failed to run terraform plan: %w", err)}
-				}
-				return runPlanFinishedMsg{}
-			}))
+		cmds = append(cmds,
+			tea.Sequence(
+				func() tea.Msg { return freezeViewMsg{} },
+				tea.Exec( // nolint:spancheck // will be ended in the tea.Exec cleanup func
+					m.execCommandFunc(c),
+					func(err error) tea.Msg {
+						defer span.End()
+
+						if err != nil {
+							return fatalError{err: fmt.Errorf("failed to run terraform plan: %w", err)}
+						}
+						return runPlanFinishedMsg{}
+					})))
 
 	case runPlanFinishedMsg:
 		m.taskModel.status = taskStatusDone
+		cmds = append(cmds, func() tea.Msg { return unfreezeViewMsg{} })
 
 	default:
 		// var cmd tea.Cmd

cmd/tea_revlink.go

@@ -66,10 +66,6 @@ func (m revlinkWarmupModel) Update(msg tea.Msg) (revlinkWarmupModel, tea.Cmd) {
 		cmds = append(cmds, m.waitForStatusActivity)
 	case revlinkWarmupFinishedMsg:
 		m.taskModel.status = taskStatusDone
-	case fatalError:
-		if msg.id == m.spinner.ID() {
-			m.taskModel.status = taskStatusError
-		}
 	default:
 		var taskCmd tea.Cmd
 		m.taskModel, taskCmd = m.taskModel.Update(msg)