bugsnag-android icon indicating copy to clipboard operation
bugsnag-android copied to clipboard

Unable to notify with specific severity and have that severity reflected in other OnErrorCallback

Open elihart opened this issue 4 years ago • 3 comments

Description

If I want to notify a handled issue with Error severity I must use a callback like

  Bugsnag.notify(throwable) { event ->
            event.severity = Severity.Error
            true
}

However, if there are any other OnErrorCallback registered on the Bugsnag client those callbacks will not see this intended severity because the Client#notifyInternal implementation calls other OnErrorCallback before the callback of the notify call

if (!callbackState.runOnErrorTasks(event, logger)
                || (onError != null && !onError.onError(event))) {

As far as I can tell this makes it impossible for our other callbacks to see the intended severity, and we can't react to it with any custom code that we may need.

Describe the solution you'd like Ideally I'd like to see a Severity parameter added to notify.

Describe alternatives you've considered Since that was removed in the 5.0 upgrade it doesn't seem like you want to provide that, so then maybe the callback order can be switched, so that the OnErrorCallback passed to notify can be called before all other OnErrorCallback.

elihart avatar Dec 16 '20 04:12 elihart

Hi @elihart

The ordering of the callbacks is by design. Our thinking is that the primary purpose of callbacks would be to modify the error report, and that the closest callback to the original code should be able to override any callbacks with wider scope.

i.e. you should be able to implement a global callback overriding the severity but then override that further in a callback on the specific notify call.

Can you share more details of your use case so we can consider whether there's another way we could support this better? E.g. how are you reacting to the overridden severity? Is this to modify another aspect of what's sent to Bugsnag?

mattdyoung avatar Dec 18 '20 15:12 mattdyoung

how are you reacting to the overridden severity? Is this to modify another aspect of what's sent to Bugsnag?

This is not to modify anything that is sent to bugsnag. The purpose is to have other, decoupled systems in our infrastructure be able to recognize when a handled error is sent.

We have a few uses for this:

  • make custom logs to our internal servers upon a handled error
  • During integration testing have handled errors be fatal

While we could have the handled error call site hook into these custom behaviors, it would be much cleaner to rely on other Bugsnag error callbacks so that it can be decoupled. For example, for our instrumentation testing I don't want to make any changes to our production code but instead add a OnErrorCallback in the test sources that will fail the test if a handled error is sent. Due to the callback ordering this is not currently possible to do cleanly so our production code must currently be modified to enable this.

elihart avatar Dec 27 '20 17:12 elihart

Hi @elihart

Thanks for the information, that's really helpful. We'll consider what our best options are for improvements here. For example, one possibility would be we could provide a new callback which always runs on the final report sent to Bugsnag.

mattdyoung avatar Jan 06 '21 11:01 mattdyoung