From b82832be9dbf868efb7e38b3fdf01c10e4253cd7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= <ksuszyns@redhat.com>
Date: Mon, 25 Mar 2024 14:48:59 +0100
Subject: [PATCH] Bubbletea initialization error handling

---
 pkg/output/logging/context.go   |  2 +-
 pkg/output/term/io.go           | 14 ++++++++++----
 pkg/output/tui/progress.go      | 15 +++++++++++----
 pkg/output/tui/progress_test.go |  4 +---
 pkg/output/tui/spinner.go       | 15 +++++++++++----
 pkg/output/tui/spinner_test.go  |  3 ---
 6 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/pkg/output/logging/context.go b/pkg/output/logging/context.go
index 1b94ba01..534d7ca9 100644
--- a/pkg/output/logging/context.go
+++ b/pkg/output/logging/context.go
@@ -132,7 +132,7 @@ func createDefaultLogger(ctx context.Context) *zap.Logger {
 		zapcore.AddSync(errout),
 		lvl,
 	))
-	if !term.IsTerminal(errout) {
+	if !term.IsFancy(errout) {
 		ec = zap.NewProductionEncoderConfig()
 		logger = zap.New(zapcore.NewCore(
 			zapcore.NewJSONEncoder(ec),
diff --git a/pkg/output/term/io.go b/pkg/output/term/io.go
index b481ebea..38cbcca1 100644
--- a/pkg/output/term/io.go
+++ b/pkg/output/term/io.go
@@ -30,11 +30,17 @@ func IsReaderTerminal(r io.Reader) bool {
 	return ok && term.IsTerminal(int(f.Fd()))
 }
 
-// IsTerminal returns true if the given writer is a real terminal.
-func IsTerminal(w io.Writer) bool {
+// IsWriterTerminal returns true if the given writer is a real terminal.
+func IsWriterTerminal(w io.Writer) bool {
+	f, ok := w.(*os.File)
+	return ok && term.IsTerminal(int(f.Fd()))
+}
+
+// IsFancy returns true if the given writer is a real terminal, or the color
+// is forced by the environment variables.
+func IsFancy(w io.Writer) bool {
 	if environment.ColorIsForced() {
 		return true
 	}
-	f, ok := w.(*os.File)
-	return ok && environment.NonColorRequested() && term.IsTerminal(int(f.Fd()))
+	return !environment.NonColorRequested() && IsWriterTerminal(w)
 }
diff --git a/pkg/output/tui/progress.go b/pkg/output/tui/progress.go
index cb26f70f..403816d0 100644
--- a/pkg/output/tui/progress.go
+++ b/pkg/output/tui/progress.go
@@ -26,6 +26,7 @@ import (
 	"github.com/charmbracelet/bubbles/progress"
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/lipgloss"
+	"go.uber.org/multierr"
 	"knative.dev/client-pkg/pkg/output"
 	"knative.dev/client-pkg/pkg/output/term"
 )
@@ -79,12 +80,16 @@ type BubbleProgress struct {
 	prevSpeed  []int
 	err        error
 	quitChan   chan struct{}
+	teaErr     error
 }
 
 func (b *BubbleProgress) With(fn func(ProgressControl) error) error {
 	b.start()
-	defer b.stop()
-	return fn(b)
+	err := func() error {
+		defer b.stop()
+		return fn(b)
+	}()
+	return multierr.Combine(err, b.teaErr)
 }
 
 func (b *BubbleProgress) Error(err error) {
@@ -244,9 +249,11 @@ func (b *BubbleProgress) start() {
 	b.quitChan = make(chan struct{})
 	go func() {
 		t := b.tea
-		_, _ = t.Run()
+		if _, err := t.Run(); err != nil {
+			b.teaErr = err
+		}
 		close(b.quitChan)
-		if term.IsTerminal(out) {
+		if term.IsWriterTerminal(out) {
 			if err := t.ReleaseTerminal(); err != nil {
 				panic(err)
 			}
diff --git a/pkg/output/tui/progress_test.go b/pkg/output/tui/progress_test.go
index 29e482cd..bb5df64e 100644
--- a/pkg/output/tui/progress_test.go
+++ b/pkg/output/tui/progress_test.go
@@ -33,9 +33,7 @@ import (
 )
 
 func TestProgress(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping test in short mode.")
-	}
+	testing.Short()
 	ctx := context.TestContext(t)
 	prt := output.NewTestPrinter()
 	ctx = output.WithContext(ctx, prt)
diff --git a/pkg/output/tui/spinner.go b/pkg/output/tui/spinner.go
index 438ef358..6f44c4f5 100644
--- a/pkg/output/tui/spinner.go
+++ b/pkg/output/tui/spinner.go
@@ -22,6 +22,7 @@ import (
 	"github.com/charmbracelet/bubbles/spinner"
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/lipgloss"
+	"go.uber.org/multierr"
 	"knative.dev/client-pkg/pkg/output"
 	"knative.dev/client-pkg/pkg/output/term"
 )
@@ -46,12 +47,16 @@ type BubbleSpinner struct {
 	spin     spinner.Model
 	tea      *tea.Program
 	quitChan chan struct{}
+	teaErr   error
 }
 
 func (b *BubbleSpinner) With(fn func(Spinner) error) error {
 	b.start()
-	defer b.stop()
-	return fn(b)
+	err := func() error {
+		defer b.stop()
+		return fn(b)
+	}()
+	return multierr.Combine(err, b.teaErr)
 }
 
 func (b *BubbleSpinner) Init() tea.Cmd {
@@ -81,9 +86,11 @@ func (b *BubbleSpinner) start() {
 	b.quitChan = make(chan struct{})
 	go func() {
 		t := b.tea
-		_, _ = t.Run()
+		if _, err := t.Run(); err != nil {
+			b.teaErr = err
+		}
 		close(b.quitChan)
-		if term.IsTerminal(out) {
+		if term.IsWriterTerminal(out) {
 			_ = t.ReleaseTerminal()
 		}
 	}()
diff --git a/pkg/output/tui/spinner_test.go b/pkg/output/tui/spinner_test.go
index 6e102ecc..343f44be 100644
--- a/pkg/output/tui/spinner_test.go
+++ b/pkg/output/tui/spinner_test.go
@@ -26,9 +26,6 @@ import (
 )
 
 func TestSpinner(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping test in short mode.")
-	}
 	ctx := context.TestContext(t)
 	prt := output.NewTestPrinter()
 	ctx = output.WithContext(ctx, prt)