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

Add a Closer function to the notifier

Open sagikazarmark opened this issue 5 years ago • 4 comments

Expected behavior

All errors make it to bugsnag, especially panics, even when Synchronous mode is off.

Observed behavior

I may be wrong, but looking at the code: there is no guarantees that in case of a recovered panic with Synchronous mode off an error makes it to bugsnag as the goroutine is not protected from exiting the application (if the panic is so fatal that the application has to exit).

Adding a closer function which can be registered as a deferred function could make sure that before exiting the application all pending errors are actually sent to Bugsnag.

Version

1.3.1

sagikazarmark avatar Jul 26 '18 21:07 sagikazarmark

Thanks for the report @sagikazarmark - I've created a ticket internally to discuss whether this is something the notifier could support automatically in future. In the meantime, our documentation has a section on reporting panics from goroutines which may be helpful depending on your exact use-case.

fractalwrench avatar Aug 01 '18 10:08 fractalwrench

I'm familiar with that documentation, but I believe even then there is a case that the error gets lost.

sagikazarmark avatar Aug 01 '18 15:08 sagikazarmark

I have the same problem. If I log a Fatal error, my program exits before Bugsnag can publish the error when Synchronous mode is off.

Would you be open to adding a Configuration value for a sync.WaitGroup? The reportPublisher can increment/decrement the WaitGroup when it starts a delivery, and I can wait to exit until the WaitGroup is done.

I can create a PR if you're interested but don't have the bandwidth.

countingtoten avatar Oct 01 '19 22:10 countingtoten

Hi @countingtoten

Thanks for the offer, but I'm not sure we'd accept a PR at present. We'd like to more carefully consider other possible scenarios here and decide on the best way forward.

I'd suggest forking the repo in the short term with your proposed approach, and we'll report back on this issue when we've considered the best long term fix.

mattdyoung avatar Oct 08 '19 09:10 mattdyoung