Merge pull request #659 from peco/fix-inline-pollevent-panic-swallow

don't just swallow the panic from inline tcell polling

screen_inline.go

@@ -3,7 +3,9 @@ package peco
 import (
 	"context"
 	"fmt"
+	"io"
 	"os"
+	"runtime/debug"
 	"sync"
 
 	"github.com/gdamore/tcell/v2"
@@ -23,12 +25,15 @@ type InlineScreen struct {
 	// savedAltscreen holds the original TCELL_ALTSCREEN value so we can
 	// restore it on Close.
 	savedAltscreen string
+
+	errWriter io.Writer // destination for error output (defaults to os.Stderr)
 }
 
 // NewInlineScreen creates a new InlineScreen with the given height spec.
 func NewInlineScreen(spec HeightSpec) *InlineScreen {
 	return &InlineScreen{
 		heightSpec: spec,
+		errWriter:  os.Stderr,
 	}
 }
 
@@ -168,8 +173,12 @@ func (s *InlineScreen) PollEvent(ctx context.Context, cfg *Config) chan Event {
 	evCh := make(chan Event)
 
 	go func() {
-		defer func() { recover() }()
-		defer func() { close(evCh) }()
+		defer func() {
+			if r := recover(); r != nil {
+				fmt.Fprintf(s.errWriter, "peco: panic in PollEvent goroutine: %v\n%s", r, debug.Stack())
+			}
+			close(evCh)
+		}()
 
 		for {
 			s.mutex.Lock()

screen_inline_test.go

@@ -1,7 +1,10 @@
 package peco
 
 import (
+	"bytes"
+	"context"
 	"testing"
+	"time"
 
 	"github.com/gdamore/tcell/v2"
 	"github.com/stretchr/testify/require"
@@ -66,6 +69,41 @@ func TestInlineScreenSetCursor(t *testing.T) {
 	require.Equal(t, 16, cy) // 2 + (24-10) = 16
 }
 
+// TestInlineScreenPollEventLogsPanic verifies that when a panic occurs
+// in the InlineScreen PollEvent goroutine, it is logged to errWriter
+// rather than being silently swallowed (CODE_REVIEW.md §3.5).
+func TestInlineScreenPollEventLogsPanic(t *testing.T) {
+	var buf bytes.Buffer
+
+	sim := tcell.NewSimulationScreen("")
+	sim.Init()
+
+	s := &InlineScreen{
+		screen:    &panickingScreen{Screen: sim},
+		errWriter: &buf,
+		height:    10,
+		yOffset:   14,
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	evCh := s.PollEvent(ctx, nil)
+
+	// The goroutine should panic, log it, and close the channel.
+	select {
+	case _, ok := <-evCh:
+		require.False(t, ok, "expected channel to be closed after panic")
+	case <-time.After(2 * time.Second):
+		t.Fatal("PollEvent channel was not closed after panic")
+	}
+
+	// Verify that the panic was logged (not silently swallowed).
+	output := buf.String()
+	require.Contains(t, output, "peco: panic in PollEvent goroutine")
+	require.Contains(t, output, "test: deliberate panic in PollEvent")
+}
+
 func TestInlineScreenNilSafety(t *testing.T) {
 	s := &InlineScreen{}