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

Add support for calling `notify()` to report unhandled exceptions

Open stevenbenitez opened this issue 5 years ago • 5 comments

I just integrated Bugsnag into a Dropwizard application. In this application I have a mechanism in place to catch unhandled exceptions, log them, delegate to my own ErrorReporter interface, and then return an appropriate JSON response (e.g., 500 ISE).

I integrated Bugsnag by creating a BugsnagErrorReporter and suppressing the automatic uncaught exception handler. This is working great, but as my implementation calls bugsnag.notify() it reports all exceptions as handled. I would like to be able to report these as unhandled exceptions since they're being caught by a JAX-RS ExceptionMapper rather than in a catch block somewhere. There are other cases where I would want to report handled exceptions (i.e., places where I can gracefully recover but still want to know of a problem). This is similar in concept to how the Spring module is specifying a SeverityReasonType of REASON_UNHANDLED_EXCEPTION_MIDDLEWARE in its various handlers.

Looking at Report, the getUnhandled() method drives this setting in the JSON sent to Bugsnag and that method keys off of the HandledState.

Implementation Options:

  1. Alter the bugsnag.notify() methods. There are a number of overloads and I am hesitant to disrupt this since it's the primary API a user would interact with.

  2. Alter the Report class to provide a way to flag the report as unhandled. e.g., report.flagAsUnhandled(), report.flagAsUnhandledFromMiddleware(), or etc. This would change the underlying HandledState.

  3. Place my BugsnagErrorReporter in the com.bugsnag package in my own code so that I can access package-private functionality. This would solve my immediate problem but would require me to use a non-public API that could change later and doesn't help anyone else who may want to report unhandled exceptions themselves.

I would be happy to create a PR that implements this, but I wanted to check with you first to see if this is something you agree with and/or if you have a preferred approach.

stevenbenitez avatar Jun 16 '19 02:06 stevenbenitez

Thanks for getting in touch with a detailed report @stevenbenitez. We're aware that in certain scenarios it would be helpful to be able to toggle whether an error is handled/unhandled, and are deciding what the best approach would be for all the languages we support before we add this to our roadmap. Option 2 seems to be the most likely route we'll take, but we're still in the design phase and this is subject to change, so we wouldn't accept a PR at this stage.

If you were comfortable with modifying a non-public API then I believe modifying severityReason.type to REASON_UNHANDLED_EXCEPTION in the JSON payload should have the effect of changing whether an error is handled/unhandled. We can't guarantee that this won't change in a future release, but this would solve the problem in hand.

fractalwrench avatar Jun 17 '19 09:06 fractalwrench

Thanks for the response, @fractalwrench. I can appreciate wanting to design a consistent API for all of your languages/notifiers first. I'll pursue option 3 or your suggestion and then switch to the official solution when that is available. Thanks for building such a slick product!

stevenbenitez avatar Jun 17 '19 13:06 stevenbenitez

Is there any movement on this issue? Not being able to flag errors as 'unhandled' makes the stability score very difficult to use for java apps.

sdufel avatar Jun 04 '20 17:06 sdufel

Hi @sdufel, This has increased in priority recently so we are planning to work on this soon but as yet don't have a definite timeframe.

johnkiely1 avatar Jun 12 '20 14:06 johnkiely1