Merge pull request #293 from overmindtech/polish

Improve error output handling

cmd/root.go

@@ -78,20 +78,17 @@ func NewOvermindInstance(ctx context.Context, app string) (OvermindInstance, err
 	log.WithField("instanceDataUrl", instanceDataUrl).Debug("Fetching instance-data")
 	res, err := otelhttp.DefaultClient.Do(req)
 	if err != nil {
-		log.WithContext(ctx).WithError(err).Error("could not fetch instance-data")
 		return OvermindInstance{}, fmt.Errorf("could not fetch instance-data: %w", err)
 	}
 
 	if res.StatusCode != 200 {
-		log.WithContext(ctx).WithField("status-code", res.StatusCode).Error("instance-data fetch returned non-200 status")
 		return OvermindInstance{}, fmt.Errorf("instance-data fetch returned non-200 status: %v", res.StatusCode)
 	}
 
 	defer res.Body.Close()
 	data := instanceData{}
 	err = json.NewDecoder(res.Body).Decode(&data)
 	if err != nil {
-		log.WithContext(ctx).WithError(err).Error("could not parse instance-data")
 		return OvermindInstance{}, fmt.Errorf("could not parse instance-data: %w", err)
 	}
 

cmd/tea_app.go

@@ -55,6 +55,13 @@ type taskModel struct {
 	spinner spinner.Model
 }
 
+type WithTaskModel interface {
+	TaskModel() taskModel
+}
+
+// assert that taskModel implements WithTaskModel
+var _ WithTaskModel = (*taskModel)(nil)
+
 func NewTaskModel(title string) taskModel {
 	return taskModel{
 		status: taskStatusPending,
@@ -73,6 +80,10 @@ func (m taskModel) Init() tea.Cmd {
 	return nil
 }
 
+func (m taskModel) TaskModel() taskModel {
+	return m
+}
+
 func (m taskModel) Update(msg tea.Msg) (taskModel, tea.Cmd) {
 	switch msg := msg.(type) {
 	case tea.KeyMsg:

cmd/tea_ensuretoken.go

@@ -76,6 +76,10 @@ func NewEnsureTokenModel(ctx context.Context, app string, apiKey string, require
 	}
 }
 
+func (m ensureTokenModel) TaskModel() taskModel {
+	return m.taskModel
+}
+
 func (m ensureTokenModel) Init() tea.Cmd {
 	return m.taskModel.Init()
 }

cmd/tea_initialisesources.go

@@ -62,6 +62,10 @@ func NewInitialiseSourcesModel() tea.Model {
 	}
 }
 
+func (m initialiseSourcesModel) TaskModel() taskModel {
+	return m.taskModel
+}
+
 func (m initialiseSourcesModel) Init() tea.Cmd {
 	return m.taskModel.Init()
 }

cmd/tea_startup.go

@@ -27,6 +27,10 @@ func NewInstanceLoaderModel(ctx context.Context, app string) tea.Model {
 	return result
 }
 
+func (m instanceLoaderModel) TaskModel() taskModel {
+	return m.taskModel
+}
+
 func (m instanceLoaderModel) Init() tea.Cmd {
 	return tea.Batch(
 		m.taskModel.Init(),

cmd/tea_terraform.go

@@ -30,8 +30,10 @@ type cmdModel struct {
 	requiredScopes []string
 
 	// UI state
-	tasks      map[string]tea.Model
-	fatalError string // this will get set if there's a fatalError coming through that doesn't have a task ID set
+	tasks               map[string]tea.Model
+	terraformHasStarted bool   // remember whether terraform already has started. this is important to do the correct workarounds on errors. See also `skipView()`
+	fatalErrorSeen      bool   // remember whether a fatalError has been seen to avoid showing pending tasks
+	fatalError          string // this will get set if there's a fatalError coming through that doesn't have a task ID set
 
 	// business logic. This model will implement the actual CLI functionality requested.
 	cmd tea.Model
@@ -82,10 +84,21 @@ func (m cmdModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 
 	case fatalError:
 		log.WithError(msg.err).WithField("msg.id", msg.id).Debug("cmdModel: fatalError received")
+
+		// skipView based on the previous view. While this is not perfect, it's
+		// the best we can currently do without taking complete control of
+		// terraform command i/o
+		if m.terraformHasStarted {
+			skipView(m.View())
+		}
+
+		m.fatalErrorSeen = true
+
+		// record the fatal error here if it was not from a specific taskModel
 		if msg.id == 0 {
 			m.fatalError = msg.err.Error()
 		}
-		skipView(m.View())
+
 		return m, tea.Sequence(
 			tea.Batch(batch...),
 			tea.Quit,
@@ -101,6 +114,9 @@ func (m cmdModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		batch = append(batch, cmd)
 		return tm, tea.Batch(batch...)
 
+	case triggerTfPlanMsg, runTfApplyMsg:
+		m.terraformHasStarted = true
+
 	case tfPlanFinishedMsg, tfApplyFinishedMsg:
 		// bump screen after terraform ran
 		skipView(m.View())
@@ -172,6 +188,16 @@ func (m cmdModel) View() string {
 	}
 	sort.Strings(keys)
 	for _, k := range keys {
+		// when we're quitting due to a fatal error, don't show pending tasks as
+		// they are not relevant anymore
+		if m.fatalErrorSeen {
+			t, ok := m.tasks[k].(WithTaskModel)
+			if ok {
+				if t.TaskModel().status == taskStatusPending {
+					continue
+				}
+			}
+		}
 		tasks = append(tasks, m.tasks[k].View())
 	}
 	tasks = append(tasks, m.cmd.View())

cmd/terraform_plan.go

@@ -378,9 +378,11 @@ func (m tfPlanModel) View() string {
 		bits = append(bits, fmt.Sprintf("\nCheck the blast radius graph and risks at:\n%v\n\n", m.changeUrl))
 	}
 
-	if m.fatalError != "" {
-		bits = append(bits, deletedLineStyle.Render(fmt.Sprintf("Error: %v", m.fatalError)))
-	}
+	// This doesn't do line-wrapping for long errors and is duplicated by the
+	// fatalError view on cmdModel
+	// if m.fatalError != "" {
+	// 	bits = append(bits, deletedLineStyle.Render(fmt.Sprintf("Error: %v", m.fatalError)))
+	// }
 
 	return strings.Join(bits, "\n") + "\n"
 }