log pollevent errors

This commit is contained in:
Daisuke Maki 2026-02-17 07:38:56 +09:00
parent 79bc52f006
commit cfc3736e8e
2 changed files with 57 additions and 1 deletions

View file

@ -3,6 +3,9 @@ package peco
import (
"context"
"fmt"
"io"
"os"
"runtime/debug"
"sync"
"unicode/utf8"
@ -21,6 +24,7 @@ type TcellScreen struct {
suspendCh chan struct{}
doneCh chan struct{} // closed on permanent Close() to signal goroutines to exit
closeOnce sync.Once // ensures doneCh is closed exactly once
errWriter io.Writer // destination for error output (defaults to os.Stderr)
}
// tcellKeyToKeyseq maps tcell navigation/function key constants to peco keyseq constants.
@ -177,6 +181,7 @@ func NewTcellScreen() *TcellScreen {
suspendCh: make(chan struct{}),
resumeCh: make(chan chan struct{}),
doneCh: make(chan struct{}),
errWriter: os.Stderr,
}
}
@ -268,7 +273,11 @@ func (t *TcellScreen) PollEvent(ctx context.Context, cfg *Config) chan Event {
}()
go func() {
defer func() { recover() }()
defer func() {
if r := recover(); r != nil {
fmt.Fprintf(t.errWriter, "peco: panic in PollEvent goroutine: %v\n%s", r, debug.Stack())
}
}()
defer func() { close(evCh) }()
for {

View file

@ -1,13 +1,60 @@
package peco
import (
"bytes"
"context"
"testing"
"time"
"github.com/gdamore/tcell/v2"
"github.com/stretchr/testify/require"
)
// panickingScreen wraps a tcell.Screen and panics on PollEvent.
// Used to test that TcellScreen's PollEvent goroutine logs panics
// instead of silently swallowing them.
type panickingScreen struct {
tcell.Screen
}
func (s *panickingScreen) PollEvent() tcell.Event {
panic("test: deliberate panic in PollEvent")
}
// TestTcellScreenPollEventLogsPanic verifies that when a panic occurs
// in the PollEvent goroutine, it is logged to errWriter rather than
// being silently swallowed (the bug described in CODE_REVIEW.md §3.2).
func TestTcellScreenPollEventLogsPanic(t *testing.T) {
var buf bytes.Buffer
ts := NewTcellScreen()
ts.errWriter = &buf
// Set the screen to a wrapper that panics on PollEvent.
sim := tcell.NewSimulationScreen("")
sim.Init()
ts.screen = &panickingScreen{Screen: sim}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
evCh := ts.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")
ts.Close()
}
// TestTcellScreenSuspendHandlerExitsOnClose verifies that the suspend handler
// goroutine (started by PollEvent) exits when Close() is called, even if
// the context has not been cancelled. This is the goroutine leak described