Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
elihart opened this issue Dec 16, 2020 · 3 comments
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature

Comments

@elihart
Copy link

elihart commented Dec 16, 2020

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.

@mattdyoung
Copy link

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 mattdyoung added the awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. label Dec 18, 2020
@elihart
Copy link
Author

elihart commented Dec 27, 2020

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.

@mattdyoung
Copy link

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 mattdyoung added backlog We hope to fix this feature/bug in the future feature request Request for a new feature and removed awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. labels Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

2 participants