Add support for context.Context

CHANGELOG.md

@@ -4,6 +4,14 @@ CHANGELOG](http://keepachangelog.com/) for how to update this file. This project
 adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased][unreleased]
+### Added
+- Added `honeybadger.FromContext` to retrieve a honeybadger.Context from a
+  context.Context.
+- Added `Context.WithContext` for storing a honeybadger.Context into a
+  context.Context.
+
+### Changed
+- Removed honeybadger.SetContext and client.SetContext (#35) -@gaffneyc
 
 ## [0.5.0] - 2019-10-17
 ### Added

README.md

@@ -156,21 +156,22 @@ honeybadger.Notify(err, honeybadger.Tags{"timeout", "http"})
 
 ---
 
+When using Go's context.Context you can store a honeybadger.Context to build it
+up across multiple middleware. Be aware that honeybadger.Context is not thread
+safe.
+```go
+func(resp http.ResponseWriter, req *http.Request) {
+  // To store a honeybadger.Context (or use honeybadger.Handler which does this for you)
+  hbCtx := honeybadger.Context{}
+  req = req.WithContext(hbCtx.WithContext(req.Context()))
 
-### `honeybadger.SetContext()`: Set metadata to be sent if an error occurs
-
-This method lets you set context data that will be sent if an error should occur.
-
-For example, it's often useful to record the current user's ID when an error occurs in a web app. To do that, just use `SetContext` to set the user id on each request. If an error occurs, the id will be reported with it.
-
-**Note**: This method is currently shared across goroutines, and therefore may not be optimal for use in highly concurrent use cases, such as HTTP requests. See [issue #35](https://github.com/honeybadger-io/honeybadger-go/issues/35).
+  // To add to an existing context
+  hbCtx = honeybadger.FromContext(req.Context())
+  hbCtx["user_id"] = "ID"
 
-#### Examples:
-
-```go
-honeybadger.SetContext(honeybadger.Context{
-  "user_id": 1,
-})
+  // To add the context when sending you can just pass the context.Context
+  honeybadger.Notify(err, ctx)
+}
 ```
 
 ---

client.go

@@ -23,7 +23,6 @@ type noticeHandler func(*Notice) error
 // the configuration and implements the public API.
 type Client struct {
 	Config               *Configuration
-	context              *contextSync
 	worker               worker
 	beforeNotifyHandlers []noticeHandler
 }
@@ -33,11 +32,6 @@ func (client *Client) Configure(config Configuration) {
 	client.Config.update(&config)
 }
 
-// SetContext updates the client context with supplied context.
-func (client *Client) SetContext(context Context) {
-	client.context.Update(context)
-}
-
 // Flush blocks until the worker has processed its queue.
 func (client *Client) Flush() {
 	client.worker.Flush()
@@ -52,7 +46,6 @@ func (client *Client) BeforeNotify(handler func(notice *Notice) error) {
 
 // Notify reports the error err to the Honeybadger service.
 func (client *Client) Notify(err interface{}, extra ...interface{}) (string, error) {
-	extra = append([]interface{}{client.context.internal}, extra...)
 	notice := newNotice(client.Config, newError(err, 2), extra...)
 	for _, handler := range client.beforeNotifyHandlers {
 		if err := handler(notice); err != nil {
@@ -95,14 +88,20 @@ func (client *Client) Handler(h http.Handler) http.Handler {
 	if h == nil {
 		h = http.DefaultServeMux
 	}
-	fn := func(w http.ResponseWriter, r *http.Request) {
+	fn := func(w http.ResponseWriter, req *http.Request) {
 		defer func() {
 			if err := recover(); err != nil {
-				client.Notify(newError(err, 2), Params(r.Form), getCGIData(r), *r.URL)
+				client.Notify(newError(err, 2), Params(req.Form), getCGIData(req), *req.URL)
 				panic(err)
 			}
 		}()
-		h.ServeHTTP(w, r)
+
+		// Add a fresh Context to the request if one is not already set
+		if hbCtx := FromContext(req.Context()); hbCtx == nil {
+			req = req.WithContext(Context{}.WithContext(req.Context()))
+		}
+
+		h.ServeHTTP(w, req)
 	}
 	return http.HandlerFunc(fn)
 }
@@ -113,9 +112,8 @@ func New(c Configuration) *Client {
 	worker := newBufferedWorker(config)
 
 	client := Client{
-		Config:  config,
-		worker:  worker,
-		context: newContextSync(),
+		Config: config,
+		worker: worker,
 	}
 
 	return &client

client_test.go

@@ -1,7 +1,8 @@
 package honeybadger
 
 import (
-	"sync"
+	"context"
+	"fmt"
 	"testing"
 )
 
@@ -29,48 +30,6 @@ func TestConfigureClientEndpoint(t *testing.T) {
 	}
 }
 
-func TestClientContext(t *testing.T) {
-	client := New(Configuration{})
-
-	client.SetContext(Context{"foo": "bar"})
-	client.SetContext(Context{"bar": "baz"})
-
-	context := client.context.internal
-
-	if context["foo"] != "bar" {
-		t.Errorf("Expected client to merge global context. expected=%#v actual=%#v", "bar", context["foo"])
-	}
-
-	if context["bar"] != "baz" {
-		t.Errorf("Expected client to merge global context. expected=%#v actual=%#v", "baz", context["bar"])
-	}
-}
-
-func TestClientConcurrentContext(t *testing.T) {
-	var wg sync.WaitGroup
-
-	client := New(Configuration{})
-	newContext := Context{"foo": "bar"}
-
-	wg.Add(2)
-
-	go updateContext(&wg, client, newContext)
-	go updateContext(&wg, client, newContext)
-
-	wg.Wait()
-
-	context := client.context.internal
-
-	if context["foo"] != "bar" {
-		t.Errorf("Expected context value. expected=%#v result=%#v", "bar", context["foo"])
-	}
-}
-
-func updateContext(wg *sync.WaitGroup, client *Client, context Context) {
-	client.SetContext(context)
-	wg.Done()
-}
-
 func TestNotifyPushesTheEnvelope(t *testing.T) {
 	client, worker, _ := mockClient(Configuration{})
 
@@ -121,10 +80,43 @@ func mockClient(c Configuration) (Client, *mockWorker, *mockBackend) {
 	backendConfig.update(&c)
 
 	client := Client{
-		Config:  newConfig(*backendConfig),
-		worker:  worker,
-		context: newContextSync(),
+		Config: newConfig(*backendConfig),
+		worker: worker,
 	}
 
 	return client, worker, backend
 }
+
+func TestClientContext(t *testing.T) {
+	backend := NewMemoryBackend()
+
+	client := New(Configuration{
+		APIKey:  "badgers",
+		Backend: backend,
+	})
+
+	err := NewError(fmt.Errorf("which context is which"))
+
+	hbCtx := Context{"user_id": 1}
+	goCtx := Context{"request_id": "1234"}.WithContext(context.Background())
+
+	_, nErr := client.Notify(err, hbCtx, goCtx)
+	if nErr != nil {
+		t.Fatal(nErr)
+	}
+
+	// Flush otherwise backend.Notices will be empty
+	client.Flush()
+
+	if len(backend.Notices) != 1 {
+		t.Fatalf("Notices expected=%d actual=%d", 1, len(backend.Notices))
+	}
+
+	notice := backend.Notices[0]
+	if notice.Context["user_id"] != 1 {
+		t.Errorf("notice.Context[user_id] expected=%d actual=%v", 1, notice.Context["user_id"])
+	}
+	if notice.Context["request_id"] != "1234" {
+		t.Errorf("notice.Context[request_id] expected=%q actual=%v", "1234", notice.Context["request_id"])
+	}
+}

context.go

@@ -1,11 +1,33 @@
 package honeybadger
 
+import "context"
+
 // Context is used to send extra data to Honeybadger.
 type Context hash
 
+// ctxKey is use in WithContext and FromContext to store and load the
+// honeybadger.Context into a context.Context.
+type ctxKey struct{}
+
 // Update applies the values in other Context to context.
-func (context Context) Update(other Context) {
+func (c Context) Update(other Context) {
 	for k, v := range other {
-		context[k] = v
+		c[k] = v
+	}
+}
+
+// WithContext adds the honeybadger.Context to the given context.Context and
+// returns the new context.Context.
+func (c Context) WithContext(ctx context.Context) context.Context {
+	return context.WithValue(ctx, ctxKey{}, c)
+}
+
+// FromContext retrieves a honeybadger.Context from the context.Context.
+// FromContext will return nil if no Honeybadger context exists in ctx.
+func FromContext(ctx context.Context) Context {
+	if c, ok := ctx.Value(ctxKey{}).(Context); ok {
+		return c
 	}
+
+	return nil
 }

context_test.go

@@ -1,6 +1,9 @@
 package honeybadger
 
-import "testing"
+import (
+	"context"
+	"testing"
+)
 
 func TestContextUpdate(t *testing.T) {
 	c := Context{"foo": "bar"}
@@ -9,3 +12,26 @@ func TestContextUpdate(t *testing.T) {
 		t.Errorf("Context should update values. expected=%#v actual=%#v", "baz", c["foo"])
 	}
 }
+
+func TestContext(t *testing.T) {
+	t.Run("setting values is allowed between reads", func(t *testing.T) {
+		ctx := context.Background()
+		ctx = Context{"foo": "bar"}.WithContext(ctx)
+
+		stored := FromContext(ctx)
+		if stored == nil {
+			t.Fatalf("FromContext returned nil")
+		}
+		if stored["foo"] != "bar" {
+			t.Errorf("stored[foo] expected=%q actual=%v", "bar", stored["foo"])
+		}
+
+		// Write a new key then we'll read from the ctx again and make sure it is
+		// still set.
+		stored["baz"] = "qux"
+		stored = FromContext(ctx)
+		if stored["baz"] != "qux" {
+			t.Errorf("stored[baz] expected=%q actual=%v", "qux", stored["baz"])
+		}
+	})
+}

honeybadger.go

@@ -54,11 +54,6 @@ func Configure(c Configuration) {
 	DefaultClient.Configure(c)
 }
 
-// SetContext merges c Context into the Context of the global client.
-func SetContext(c Context) {
-	DefaultClient.SetContext(c)
-}
-
 // Notify reports the error err to the Honeybadger service.
 //
 // The first argument err may be an error, a string, or any other type in which