Merge pull request #681 from peco/fix-screen-init-resume-error

Propagate screen Init error on resume

action.go

@@ -390,7 +390,10 @@ func doFinish(ctx context.Context, state *Peco, _ Event) {
 	state.screen.Suspend()
 
 	err = cmd.Run()
-	state.screen.Resume(ctx)
+	if err := state.screen.Resume(ctx); err != nil {
+		state.Exit(fmt.Errorf("failed to resume screen: %w", err))
+		return
+	}
 	state.Hub().SendDraw(ctx, &hub.DrawOptions{DisableCache: true})
 	if err != nil {
 		// bail out, or otherwise the user cannot know what happened

interface.go

@@ -156,7 +156,7 @@ type Screen interface {
 	Flush() error
 	PollEvent(context.Context, *Config) chan Event
 	Print(PrintArgs) int
-	Resume(context.Context)
+	Resume(context.Context) error
 	SetCell(int, int, rune, Attribute, Attribute)
 	SetCursor(int, int)
 	Size() (int, int)

peco_test.go

@@ -261,8 +261,8 @@ func (s *SimScreen) Size() (int, int) {
 	return s.screen.Size()
 }
 
-func (s *SimScreen) Resume(_ context.Context) {}
+func (s *SimScreen) Resume(_ context.Context) error { return nil }
-func (s *SimScreen) Suspend()                 {}
+func (s *SimScreen) Suspend()                       {}
 
 // Sync records a "Sync" event via the interceptor. This satisfies the
 // optional syncer interface used by BasicLayout.DrawScreen when

screen.go

@@ -20,7 +20,7 @@ import (
 type TcellScreen struct {
 	mutex     sync.Mutex
 	screen    tcell.Screen
-	resumeCh  chan chan struct{}
+	resumeCh  chan chan error
 	suspendCh chan struct{}
 	doneCh    chan struct{} // closed on permanent Close() to signal goroutines to exit
 	closeOnce sync.Once     // ensures doneCh is closed exactly once
@@ -179,7 +179,7 @@ func (t *TcellScreen) Init(_ *Config) error {
 func NewTcellScreen() *TcellScreen {
 	return &TcellScreen{
 		suspendCh: make(chan struct{}),
-		resumeCh:  make(chan chan struct{}),
+		resumeCh:  make(chan chan error),
 		doneCh:    make(chan struct{}),
 		errWriter: os.Stderr,
 	}
@@ -293,8 +293,12 @@ func (t *TcellScreen) PollEvent(ctx context.Context, cfg *Config) chan Event {
 				case <-t.doneCh:
 					return
 				case replyCh := <-t.resumeCh:
-					_ = t.Init(cfg)
+					if err := t.Init(cfg); err != nil {
-					close(replyCh)
+						fmt.Fprintf(t.errWriter, "peco: failed to re-initialize screen on resume: %v\n", err)
+						replyCh <- err
+					} else {
+						replyCh <- nil
+					}
 					continue
 				}
 			}
@@ -309,8 +313,12 @@ func (t *TcellScreen) PollEvent(ctx context.Context, cfg *Config) chan Event {
 				case <-t.doneCh:
 					return
 				case replyCh := <-t.resumeCh:
-					_ = t.Init(cfg)
+					if err := t.Init(cfg); err != nil {
-					close(replyCh)
+						fmt.Fprintf(t.errWriter, "peco: failed to re-initialize screen on resume: %v\n", err)
+						replyCh <- err
+					} else {
+						replyCh <- nil
+					}
 				}
 				continue
 			}
@@ -328,7 +336,7 @@ func (t *TcellScreen) Suspend() {
 	}
 }
 
-func (t *TcellScreen) Resume(ctx context.Context) {
+func (t *TcellScreen) Resume(ctx context.Context) error {
 	// Resume must be a block operation, because we can't safely proceed
 	// without actually knowing that the screen has been re-initialized.
 	// So we send a channel where we expect a reply back, and wait for that.
@@ -337,16 +345,18 @@ func (t *TcellScreen) Resume(ctx context.Context) {
 	// polling goroutine is not yet waiting on resumeCh, a non-blocking
 	// send would silently drop the message and the subsequent receive
 	// would block forever.
-	ch := make(chan struct{})
+	ch := make(chan error, 1)
 	select {
 	case t.resumeCh <- ch:
 	case <-ctx.Done():
-		return
+		return ctx.Err()
 	}
 
 	select {
-	case <-ch:
+	case err := <-ch:
+		return err
 	case <-ctx.Done():
+		return ctx.Err()
 	}
 }
 

screen_inline.go

@@ -225,4 +225,4 @@ func (s *InlineScreen) SendEvent(_ Event) {}
 func (s *InlineScreen) Suspend() {}
 
 // Resume is a no-op for inline mode.
-func (s *InlineScreen) Resume(_ context.Context) {}
+func (s *InlineScreen) Resume(_ context.Context) error { return nil }

screen_test.go

@@ -3,6 +3,7 @@ package peco
 import (
 	"bytes"
 	"context"
+	"errors"
 	"testing"
 	"time"
 
@@ -28,7 +29,7 @@ func (s *recordingScreen) Close() error                                  { retur
 func (s *recordingScreen) Flush() error                                  { return nil }
 func (s *recordingScreen) PollEvent(context.Context, *Config) chan Event { return nil }
 func (s *recordingScreen) Print(args PrintArgs) int                      { return screenPrint(s, args) }
-func (s *recordingScreen) Resume(context.Context)                        {}
+func (s *recordingScreen) Resume(context.Context) error                  { return nil }
 func (s *recordingScreen) SetCursor(int, int)                            {}
 func (s *recordingScreen) SendEvent(Event)                               {}
 func (s *recordingScreen) Suspend()                                      {}
@@ -240,7 +241,7 @@ func TestTcellScreenPollingGoroutineExitsOnClose(t *testing.T) {
 		case <-tb.doneCh:
 			return
 		case replyCh := <-tb.resumeCh:
-			close(replyCh)
+			replyCh <- nil
 		}
 	}()
 
@@ -313,16 +314,16 @@ func TestTcellScreenResumeNoDeadlock(t *testing.T) {
 	defer cancel()
 
 	// Simulate the polling goroutine: receive from resumeCh after a short delay,
-	// then close the reply channel (as PollEvent does after re-init).
+	// then send nil error (as PollEvent does after successful re-init).
 	go func() {
 		time.Sleep(50 * time.Millisecond)
 		replyCh := <-tb.resumeCh
-		close(replyCh)
+		replyCh <- nil
 	}()
 
 	done := make(chan struct{})
 	go func() {
-		tb.Resume(ctx)
+		require.NoError(t, tb.Resume(ctx))
 		close(done)
 	}()
 
@@ -344,10 +345,10 @@ func TestTcellScreenResumeDoesNotDropSend(t *testing.T) {
 	go func() {
 		replyCh := <-tb.resumeCh
 		close(received)
-		close(replyCh)
+		replyCh <- nil
 	}()
 
-	tb.Resume(ctx)
+	require.NoError(t, tb.Resume(ctx))
 
 	select {
 	case <-received:
@@ -367,7 +368,8 @@ func TestTcellScreenResumeContextCancelled(t *testing.T) {
 
 	done := make(chan struct{})
 	go func() {
-		tb.Resume(ctx)
+		err := tb.Resume(ctx)
+		require.Error(t, err)
 		close(done)
 	}()
 
@@ -393,7 +395,8 @@ func TestTcellScreenResumeContextCancelledWhileWaitingForReply(t *testing.T) {
 
 	done := make(chan struct{})
 	go func() {
-		tb.Resume(ctx)
+		err := tb.Resume(ctx)
+		require.Error(t, err)
 		close(done)
 	}()
 
@@ -411,3 +414,23 @@ func TestTcellScreenResumeContextCancelledWhileWaitingForReply(t *testing.T) {
 	// Verify context was indeed cancelled.
 	require.Error(t, ctx.Err())
 }
+
+func TestTcellScreenResumeInitError(t *testing.T) {
+	tb := NewTcellScreen()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	initErr := errors.New("simulated screen init failure")
+
+	// Simulate the polling goroutine: receive from resumeCh and send
+	// an error as if Init() failed.
+	go func() {
+		replyCh := <-tb.resumeCh
+		replyCh <- initErr
+	}()
+
+	err := tb.Resume(ctx)
+	require.Error(t, err)
+	require.Equal(t, initErr, err)
+}