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 a Closer function to the notifier #67

Open
sagikazarmark opened this issue Jul 26, 2018 · 4 comments
Open

Add a Closer function to the notifier #67

sagikazarmark opened this issue Jul 26, 2018 · 4 comments
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature

Comments

@sagikazarmark
Copy link

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

@fractalwrench
Copy link
Contributor

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.

@sagikazarmark
Copy link
Author

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

@countingtoten
Copy link

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.

@mattdyoung
Copy link

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 mattdyoung added the feature request Request for a new feature label Oct 8, 2019
@abigailbramble abigailbramble added the backlog We hope to fix this feature/bug in the future label Oct 11, 2019
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

5 participants