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

Add support for calling notify() to report unhandled exceptions #145

Open
stevenbenitez opened this issue Jun 16, 2019 · 5 comments
Open
Labels
feature request Request for a new feature scheduled Work is starting on this feature/bug

Comments

@stevenbenitez
Copy link

stevenbenitez commented Jun 16, 2019

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.

@fractalwrench
Copy link
Contributor

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.

@stevenbenitez
Copy link
Author

stevenbenitez commented Jun 17, 2019

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!

@abigailbramble abigailbramble added feature request Request for a new feature scheduled Work is starting on this feature/bug labels Aug 9, 2019
@sdufel
Copy link

sdufel commented Jun 4, 2020

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.

@johnkiely1
Copy link
Member

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.

@msquitieri
Copy link

This is 2.5 years old. Is this being fixed? Is there any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for a new feature scheduled Work is starting on this feature/bug
Projects
None yet
Development

No branches or pull requests

6 participants