From a311e73a6949e522157610cdeaa96d1f5a56ee5a Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Mon, 16 Dec 2024 09:36:29 +1100 Subject: [PATCH] fix: stop readline eating exit messages The interactive console could eat error messages on exit --- frontend/cli/cmd_dev.go | 2 - frontend/cli/cmd_serve.go | 4 +- internal/terminal/interactive.go | 69 +++++++++++++++++++++++++++----- internal/terminal/status.go | 54 ++++++++++++++++--------- 4 files changed, 96 insertions(+), 33 deletions(-) diff --git a/frontend/cli/cmd_dev.go b/frontend/cli/cmd_dev.go index cb4f88cf4..a41ae2a22 100644 --- 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 index b5f4c1f87..abaa02a89 100644 --- 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 index 8008c7426..cdb3a92ad 100644 --- 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,6 +44,48 @@ 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 @@ -45,13 +93,10 @@ func RunInteractiveConsole(ctx context.Context, k *kong.Kong, binder KongContext 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 index c903584c2..b20fba3ac 100644 --- 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