From 60d90d2ae159f590bb9f936ed1354d2a94c20aee Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Tue, 17 Feb 2026 18:15:57 +0900 Subject: [PATCH] don't just swallow the panic from inline tcell polling --- screen_inline.go | 13 +++++++++++-- screen_inline_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/screen_inline.go b/screen_inline.go index 6aec588..5c9faef 100644 --- a/screen_inline.go +++ b/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() diff --git a/screen_inline_test.go b/screen_inline_test.go index ef0a227..59e1cc5 100644 --- a/screen_inline_test.go +++ b/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{}