From 6cf12236341aa4bb5467d45b25641a3489591435 Mon Sep 17 00:00:00 2001 From: Johan Walles Date: Sat, 10 Aug 2024 08:40:48 +0200 Subject: [PATCH] Catch errors in goroutines and log them --- m/pager.go | 12 ++++++++++++ m/panicHandler.go | 17 +++++++++++++++++ m/reader.go | 12 ++++++++---- m/search.go | 4 ++++ twin/panicHandler.go | 17 +++++++++++++++++ twin/screen-setup.go | 4 ++++ twin/screen-setup_test.go | 4 ++++ twin/screen.go | 8 +++++++- twin/screen_test.go | 4 ++++ 9 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 m/panicHandler.go create mode 100644 twin/panicHandler.go diff --git a/m/pager.go b/m/pager.go index b28849c..ca1dc98 100644 --- a/m/pager.go +++ b/m/pager.go @@ -322,6 +322,10 @@ func (p *Pager) StartPaging(screen twin.Screen, chromaStyle *chroma.Style, chrom p.marks = make(map[rune]scrollPosition) go func() { + defer func() { + panicHandler("StartPaging()/moreLinesAvailable", recover()) + }() + for range p.reader.moreLinesAdded { // Notify the main loop about the new lines so it can show them screen.Events() <- eventMoreLinesAvailable{} @@ -337,6 +341,10 @@ func (p *Pager) StartPaging(screen twin.Screen, chromaStyle *chroma.Style, chrom }() go func() { + defer func() { + panicHandler("StartPaging()/spinner", recover()) + }() + // Spin the spinner as long as contents is still loading spinnerFrames := [...]string{"/.\\", "-o-", "\\O/", "| |"} spinnerIndex := 0 @@ -359,6 +367,10 @@ func (p *Pager) StartPaging(screen twin.Screen, chromaStyle *chroma.Style, chrom }() go func() { + defer func() { + panicHandler("StartPaging()/maybeDone", recover()) + }() + for range p.reader.maybeDone { screen.Events() <- eventMaybeDone{} } diff --git a/m/panicHandler.go b/m/panicHandler.go new file mode 100644 index 0000000..585ba92 --- /dev/null +++ b/m/panicHandler.go @@ -0,0 +1,17 @@ +package m + +// NOTE: This file should be identical to twin/panicHandler.go + +import ( + log "github.com/sirupsen/logrus" +) + +func panicHandler(goroutineName string, recoverResult any) { + if recoverResult == nil { + return + } + + log.WithFields(log.Fields{ + "recoverResult": recoverResult, + }).Error("Goroutine panicked: " + goroutineName) +} diff --git a/m/reader.go b/m/reader.go index 9e80959..0bfa1df 100644 --- a/m/reader.go +++ b/m/reader.go @@ -342,7 +342,7 @@ func NewReaderFromStream(name string, reader io.Reader, style chroma.Style, form // then used for pre-allocating the lines slice, which improves large file // loading performance. // -// If lexer is not nil, the file will be highlighted after being fully read. +// If lexer is set, the file will be highlighted after being fully read. // // Note that you must call reader.SetStyleForHighlighting() after this to get // highlighting. @@ -364,9 +364,13 @@ func newReaderFromStream(reader io.Reader, originalFileName *string, formatter c done: &done, } - // FIXME: Make sure that if we panic somewhere inside of this goroutine, - // the main program terminates and prints our panic stack trace. - go returnMe.readStream(reader, formatter, lexer) + go func() { + defer func() { + panicHandler("newReaderFromStream()/readStream()", recover()) + }() + + returnMe.readStream(reader, formatter, lexer) + }() return &returnMe } diff --git a/m/search.go b/m/search.go index 5b67346..74be668 100644 --- a/m/search.go +++ b/m/search.go @@ -124,6 +124,10 @@ func (p *Pager) findFirstHit(startPosition linenumbers.LineNumber, beforePositio } go func(i int, searchStart linenumbers.LineNumber, chunkBefore *linenumbers.LineNumber) { + defer func() { + panicHandler("findFirstHit()/chunkSearch", recover()) + }() + findings[i] <- p._findFirstHit(searchStart, chunkBefore, backwards) }(i, searchStart, chunkBefore) } diff --git a/twin/panicHandler.go b/twin/panicHandler.go new file mode 100644 index 0000000..83efb8b --- /dev/null +++ b/twin/panicHandler.go @@ -0,0 +1,17 @@ +package twin + +// NOTE: This file should be identical to m/panicHandler.go + +import ( + log "github.com/sirupsen/logrus" +) + +func panicHandler(goroutineName string, recoverResult any) { + if recoverResult == nil { + return + } + + log.WithFields(log.Fields{ + "recoverResult": recoverResult, + }).Error("Goroutine panicked: " + goroutineName) +} diff --git a/twin/screen-setup.go b/twin/screen-setup.go index 05a1c50..ae7077b 100644 --- a/twin/screen-setup.go +++ b/twin/screen-setup.go @@ -117,6 +117,10 @@ func (screen *UnixScreen) setupSigwinchNotification() { sigwinch := make(chan os.Signal, 1) signal.Notify(sigwinch, syscall.SIGWINCH) go func() { + defer func() { + panicHandler("setupSigwinchNotification()/SIGWINCH", recover()) + }() + for { // Await window resize signal <-sigwinch diff --git a/twin/screen-setup_test.go b/twin/screen-setup_test.go index 64eb433..542e289 100644 --- a/twin/screen-setup_test.go +++ b/twin/screen-setup_test.go @@ -31,6 +31,10 @@ func TestInterruptableReader_blockedOnReadImmediate(t *testing.T) { } readResultChan := make(chan readResult) go func() { + defer func() { + panicHandler("TestInterruptableReader_blockedOnReadImmediate()", recover()) + }() + buffer := make([]byte, 1) n, err := testMe.Read(buffer) readResultChan <- readResult{n, err} diff --git a/twin/screen.go b/twin/screen.go index fd7fe98..30e179d 100644 --- a/twin/screen.go +++ b/twin/screen.go @@ -179,7 +179,13 @@ func NewScreenWithMouseModeAndColorType(mouseMode MouseMode, terminalColorCount screen.hideCursor(true) - go screen.mainLoop() + go func() { + defer func() { + panicHandler("NewScreenWithMouseModeAndColorType()/mainLoop()", recover()) + }() + + screen.mainLoop() + }() return &screen, nil } diff --git a/twin/screen_test.go b/twin/screen_test.go index 48e65e8..4f1957e 100644 --- a/twin/screen_test.go +++ b/twin/screen_test.go @@ -269,6 +269,10 @@ func TestInterruptableReader_blockedOnRead(t *testing.T) { } readResultChan := make(chan readResult) go func() { + defer func() { + panicHandler("TestInterruptableReader_blockedOnRead()", recover()) + }() + buffer := make([]byte, 1) n, err := testMe.Read(buffer) readResultChan <- readResult{n, err}