From ac9d05efa27d07a65511b6e55ebe2f50203cb1f5 Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Sun, 10 Mar 2013 13:30:00 +0000 Subject: [PATCH] Port sp0rkle's panic recovery back into goirc. --- client/connection.go | 4 ++++ client/dispatch.go | 11 ++++++++++- client/dispatch_test.go | 35 ++++++++++++++++++++++++++++++----- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/client/connection.go b/client/connection.go index 02d1b4b..88f992e 100644 --- a/client/connection.go +++ b/client/connection.go @@ -68,6 +68,9 @@ type Config struct { // Sent as the QUIT message. QuitMessage string + + // Configurable panic recovery for all handlers. + Recover func(*Conn, *Line) } func NewConfig(nick string, args ...string) *Config { @@ -75,6 +78,7 @@ func NewConfig(nick string, args ...string) *Config { Me: state.NewNick(nick), PingFreq: 3 * time.Minute, NewNick: func(s string) string { return s + "_" }, + Recover: (*Conn).LogPanic, // in dispatch.go } cfg.Me.Ident = "goirc" if len(args) > 0 && args[0] != "" { diff --git a/client/dispatch.go b/client/dispatch.go index e0645a4..0c7ae89 100644 --- a/client/dispatch.go +++ b/client/dispatch.go @@ -2,6 +2,7 @@ package client import ( "github.com/fluffle/golog/logging" + "runtime" "strings" "sync" ) @@ -45,8 +46,9 @@ type hNode struct { handler Handler } -// A hNode implements both Handler... +// A hNode implements both Handler (with configurable panic recovery)... func (hn *hNode) Handle(conn *Conn, line *Line) { + defer conn.cfg.Recover(conn, line) hn.handler.Handle(conn, line) } @@ -141,3 +143,10 @@ func (conn *Conn) HandleFunc(name string, hf HandlerFunc) Remover { func (conn *Conn) dispatch(line *Line) { conn.handlers.dispatch(conn, line) } + +func (conn *Conn) LogPanic(line *Line) { + if err := recover(); err != nil { + _, f, l, _ := runtime.Caller(2) + logging.Error("%s:%d: panic: %v", f, l, err) + } +} diff --git a/client/dispatch_test.go b/client/dispatch_test.go index 965528c..026dea9 100644 --- a/client/dispatch_test.go +++ b/client/dispatch_test.go @@ -6,6 +6,11 @@ import ( ) func TestHandlerSet(t *testing.T) { + // A Conn is needed here because the previous behaviour of passing nil to + // hset.dispatch causes a nil pointer dereference with panic recovery. + c, s := setUp(t) + defer s.tearDown() + hs := handlerSet() if len(hs.set) != 0 { t.Errorf("New set contains things!") @@ -83,7 +88,7 @@ func TestHandlerSet(t *testing.T) { if callcount != 0 { t.Errorf("Something incremented call count before we were expecting it.") } - hs.dispatch(nil, &Line{Cmd: "One"}) + hs.dispatch(c, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 4 { t.Errorf("Our handler wasn't called four times :-(") @@ -107,7 +112,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in 3 additions. - hs.dispatch(nil, &Line{Cmd: "One"}) + hs.dispatch(c, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 7 { t.Errorf("Our handler wasn't called three times :-(") @@ -129,7 +134,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in 2 additions. - hs.dispatch(nil, &Line{Cmd: "One"}) + hs.dispatch(c, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 9 { t.Errorf("Our handler wasn't called two times :-(") @@ -151,7 +156,7 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in 1 addition. - hs.dispatch(nil, &Line{Cmd: "One"}) + hs.dispatch(c, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 10 { t.Errorf("Our handler wasn't called once :-(") @@ -170,9 +175,29 @@ func TestHandlerSet(t *testing.T) { } // Dispatch should result in NO additions. - hs.dispatch(nil, &Line{Cmd: "One"}) + hs.dispatch(c, &Line{Cmd: "One"}) <-time.After(time.Millisecond) if callcount != 10 { t.Errorf("Our handler was called?") } } + +func TestPanicRecovery(t *testing.T) { + c, s := setUp(t) + defer s.tearDown() + + recovered := false + c.cfg.Recover = func(conn *Conn, line *Line) { + if err, ok := recover().(string); ok && err == "panic!" { + recovered = true + } + } + c.HandleFunc(PRIVMSG, func(conn *Conn, line *Line) { + panic("panic!") + }) + c.in <- parseLine(":nick!user@host.com PRIVMSG #channel :OH NO PIGEONS") + <-time.After(time.Millisecond) + if recovered != true { + t.Errorf("Failed to recover panic!") + } +}