fix: stop readline eating exit messages (#3749)

The interactive console could eat error messages on exit
block · Dec 17, 2024 · de06a31 · de06a31
1 parent 36377bd
commit de06a31
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 33 deletions.
diff --git a/frontend/cli/cmd_dev.go b/frontend/cli/cmd_dev.go
@@ -54,7 +54,6 @@ func (d *devCmd) Run(
 	verbClient ftlv1connect.VerbServiceClient,
 ) error {
 	startTime := time.Now()
-	logger := log.FromContext(ctx)
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 	if len(d.Build.Dirs) == 0 {
@@ -145,7 +144,6 @@ func (d *devCmd) Run(
 
 	err = g.Wait()
 	if err != nil {
-		logger.Errorf(err, "error during dev")
 		return fmt.Errorf("error during dev: %w", err)
 	}
 	return nil

diff --git a/frontend/cli/cmd_serve.go b/frontend/cli/cmd_serve.go
@@ -128,7 +128,7 @@ func (s *serveCommonConfig) run(
 		_, err := controllerClient.Ping(ctx, connect.NewRequest(&ftlv1.PingRequest{}))
 		if err == nil {
 			// The controller is already running, bail out.
-			return fmt.Errorf("controller is already running")
+			return errors.New(ftlRunningErrorMsg)
 		}
 		if err := runInBackground(logger); err != nil {
 			return err
@@ -152,7 +152,7 @@ func (s *serveCommonConfig) run(
 	_, err := controllerClient.Ping(ctx, connect.NewRequest(&ftlv1.PingRequest{}))
 	if err == nil {
 		// The controller is already running, bail out.
-		return fmt.Errorf("controller is already running")
+		return errors.New(ftlRunningErrorMsg)
 	}
 	if s.Provisioners > 0 {
 		logger.Debugf("Starting FTL with %d controller(s) and %d provisioner(s)", s.Controllers, s.Provisioners)

diff --git a/internal/terminal/interactive.go b/internal/terminal/interactive.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"strings"
+	"sync"
 	"syscall"
 
 	"github.com/alecthomas/kong"
@@ -26,10 +27,15 @@ type KongContextBinder func(ctx context.Context, kctx *kong.Context) context.Con
 
 type exitPanic struct{}
 
-func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContextBinder, eventSource schemaeventsource.EventSource) error {
-	if !readline.DefaultIsTerminal() {
-		return nil
-	}
+type interactiveConsole struct {
+	l         *readline.Instance
+	binder    KongContextBinder
+	k         *kong.Kong
+	closeWait sync.WaitGroup
+	closed    bool
+}
+
+func newInteractiveConsole(k *kong.Kong, binder KongContextBinder, eventSource schemaeventsource.EventSource) (*interactiveConsole, error) {
 	l, err := readline.NewEx(&readline.Config{
 		Prompt:          interactivePrompt,
 		InterruptPrompt: "^C",
@@ -38,20 +44,59 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext
 			_ = syscall.Kill(-syscall.Getpid(), syscall.SIGINT) //nolint:forcetypeassert,errcheck // best effort
 		}},
 	})
+	if err != nil {
+		return nil, fmt.Errorf("init readline: %w", err)
+	}
+
+	it := &interactiveConsole{k: k, binder: binder, l: l}
+	it.closeWait.Add(1)
+	return it, nil
+}
+
+func (r *interactiveConsole) Close() {
+	if r.closed {
+		return
+	}
+	r.closed = true
+	r.Close()
+	r.closeWait.Wait()
+}
+func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContextBinder, eventSource schemaeventsource.EventSource) error {
+	if !readline.DefaultIsTerminal() {
+		return nil
+	}
+	ic, err := newInteractiveConsole(k, binder, eventSource)
+	if err != nil {
+		return err
+	}
+	defer ic.Close()
+	err = ic.run(ctx)
+	if err != nil {
+		return err
+	}
+	return nil
+
+}
+
+func (r *interactiveConsole) run(ctx context.Context) error {
+	if !readline.DefaultIsTerminal() {
+		return nil
+	}
+	l := r.l
+	k := r.k
+	defer r.closeWait.Done()
+
 	sm := FromContext(ctx)
 	var tsm *terminalStatusManager
 	ok := false
 	if tsm, ok = sm.(*terminalStatusManager); ok {
 		tsm.statusLock.Lock()
 		tsm.clearStatusMessages()
 		tsm.console = true
-		tsm.consoleRefresh = l.Refresh
+		tsm.consoleRefresh = r.l.Refresh
 		tsm.recalculateLines()
 		tsm.statusLock.Unlock()
 	}
-	if err != nil {
-		return fmt.Errorf("init readline: %w", err)
-	}
 	context.AfterFunc(ctx, func() {
 		_ = l.Close()
 	})
@@ -71,6 +116,7 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext
 		// we recover from this and continue the loop
 		panic(exitPanic{})
 	}
+
 	for {
 		line, err := l.Readline()
 		if errors.Is(err, readline.ErrInterrupt) {
@@ -80,7 +126,7 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext
 			}
 			continue
 		} else if errors.Is(err, io.EOF) {
-			os.Exit(0)
+			return nil
 		}
 		if tsm != nil {
 			tsm.consoleNewline(line)
@@ -108,7 +154,7 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext
 				errorf("%s", err)
 				return
 			}
-			subctx := binder(ctx, kctx)
+			subctx := r.binder(ctx, kctx)
 
 			err = kctx.Run(subctx)
 			if err != nil {
@@ -117,7 +163,8 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext
 			}
 		}()
 	}
-	_ = l.Close()
+	_ = l.Close() //nolint:errcheck // best effort
+
 	return nil
 }
 

diff --git a/internal/terminal/status.go b/internal/terminal/status.go
@@ -15,6 +15,7 @@ import (
 
 	"github.com/alecthomas/atomic"
 	"github.com/alecthomas/kong"
+	"github.com/alecthomas/types/optional"
 	"github.com/tidwall/pretty"
 	"golang.org/x/term"
 
@@ -92,22 +93,23 @@ type StatusLine interface {
 }
 
 type terminalStatusManager struct {
-	old              *os.File
-	oldErr           *os.File
-	read             *os.File
-	write            *os.File
-	closed           atomic.Value[bool]
-	totalStatusLines int
-	statusLock       sync.Mutex
-	lines            []*terminalStatusLine
-	moduleLine       *terminalStatusLine
-	moduleStates     map[string]BuildState
-	height           int
-	width            int
-	exitWait         sync.WaitGroup
-	console          bool
-	consoleRefresh   func()
-	spinnerCount     int
+	old                *os.File
+	oldErr             *os.File
+	read               *os.File
+	write              *os.File
+	closed             atomic.Value[bool]
+	totalStatusLines   int
+	statusLock         sync.Mutex
+	lines              []*terminalStatusLine
+	moduleLine         *terminalStatusLine
+	moduleStates       map[string]BuildState
+	height             int
+	width              int
+	exitWait           sync.WaitGroup
+	console            bool
+	consoleRefresh     func()
+	spinnerCount       int
+	interactiveConsole optional.Option[*interactiveConsole]
 }
 
 type statusKey struct{}
@@ -162,10 +164,16 @@ func NewStatusManager(ctx context.Context) StatusManager {
 					sm.writeLine(current, true)
 				}
 				if !closed {
+					sm.clearStatusMessages()
 					sm.exitWait.Done()
 				}
 				return
 			}
+			if closed {
+				// When we are closed we just write the data to the old stdout
+				_, _ = sm.old.Write(rawData[:n]) //nolint:errcheck
+				continue
+			}
 			buf.Write(rawData[:n])
 			for buf.Len() > 0 {
 				d, s, err := buf.ReadRune()
@@ -175,6 +183,7 @@ func NewStatusManager(ctx context.Context) StatusManager {
 					// that we handle on a best effort basis
 					sm.writeLine(current, true)
 					if !closed {
+						sm.clearStatusMessages()
 						sm.exitWait.Done()
 						closed = true
 					}
@@ -345,6 +354,9 @@ func (r *terminalStatusManager) SetModuleState(module string, state BuildState)
 
 func (r *terminalStatusManager) Close() {
 	r.statusLock.Lock()
+	if it, ok := r.interactiveConsole.Get(); ok {
+		it.Close()
+	}
 	r.clearStatusMessages()
 	r.totalStatusLines = 0
 	r.lines = []*terminalStatusLine{}
@@ -498,9 +510,15 @@ func (r *terminalStatusLine) SetMessage(message string) {
 
 func LaunchEmbeddedConsole(ctx context.Context, k *kong.Kong, binder KongContextBinder, eventSource schemaeventsource.EventSource) {
 	sm := FromContext(ctx)
-	if _, ok := sm.(*terminalStatusManager); ok {
+	if tsm, ok := sm.(*terminalStatusManager); ok {
+		it, err := newInteractiveConsole(k, binder, eventSource)
+		if err != nil {
+			fmt.Printf("\033[31mError: %s\033[0m\n", err)
+			return
+		}
+		tsm.interactiveConsole = optional.Some(it)
 		go func() {
-			err := RunInteractiveConsole(ctx, k, binder, eventSource)
+			err := it.run(ctx)
 			if err != nil {
 				fmt.Printf("\033[31mError: %s\033[0m\n", err)
 				return