signalfx / signalfx-go

Go client library and instrumentation bindings for SignalFx
https://www.signalfx.com
Apache License 2.0
14 stars 51 forks source link

Safely Closing Client #98

Closed shrivu-stripe closed 4 years ago

shrivu-stripe commented 4 years ago

I'm trying to connect, execute, read detector meta data, and close the connection.

The result of the code below is an occasional panic, I'm wondering if there's a panic-free way to do this and/or if this a bug?

streamClient, err := signalflow.NewClient(signalflow.StreamURL(streamURL), signalflow.AccessToken(srv.AccessToken))
if err != nil {
    panic(err)
}
execResult, err := streamClient.Execute(&signalflow.ExecuteRequest{
    Program: sourceCode,
})
if err != nil {
    panic(err)
}
fmt.Println(execResult.LimitSize())
streamClient.Close()
// panic: send on closed channel

I've tried including execResult.Stop() (placed before .Close()) but it still panics.

cory-signalfx commented 4 years ago

cc @keitwb

keitwb commented 4 years ago

@shrivu-stripe try the latest master of this library and see if that doesn't fix it.

shrivu-stripe commented 4 years ago

On master bbd9b6f42b21 --

Still getting ~20% of runs:

coroutine 72 [running]:
github.com/signalfx/signalfx-go/signalflow.(*Computation).bufferDataMessages(0xc000113790)
        /Users/shrivu/stripe/siren/vendor/github.com/signalfx/signalfx-go/signalflow/computation.go:302 +0x251
created by github.com/signalfx/signalfx-go/signalflow.newComputation
        /Users/shrivu/stripe/siren/vendor/github.com/signalfx/signalfx-go/signalflow/computation.go:81 +0x1a8
exit status 2

(should have included the error msg in the original issue, my b)

keitwb commented 4 years ago

Ok, that is another channel that is closed that I didn't cover in the last PR, let me deal with that one too.