bugsnag / bugsnag-go

Automatic panic monitoring for Go and Go web frameworks, like negroni, gin, and revel
https://docs.bugsnag.com/platforms/go
MIT License
200 stars 69 forks source link

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

Open veqryn opened 11 months ago

veqryn commented 11 months ago

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

clr182 commented 11 months ago

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 commented 9 months ago

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.

veqryn commented 9 months ago

@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.

SpencerMalone commented 5 months ago

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 commented 5 months ago

@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 commented 3 months ago

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!