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

Existing async notify reports disappear if app exits. No apparent way to flush them. #200

Open
veqryn opened this issue Sep 6, 2023 · 6 comments
Labels
awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. backlog We hope to fix this feature/bug in the future feature request Request for a new feature

Comments

@veqryn
Copy link
Contributor

veqryn commented Sep 6, 2023

Describe the bug

A default configuration of bugsnag (Synchronous = false) will drop notify reports if the app exits.
There does not appear to be any way to flush these notify reports, or to delay the exit of the application until they have been sent.

Steps to reproduce

package main

import (
	"context"
	"errors"
	"fmt"
	"github.com/bugsnag/bugsnag-go/v2"
)

func main() {
	bugsnag.Configure(bugsnag.Configuration{APIKey: "***"})
	ctx := bugsnag.StartSession(context.Background())

	err := errors.New("my error")
	berr := bugsnag.Notify(err, ctx)
	if berr != nil {
		panic(berr)
	}
	fmt.Println("done")
}

The above golang application will print "done", but no bugs will show up in bugsnag.

If you add time.Sleep(120*time.Second) to the end of it, the bugs will start showing up in bugsnag.

Bugsnag should probably have a buffered channel accumulating errors, with a worker pool publishing the channels to bugsnag's servers. There should be a function you can call that waits for the channel to be empty, or closes the channel and waits for the workers to finish consuming from the channel and exit.

Environment

  • Bugsnag Go version: v2.2.0
  • Go version: v1.21.0
@clr182 clr182 added the needs discussion Requires internal analysis/discussion label Sep 8, 2023
@clr182 clr182 added feature request Request for a new feature backlog We hope to fix this feature/bug in the future and removed needs discussion Requires internal analysis/discussion labels Sep 18, 2023
@clr182
Copy link
Contributor

clr182 commented Sep 18, 2023

Hi @veqryn

Due to a race condition between the parent(application) and child(monitoring) fork there is a possibility that some reports may be lost as the parent process terminates the child process before it has a chance to flush and report all errors to BugSnag.
 
The Synchronous configuration or Bugsnag.notify() parameter option can be used to send reports on the same process. Using this option would allow you to send the error reports on graceful terminations, however we understand that this may have an impact on the efficiency of your application. 
 
While we don't have anything in place at the moment to handle reporting on application termination, we do have an item on our backlog aimed at solving this issue which we will get to when priorities allow.

@clr182 clr182 added the awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. label Sep 18, 2023
@clr182
Copy link
Contributor

clr182 commented Nov 15, 2023

Hi,
As this issue hasn't seen any activity for some time I will be closing it out. Please feel free to reopen if there are any further questions.

@clr182 clr182 closed this as completed Nov 15, 2023
@veqryn
Copy link
Contributor Author

veqryn commented Nov 16, 2023

@clr182 Please re-open, I don't have the ability to.
I'd say this should be re-opened until the issue is solved.
Using channels and worker pools, rather than potentially infinite number of goroutines, would have a lot of benefits beyond just being able to ensure that Bugsnag reports make it to Bugsnag, such as using a lot fewer resources.

@clr182 clr182 reopened this Nov 21, 2023
@SpencerMalone
Copy link

Yeah this is a reallyyyy painful design, if it's gonna stay this way I think it's worth being very explicit about the implications of not use the synchronous configuration in all the official on site docs (maybe even making it default to synchronous as a breaking change.)

@veqryn
Copy link
Contributor Author

veqryn commented Mar 7, 2024

@SpencerMalone I created a library to fix this issue, and it also acts as a slog middleware (golang's structured logging) to automatically send your logs to bugsnag if they are at Error level or higher (configurable). It allows flushing the errors to bugsnag before exit. You should check it out: https://github.com/veqryn/slog-bugsnag

@SpencerMalone
Copy link

Ha, we did a similar thing, but with Zap as we have some pre-slog apps that haven't been ported over yet. I'll take a look when we get around to wholesale adoption of slog!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. 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

3 participants