fix: prevent errors from clobbering terminal (#2161)

Signed-off-by: Keith Zantow <kzantow@gmail.com>

cmd/syft/cli/cli.go

@@ -46,10 +46,10 @@ func create(id clio.Identification) (clio.Application, *cobra.Command) {
 					return []clio.UI{noUI}, nil
 				}
 
-				h := handler.New(handler.DefaultHandlerConfig())
-
 				return []clio.UI{
-					ui.New(h, false, cfg.Log.Quiet),
+					ui.New(cfg.Log.Quiet,
+						handler.New(handler.DefaultHandlerConfig()),
+					),
 					noUI,
 				}, nil
 			},
@@ -60,7 +60,9 @@ func create(id clio.Identification) (clio.Application, *cobra.Command) {
 				// we can hoist them into the internal packages for global use.
 				stereoscope.SetBus(state.Bus)
 				bus.Set(state.Bus)
+
 				redact.Set(state.RedactStore)
+
 				log.Set(state.Logger)
 				stereoscope.SetLogger(state.Logger)
 

cmd/syft/internal/ui/select.go

@@ -1,36 +0,0 @@
-//go:build linux || darwin || netbsd
-// +build linux darwin netbsd
-
-package ui
-
-import (
-	"os"
-	"runtime"
-
-	"golang.org/x/term"
-
-	"github.com/anchore/clio"
-	handler "github.com/anchore/syft/cmd/syft/cli/ui"
-)
-
-// Select is responsible for determining the specific UI function given select user option, the current platform
-// config values, and environment status (such as a TTY being present). The first UI in the returned slice of UIs
-// is intended to be used and the UIs that follow are meant to be attempted only in a fallback posture when there
-// are environmental problems (e.g. cannot write to the terminal). A writer is provided to capture the output of
-// the final SBOM report.
-func Select(verbose, quiet bool) (uis []clio.UI) {
-	isStdoutATty := term.IsTerminal(int(os.Stdout.Fd()))
-	isStderrATty := term.IsTerminal(int(os.Stderr.Fd()))
-	notATerminal := !isStderrATty && !isStdoutATty
-
-	switch {
-	case runtime.GOOS == "windows" || verbose || quiet || notATerminal || !isStderrATty:
-		uis = append(uis, None(quiet))
-	default:
-		// TODO: it may make sense in the future to pass handler options into select
-		h := handler.New(handler.DefaultHandlerConfig())
-		uis = append(uis, New(h, verbose, quiet))
-	}
-
-	return uis
-}

cmd/syft/internal/ui/select_windows.go

@@ -1,15 +0,0 @@
-//go:build windows
-// +build windows
-
-package ui
-
-import "github.com/anchore/clio"
-
-// Select is responsible for determining the specific UI function given select user option, the current platform
-// config values, and environment status (such as a TTY being present). The first UI in the returned slice of UIs
-// is intended to be used and the UIs that follow are meant to be attempted only in a fallback posture when there
-// are environmental problems (e.g. cannot write to the terminal). A writer is provided to capture the output of
-// the final SBOM report.
-func Select(verbose, quiet bool) (uis []clio.UI) {
-	return append(uis, None(quiet))
-}

cmd/syft/internal/ui/ui.go

@@ -1,16 +1,18 @@
 package ui
 
 import (
+	"fmt"
 	"os"
 	"sync"
+	"time"
 
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/wagoodman/go-partybus"
 
+	"github.com/anchore/bubbly"
 	"github.com/anchore/bubbly/bubbles/frame"
 	"github.com/anchore/clio"
 	"github.com/anchore/go-logger"
-	handler "github.com/anchore/syft/cmd/syft/cli/ui"
 	"github.com/anchore/syft/internal/bus"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/event"
@@ -29,13 +31,13 @@ type UI struct {
 	subscription   partybus.Unsubscribable
 	finalizeEvents []partybus.Event
 
-	handler *handler.Handler
+	handler *bubbly.HandlerCollection
 	frame   tea.Model
 }
 
-func New(h *handler.Handler, _, quiet bool) *UI {
+func New(quiet bool, handlers ...bubbly.EventHandler) *UI {
 	return &UI{
-		handler: h,
+		handler: bubbly.NewHandlerCollection(handlers...),
 		frame:   frame.New(),
 		running: &sync.WaitGroup{},
 		quiet:   quiet,
@@ -72,7 +74,7 @@ func (m *UI) Handle(e partybus.Event) error {
 
 func (m *UI) Teardown(force bool) error {
 	if !force {
-		m.handler.Running.Wait()
+		m.handler.Wait()
 		m.program.Quit()
 		// typically in all cases we would want to wait for the UI to finish. However there are still error cases
 		// that are not accounted for, resulting in hangs. For now, we'll just wait for the UI to finish in the
@@ -80,9 +82,19 @@ func (m *UI) Teardown(force bool) error {
 		// string from the worker (outside of the UI after teardown).
 		m.running.Wait()
 	} else {
+		_ = runWithTimeout(250*time.Millisecond, func() error {
+			m.handler.Wait()
+			return nil
+		})
+
 		// it may be tempting to use Kill() however it has been found that this can cause the terminal to be left in
 		// a bad state (where Ctrl+C and other control characters no longer works for future processes in that terminal).
 		m.program.Quit()
+
+		_ = runWithTimeout(250*time.Millisecond, func() error {
+			m.running.Wait()
+			return nil
+		})
 	}
 
 	// TODO: allow for writing out the full log output to the screen (only a partial log is shown currently)
@@ -119,7 +131,7 @@ func (m *UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 	switch msg := msg.(type) {
 	case tea.KeyMsg:
 		switch msg.String() {
-		// today we treat esc and ctrl+c the same, but in the future when the syft worker has a graceful way to
+		// today we treat esc and ctrl+c the same, but in the future when the worker has a graceful way to
 		// cancel in-flight work via a context, we can wire up esc to this path with bus.Exit()
 		case "esc", "ctrl+c":
 			bus.ExitWithInterrupt()
@@ -135,7 +147,7 @@ func (m *UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			m.finalizeEvents = append(m.finalizeEvents, msg)
 
 			// why not return tea.Quit here for exit events? because there may be UI components that still need the update-render loop.
-			// for this reason we'll let the syft event loop call Teardown() which will explicitly wait for these components
+			// for this reason we'll let the event loop call Teardown() which will explicitly wait for these components
 			return m, nil
 		}
 
@@ -159,3 +171,17 @@ func (m *UI) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 func (m UI) View() string {
 	return m.frame.View()
 }
+
+func runWithTimeout(timeout time.Duration, fn func() error) (err error) {
+	c := make(chan struct{}, 1)
+	go func() {
+		err = fn()
+		c <- struct{}{}
+	}()
+	select {
+	case <-c:
+	case <-time.After(timeout):
+		return fmt.Errorf("timed out after %v", timeout)
+	}
+	return err
+}

go.mod

@@ -72,7 +72,7 @@ require (
 	golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63
 	golang.org/x/mod v0.12.0
 	golang.org/x/net v0.15.0
-	golang.org/x/term v0.12.0
+	golang.org/x/term v0.12.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1
 	modernc.org/sqlite v1.25.0
 )