Improve handler panic reporting (#145)

Stack traces for handler panics were lost, making it hard to determine
where the panic actually happened. Now, stack traces are captured by a
special error type so that handlers can print them. By default, nothing
actually changes, but I plan to extend the default configuration of
go-baseapp so that these errors are formatted with stack traces in logs.

I copied the error code from my implementation in `bluekeyes/hatpear` so
that they'll share the same interface. The `pkg/errors` package has a
similar interface, but uses package-specific types. Using standard
library types seemed preferrable, even though we have a dependency on
`pkg/errors` already.

githubapp/errors.go

@@ -16,6 +16,8 @@ package githubapp
 
 import (
 	"fmt"
+	"io"
+	"runtime"
 
 	"github.com/rcrowley/go-metrics"
 )
@@ -24,6 +26,12 @@ const (
 	MetricsKeyHandlerError = "github.handler.error"
 )
 
+var (
+	// HandlerRecoverStackDepth is the max depth of stack trace to recover on a
+	// handler panic.
+	HandlerRecoverStackDepth = 32
+)
+
 func errorCounter(r metrics.Registry, event string) metrics.Counter {
 	if r == nil {
 		return metrics.NilCounter{}
@@ -35,3 +43,69 @@ func errorCounter(r metrics.Registry, event string) metrics.Counter {
 	}
 	return metrics.GetOrRegisterCounter(key, r)
 }
+
+// HandlerPanicError is an error created from a recovered handler panic.
+type HandlerPanicError struct {
+	value interface{}
+	stack []runtime.Frame
+}
+
+// Value returns the exact value with which panic() was called.
+func (e HandlerPanicError) Value() interface{} {
+	return e.value
+}
+
+// StackTrace returns the stack of the panicking goroutine.
+func (e HandlerPanicError) StackTrace() []runtime.Frame {
+	return e.stack
+}
+
+// Format formats the error optionally including the stack trace.
+//
+//   %s    the error message
+//   %v    the error message and the source file and line number for each stack frame
+//
+// Format accepts the following flags:
+//
+//   %+v   the error message and the function, file, and line for each stack frame
+//
+func (e HandlerPanicError) Format(s fmt.State, verb rune) {
+	switch verb {
+	case 's':
+		_, _ = io.WriteString(s, e.Error())
+	case 'v':
+		_, _ = io.WriteString(s, e.Error())
+		for _, f := range e.stack {
+			_, _ = io.WriteString(s, "\n")
+			if s.Flag('+') {
+				_, _ = fmt.Fprintf(s, "%s\n\t", f.Function)
+			}
+			_, _ = fmt.Fprintf(s, "%s:%d", f.File, f.Line)
+		}
+	}
+}
+
+func (e HandlerPanicError) Error() string {
+	v := e.value
+	if err, ok := v.(error); ok {
+		v = err.Error()
+	}
+	return fmt.Sprintf("panic: %v", v)
+}
+
+func getStack(skip int) []runtime.Frame {
+	rpc := make([]uintptr, HandlerRecoverStackDepth)
+
+	n := runtime.Callers(skip+2, rpc)
+	frames := runtime.CallersFrames(rpc[0:n])
+
+	var stack []runtime.Frame
+	for {
+		f, more := frames.Next()
+		if !more {
+			break
+		}
+		stack = append(stack, f)
+	}
+	return stack
+}

githubapp/scheduler.go

@@ -16,7 +16,6 @@ package githubapp
 
 import (
 	"context"
-	"fmt"
 	"sync/atomic"
 	"time"
 
@@ -57,8 +56,10 @@ func (d Dispatch) Execute(ctx context.Context) error {
 }
 
 // AsyncErrorCallback is called by an asynchronous scheduler when an event
-// handler returns an error. The error from the handler is passed directly as
-// the final argument.
+// handler returns an error or panics. The error from the handler is passed
+// directly as the final argument.
+//
+// If the handler panics, err will be a HandlerPanicError.
 type AsyncErrorCallback func(ctx context.Context, d Dispatch, err error)
 
 // DefaultAsyncErrorCallback logs errors.
@@ -168,10 +169,9 @@ func (s *scheduler) safeExecute(ctx context.Context, d Dispatch) {
 	defer func() {
 		atomic.AddInt64(&s.activeWorkers, -1)
 		if r := recover(); r != nil {
-			if rerr, ok := r.(error); ok {
-				err = rerr
-			} else {
-				err = fmt.Errorf("%v", r)
+			err = HandlerPanicError{
+				value: r,
+				stack: getStack(1),
 			}
 		}
 		if err != nil && s.onError != nil {