Merge pull request #66 from rollbar/wj-wrap-with-args

Wrap functions with arguments, and add Rollbar to defer/recover functions

client.go

@@ -464,60 +464,77 @@ func (c *Client) RequestMessageWithExtrasAndContext(ctx context.Context, level s
 
 // -- Panics
 
-// Wrap calls f and then recovers and reports a panic to Rollbar if it occurs.
-// If an error is captured it is subsequently returned
-func (c *Client) Wrap(f func()) (err interface{}) {
-	defer func() {
-		err = recover()
-		switch val := err.(type) {
-		case nil:
+// LogPanic accepts an error value returned by recover() and
+// handles logging to Rollbar with stack info.
+func (c *Client) LogPanic(err interface{}, wait bool) {
+	switch val := err.(type) {
+	case nil:
+		return
+	case error:
+		if c.configuration.checkIgnore(val.Error()) {
 			return
-		case error:
-			if c.configuration.checkIgnore(val.Error()) {
-				return
-			}
-			c.ErrorWithStackSkip(CRIT, val, 2)
-		default:
-			str := fmt.Sprint(val)
-			if c.configuration.checkIgnore(str) {
-				return
-			}
-			errValue := errors.New(str)
-			c.ErrorWithStackSkip(CRIT, errValue, 2)
 		}
-	}()
+		c.ErrorWithStackSkip(CRIT, val, 2)
+	default:
+		str := fmt.Sprint(val)
+		if c.configuration.checkIgnore(str) {
+			return
+		}
+		errValue := errors.New(str)
+		c.ErrorWithStackSkip(CRIT, errValue, 2)
+	}
+	if wait {
+		c.Wait()
+	}
+}
+
+// WrapWithArgs calls f with the supplied args and reports a panic to Rollbar if it occurs.
+// If wait is true, this also waits before returning to ensure the message was reported.
+// If an error is captured it is subsequently returned.
+// WrapWithArgs is compatible with any return type for f, but does not return its return value(s).
+func (c *Client) WrapWithArgs(f interface{}, wait bool, inArgs ...interface{}) (err interface{}) {
+	if f == nil {
+		err = fmt.Errorf("function is nil")
+		return
+	}
+	funcType := reflect.TypeOf(f)
+	funcValue := reflect.ValueOf(f)
+
+	if funcType.Kind() != reflect.Func {
+		err = fmt.Errorf("function kind %s is not %s", funcType.Kind(), reflect.Func)
+		return
+	}
+
+	argValues := make([]reflect.Value, len(inArgs))
+	for i, v := range inArgs {
+		argValues[i] = reflect.ValueOf(v)
+	}
+
+	handler := func(args []reflect.Value) []reflect.Value {
+		defer func() {
+			err = recover()
+			c.LogPanic(err, wait)
+		}()
+
+		return funcValue.Call(args)
+	}
+
+	handler(argValues)
 
-	f()
 	return
 }
 
+// Wrap calls f and then recovers and reports a panic to Rollbar if it occurs.
+// If an error is captured it is subsequently returned.
+func (c *Client) Wrap(f interface{}, args ...interface{}) (err interface{}) {
+	return c.WrapWithArgs(f, false, args...)
+}
+
 // WrapAndWait calls f, and recovers and reports a panic to Rollbar if it occurs.
 // This also waits before returning to ensure the message was reported
 // If an error is captured it is subsequently returned.
-func (c *Client) WrapAndWait(f func()) (err interface{}) {
-	defer func() {
-		err = recover()
-		switch val := err.(type) {
-		case nil:
-			return
-		case error:
-			if c.configuration.checkIgnore(val.Error()) {
-				return
-			}
-			c.ErrorWithStackSkip(CRIT, val, 2)
-		default:
-			str := fmt.Sprint(val)
-			if c.configuration.checkIgnore(str) {
-				return
-			}
-			errValue := errors.New(str)
-			c.ErrorWithStackSkip(CRIT, errValue, 2)
-		}
-		c.Wait()
-	}()
-
-	f()
-	return
+func (c *Client) WrapAndWait(f interface{}, args ...interface{}) (err interface{}) {
+	return c.WrapWithArgs(f, true, args...)
 }
 
 // LambdaWrapper calls handlerFunc with arguments, and recovers and reports a
@@ -537,23 +554,7 @@ func (c *Client) LambdaWrapper(handlerFunc interface{}) interface{} {
 	handler := func(args []reflect.Value) []reflect.Value {
 		defer func() {
 			err := recover()
-			switch val := err.(type) {
-			case nil:
-				return
-			case error:
-				if c.configuration.checkIgnore(val.Error()) {
-					return
-				}
-				c.ErrorWithStackSkip(CRIT, val, 2)
-			default:
-				str := fmt.Sprint(val)
-				if c.configuration.checkIgnore(str) {
-					return
-				}
-				errValue := errors.New(str)
-				c.ErrorWithStackSkip(CRIT, errValue, 2)
-			}
-			c.Wait()
+			c.LogPanic(err, true)
 		}()
 
 		ret := handlerValue.Call(args)

client_test.go

@@ -8,6 +8,7 @@ import (
 	"reflect"
 	"testing"
 	"strings"
+	"fmt"
 )
 
 type TestTransport struct {
@@ -39,6 +40,28 @@ func testClient() *rollbar.Client {
 	return c
 }
 
+func TestLogPanic(t *testing.T) {
+	client := testClient()
+	client.LogPanic(errors.New("logged error"), false)
+	if transport, ok := client.Transport.(*TestTransport); ok {
+		if transport.WaitCalled {
+			t.Error("Wait called unexpectedly")
+		}
+		body := transport.Body
+		if body["data"] == nil {
+			t.Error("body should have data")
+		}
+		data := body["data"].(map[string]interface{})
+		dataError := errorFromData(data)
+		if dataError["message"] != "logged error" {
+			t.Error("data should have correct error message")
+		}
+	} else {
+		t.Fail()
+	}
+	client.Close()
+}
+
 func TestWrap(t *testing.T) {
 	client := testClient()
 	err := errors.New("bork")
@@ -58,6 +81,33 @@ func TestWrap(t *testing.T) {
 	client.Close()
 }
 
+func TestWrapWithArgs(t *testing.T) {
+	client := testClient()
+	result := client.Wrap(func(foo string, num int) (string, int) {
+		panic(fmt.Errorf("%v-%v", foo, num))
+	}, "foo", 42)
+	if fmt.Sprintf("%T", result) != "*errors.errorString" {
+		t.Error("Return value should be error type")
+	}
+	if transport, ok := client.Transport.(*TestTransport); ok {
+		if transport.WaitCalled {
+			t.Error("Wait called unexpectedly")
+		}
+		body := transport.Body
+		if body["data"] == nil {
+			t.Error("body should have data")
+		}
+		data := body["data"].(map[string]interface{})
+		dataError := errorFromData(data)
+		if dataError["message"] != "foo-42" {
+			t.Error("data should have correct error message")
+		}
+	} else {
+		t.Fail()
+	}
+	client.Close()
+}
+
 func TestWrapNonError(t *testing.T) {
 	client := testClient()
 	err := "hello rollbar"
@@ -583,3 +633,9 @@ func configuredOptionsFromData(data map[string]interface{}) map[string]interface
 	configuredOptions := diagnostic["configuredOptions"].(map[string]interface{})
 	return configuredOptions
 }
+
+func errorFromData(data map[string]interface{}) map[string]interface{} {
+	body := data["body"].(map[string]interface{})
+	traceChain := body["trace_chain"].([]map[string]interface{})
+	return traceChain[0]["exception"].(map[string]interface{})
+}

doc.go

@@ -34,7 +34,7 @@ We provide two implementations of the `Transport` interface, `AsyncTransport` an
 
 Handling Panics
 
-Go does not provide a mechanism for handling all panics automatically, therefore we provide two functions `Wrap` and `WrapAndWait` to make working with panics easier. They both take a function and then report to Rollbar if that function panics. They use the recover mechanism to capture the panic, and therefore if you wish your process to have the normal behaviour on panic (i.e. to crash), you will need to re-panic the result of calling `Wrap`. For example,
+Go does not provide a mechanism for handling all panics automatically, therefore we provide two functions `Wrap` and `WrapAndWait` to make working with panics easier. They both take a function with arguments and then report to Rollbar if that function panics. They use the recover mechanism to capture the panic, and therefore if you wish your process to have the normal behaviour on panic (i.e. to crash), you will need to re-panic the result of calling `Wrap`. For example,
 
   package main
 
@@ -43,13 +43,14 @@ Go does not provide a mechanism for handling all panics automatically, therefore
     "github.com/rollbar/rollbar-go"
   )
 
-  func PanickyFunction() {
+  func PanickyFunction(arg string) string {
     panic(errors.New("AHHH!!!!"))
+    return arg
   }
 
   func main() {
     rollbar.SetToken("MY_TOKEN")
-    err := rollbar.Wrap(PanickyFunction)
+    err := rollbar.Wrap(PanickyFunction, "function arg1")
     if err != nil {
       // This means our function panic'd
       // Uncomment the next line to get normal
@@ -61,6 +62,20 @@ Go does not provide a mechanism for handling all panics automatically, therefore
 
 The above pattern of calling `Wrap(...)` and then `Wait(...)` can be combined via `WrapAndWait(...)`. When `WrapAndWait(...)` returns if there was a panic it has already been sent to the Rollbar API. The error is still returned by this function if there is one.
 
+`Wrap` and `WrapAndWait` will accept functions with any number and type of arguments and return values. However, they do not return the function's return value, instead returning the error value. To add Rollbar panic handling to a function while preserving access to the function's return values, we provide the `LogPanic` helper designed to be used inside your deferred function.
+
+  func PanickyFunction(arg string) string {
+    defer func() {
+      err := recover()
+      rollbar.LogPanic(err, true) // bool argument sets wait behavior
+    }()
+
+    panic(errors.New("AHHH!!!!"))
+    return arg
+  }
+
+This offers virtually the same functionality as `Wrap` and `WrapAndWait` while preserving access to the function return values.
+
 Tracing Errors
 
 Due to the nature of the `error` type in Go, it can be difficult to attribute errors to their original origin without doing some extra work. To account for this, we provide multiple ways of configuring the client to unwrap errors and extract stack traces.

rollbar.go

@@ -588,17 +588,31 @@ func Close() {
 	std.Close()
 }
 
+// LogPanic accepts an error value returned by recover() and
+// handles logging to Rollbar with stack info.
+func LogPanic(err interface{}, wait bool) {
+	std.LogPanic(err, wait)
+}
+
+// WrapWithArgs calls f with the supplied args and reports a panic to Rollbar if it occurs.
+// If wait is true, this also waits before returning to ensure the message was reported.
+// If an error is captured it is subsequently returned.
+// WrapWithArgs is compatible with any return type for f, but does not return its return value(s).
+func WrapWithArgs(f interface{}, wait bool, args ...interface{}) interface{} {
+	return std.WrapWithArgs(f, wait, args...)
+}
+
 // Wrap calls f and then recovers and reports a panic to Rollbar if it occurs.
 // If an error is captured it is subsequently returned.
-func Wrap(f func()) interface{} {
-	return std.Wrap(f)
+func Wrap(f interface{}, args ...interface{}) interface{} {
+	return std.WrapWithArgs(f, false, args...)
 }
 
 // WrapAndWait calls f, and recovers and reports a panic to Rollbar if it occurs.
 // This also waits before returning to ensure the message was reported.
 // If an error is captured it is subsequently returned.
-func WrapAndWait(f func()) interface{} {
-	return std.WrapAndWait(f)
+func WrapAndWait(f interface{}, args ...interface{}) interface{} {
+	return std.WrapWithArgs(f, true, args...)
 }
 
 // LambdaWrapper calls handlerFunc with arguments, and recovers and reports a