From ae38f69335f455f101ca305ce43c20b450f6e326 Mon Sep 17 00:00:00 2001 From: Delisa Mason Date: Thu, 29 Oct 2020 16:05:52 +0000 Subject: [PATCH 1/2] feat: Support modifying events in a callback function --- CHANGELOG.md | 8 ++++++ event.go | 10 +++++++ features/fixtures/app/main.go | 11 +++++++ features/plain_features/handled.feature | 11 +++++++ features/steps/go_steps.rb | 9 ++++++ notifier_test.go | 38 +++++++++++++++++++------ 6 files changed, 79 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ed43704..95c41ff4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ * Extract stacktrace contents on errors wrapped by [`pkg/errors`](https://github.com/pkg/errors). +* Support modifying an individual event using a callback function argument. + + ```go + bugsnag.Notify(err, func(event *bugsnag.Event) { + event.ErrorClass = "Unexpected Termination" + event.MetaData.Update(loadJobData()) + }) + ``` ### Bug fixes diff --git a/event.go b/event.go index cefbf65e..e1f72f26 100644 --- a/event.go +++ b/event.go @@ -123,6 +123,7 @@ func newEvent(rawData []interface{}, notifier *Notifier) (*Event, *Configuration } var err *errors.Error + var callbacks []func(*Event) for _, datum := range event.RawData { switch datum := datum.(type) { @@ -169,6 +170,8 @@ func newEvent(rawData []interface{}, notifier *Notifier) (*Event, *Configuration case HandledState: event.handledState = datum event.Severity = datum.OriginalSeverity + case func(*Event): + callbacks = append(callbacks, datum) } } @@ -192,6 +195,13 @@ func newEvent(rawData []interface{}, notifier *Notifier) (*Event, *Configuration } } + for _, callback := range callbacks { + callback(event) + if event.Severity != event.handledState.OriginalSeverity { + event.handledState.SeverityReason = SeverityReasonCallbackSpecified + } + } + return event, config } diff --git a/features/fixtures/app/main.go b/features/fixtures/app/main.go index 8330184a..086a81c5 100644 --- a/features/fixtures/app/main.go +++ b/features/fixtures/app/main.go @@ -76,6 +76,8 @@ func main() { unhandledCrash() case "handled", "endpoint legacy", "endpoint notify", "endpoint session": handledError() + case "handled with callback": + handledCallbackError() case "session": session() case "autonotify": @@ -231,3 +233,12 @@ func user() { time.Sleep(200 * time.Millisecond) } + +func handledCallbackError() { + bugsnag.Notify(fmt.Errorf("Inadequent Prep Error"), func(event *bugsnag.Event) { + event.Context = "nonfatal.go:14" + event.Severity = bugsnag.SeverityInfo + }) + // Give some time for the error to be sent before exiting + time.Sleep(200 * time.Millisecond) +} diff --git a/features/plain_features/handled.feature b/features/plain_features/handled.feature index e74c38eb..8a5a58a2 100644 --- a/features/plain_features/handled.feature +++ b/features/plain_features/handled.feature @@ -26,3 +26,14 @@ Scenario: A handled error sends a report with a custom name And the event "severityReason.type" equals "handledError" And the exception "errorClass" equals "MyCustomErrorClass" And the "file" of stack frame 0 equals "main.go" + +Scenario: Sending an event using a callback to modify report contents + When I run the go service "app" with the test case "handled with callback" + Then I wait to receive a request + And the request is a valid error report with api key "a35a2a72bd230ac0aa0f52715bbdc6aa" + And the event "unhandled" is false + And the event "severity" equals "info" + And the event "severityReason.type" equals "userCallbackSetSeverity" + And the event "context" equals "nonfatal.go:14" + And the "file" of stack frame 0 equals "main.go" + And stack frame 0 contains a local function spanning 238 to 244 diff --git a/features/steps/go_steps.rb b/features/steps/go_steps.rb index 337abcee..0fed1494 100644 --- a/features/steps/go_steps.rb +++ b/features/steps/go_steps.rb @@ -60,3 +60,12 @@ sleep 1 assert_equal(request_count, stored_requests.size, "#{stored_requests.size} requests received") end + +Then("stack frame {int} contains a local function spanning {int} to {int}") do |frame, val, old_val| + # Old versions of Go put the line number on the end of the function + if ['1.7', '1.8'].include? ENV['GO_VERSION'] + step "the \"lineNumber\" of stack frame #{frame} equals #{old_val}" + else + step "the \"lineNumber\" of stack frame #{frame} equals #{val}" + end +end diff --git a/notifier_test.go b/notifier_test.go index 9662957a..32b3fcb9 100644 --- a/notifier_test.go +++ b/notifier_test.go @@ -109,14 +109,36 @@ func TestStackframesAreSkippedCorrectly(t *testing.T) { }) } -func assertStackframeCount(t *testing.T, expCount int) { - report, _ := simplejson.NewJson(<-bugsnaggedReports) - stacktrace := GetIndex(GetIndex(report, "events", 0), "exceptions", 0).Get("stacktrace") - if s := stacktrace.MustArray(); len(s) != expCount { - t.Errorf("Expected %d stackframe(s), but there were %d stackframes", expCount, len(s)) - s, _ := stacktrace.EncodePretty() - t.Errorf(string(s)) - } +func TestModifyingEventsWithCallbacks(t *testing.T) { + server, eventQueue := Setup() + defer server.Close() + notifier := notifierSetup(server.URL) + + bugsnag.Configure(bugsnag.Configuration{ + APIKey: TestAPIKey, + Endpoints: bugsnag.Endpoints{Notify: server.URL, Sessions: server.URL + "/sessions"}, + }) + + t.Run("bugsnag.Notify with block", func(st *testing.T) { + notifier.Notify(fmt.Errorf("bnuuy"), bugsnag.Context{String: "should be overridden"}, func(event *bugsnag.Event) { + event.Context = "known unknowns" + }) + json, _ := simplejson.NewJson(<-eventQueue) + event := GetIndex(json, "events", 0) + context := event.Get("context").MustString() + exception := GetIndex(event, "exceptions", 0) + class := exception.Get("errorClass").MustString() + message := exception.Get("message").MustString() + if class != "*errors.errorString" { + st.Errorf("incorrect error class '%s'", class) + } + if message != "bnuuy" { + st.Errorf("incorrect error message '%s'", message) + } + if context != "known unknowns" { + st.Errorf("failed to change context in block. '%s'", context) + } + }) } func assertStackframesMatch(t *testing.T, expected []errors.StackFrame) { From 3c259da50d5e12128060dd0c52d05a92986316a0 Mon Sep 17 00:00:00 2001 From: Delisa Mason Date: Thu, 29 Oct 2020 16:29:31 +0000 Subject: [PATCH 2/2] feat: Support modifying the stacktrace within callbacks StackFrame is now public so the values can be inspected prior to editing --- CHANGELOG.md | 7 +++++++ bugsnag_test.go | 6 +++--- event.go | 8 ++++---- features/fixtures/app/main.go | 3 +++ features/plain_features/handled.feature | 2 ++ payload_test.go | 2 +- report.go | 2 +- 7 files changed, 21 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95c41ff4..84e4f0a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,9 +12,16 @@ bugsnag.Notify(err, func(event *bugsnag.Event) { event.ErrorClass = "Unexpected Termination" event.MetaData.Update(loadJobData()) + + if event.Stacktrace[0].File = "mylogger.go" { + event.Stacktrace = event.Stacktrace[1:] + } }) ``` + The stack trace of an event is now mutable so frames can be removed or + modified. + ### Bug fixes * Send web framework name with severity reason if set. Previously this value was diff --git a/bugsnag_test.go b/bugsnag_test.go index f23e6de6..4a2781af 100644 --- a/bugsnag_test.go +++ b/bugsnag_test.go @@ -132,7 +132,7 @@ func TestNotify(t *testing.T) { } exception := getIndex(event, "exceptions", 0) - verifyExistsInStackTrace(t, exception, &stackFrame{File: "bugsnag_test.go", Method: "TestNotify", LineNumber: 98, InProject: true}) + verifyExistsInStackTrace(t, exception, &StackFrame{File: "bugsnag_test.go", Method: "TestNotify", LineNumber: 98, InProject: true}) } type testPublisher struct { @@ -315,7 +315,7 @@ func TestHandler(t *testing.T) { } exception := getIndex(event, "exceptions", 0) - verifyExistsInStackTrace(t, exception, &stackFrame{File: "bugsnag_test.go", Method: "crashyHandler", InProject: true, LineNumber: 24}) + verifyExistsInStackTrace(t, exception, &StackFrame{File: "bugsnag_test.go", Method: "crashyHandler", InProject: true, LineNumber: 24}) } func TestAutoNotify(t *testing.T) { @@ -611,7 +611,7 @@ func assertValidSession(t *testing.T, event *simplejson.Json, unhandled bool) { } } -func verifyExistsInStackTrace(t *testing.T, exception *simplejson.Json, exp *stackFrame) { +func verifyExistsInStackTrace(t *testing.T, exception *simplejson.Json, exp *StackFrame) { isFile := func(frame *simplejson.Json) bool { return strings.HasSuffix(getString(frame, "file"), exp.File) } isMethod := func(frame *simplejson.Json) bool { return getString(frame, "method") == exp.Method } isLineNumber := func(frame *simplejson.Json) bool { return getInt(frame, "lineNumber") == exp.LineNumber } diff --git a/event.go b/event.go index e1f72f26..c402d50c 100644 --- a/event.go +++ b/event.go @@ -43,7 +43,7 @@ type severity struct { } // The form of stacktrace that Bugsnag expects -type stackFrame struct { +type StackFrame struct { Method string `json:"method"` File string `json:"file"` LineNumber int `json:"lineNumber"` @@ -85,7 +85,7 @@ type Event struct { // The error message to be sent to Bugsnag. This defaults to the return value of Error.Error() Message string // The stacktrrace of the error to be sent to Bugsnag. - Stacktrace []stackFrame + Stacktrace []StackFrame // The context to be sent to Bugsnag. This should be set to the part of the app that was running, // e.g. for http requests, set it to the path. @@ -136,7 +136,7 @@ func newEvent(rawData []interface{}, notifier *Notifier) (*Event, *Configuration event.ErrorClass = err.TypeName() } event.Message = err.Error() - event.Stacktrace = make([]stackFrame, len(err.StackFrames())) + event.Stacktrace = make([]StackFrame, len(err.StackFrames())) case bool: config = config.merge(&Configuration{Synchronous: bool(datum)}) @@ -187,7 +187,7 @@ func newEvent(rawData []interface{}, notifier *Notifier) (*Event, *Configuration file = config.stripProjectPackages(file) } - event.Stacktrace[i] = stackFrame{ + event.Stacktrace[i] = StackFrame{ Method: frame.Name, File: file, LineNumber: frame.LineNumber, diff --git a/features/fixtures/app/main.go b/features/fixtures/app/main.go index 086a81c5..e89e1e64 100644 --- a/features/fixtures/app/main.go +++ b/features/fixtures/app/main.go @@ -238,6 +238,9 @@ func handledCallbackError() { bugsnag.Notify(fmt.Errorf("Inadequent Prep Error"), func(event *bugsnag.Event) { event.Context = "nonfatal.go:14" event.Severity = bugsnag.SeverityInfo + + event.Stacktrace[1].File = ">insertion<" + event.Stacktrace[1].LineNumber = 0 }) // Give some time for the error to be sent before exiting time.Sleep(200 * time.Millisecond) diff --git a/features/plain_features/handled.feature b/features/plain_features/handled.feature index 8a5a58a2..d1a34476 100644 --- a/features/plain_features/handled.feature +++ b/features/plain_features/handled.feature @@ -37,3 +37,5 @@ Scenario: Sending an event using a callback to modify report contents And the event "context" equals "nonfatal.go:14" And the "file" of stack frame 0 equals "main.go" And stack frame 0 contains a local function spanning 238 to 244 + And the "file" of stack frame 1 equals ">insertion<" + And the "lineNumber" of stack frame 1 equals 0 diff --git a/payload_test.go b/payload_test.go index 6e909f52..4aa0c0df 100644 --- a/payload_test.go +++ b/payload_test.go @@ -43,7 +43,7 @@ func TestMarshalLargePayload(t *testing.T) { } func makeLargePayload() *payload { - stackframes := []stackFrame{ + stackframes := []StackFrame{ {Method: "doA", File: "a.go", LineNumber: 65, InProject: false}, {Method: "fetchB", File: "b.go", LineNumber: 99, InProject: true}, {Method: "incrementI", File: "i.go", LineNumber: 651, InProject: false}, diff --git a/report.go b/report.go index 944f201e..bd2b92b2 100644 --- a/report.go +++ b/report.go @@ -49,7 +49,7 @@ type appJSON struct { type exceptionJSON struct { ErrorClass string `json:"errorClass"` Message string `json:"message"` - Stacktrace []stackFrame `json:"stacktrace"` + Stacktrace []StackFrame `json:"stacktrace"` } type severityReasonJSON struct {