search

segmentio / nsq-go

Go package providing tools for building NSQ clients, servers and middleware.
MIT License
100 stars 20 forks source link

Panic on Finish seems problematic? #20

Open thehydroimpulse opened 7 years ago

thehydroimpulse commented 7 years ago

Could we simply return an error type instead of panicing? This seems like a recoverable error to me.

cc @achille-roussel 

panic: (*Message).Finish or (*Message).Requeue has already been called
goroutine 8 [running]:
github.com/segmentio/nsq-go.(*Message).Finish(0xc4205ffe68)
    /go/src/github.com/segmentio/nsq-go/message.go:72 +0x155
github.com/segmentio/integration-discards-archiver.(*nsqInput).handleMessage(0xc420145b90, 0xc4205ffe68)
    /go/src/github.com/segmentio/integration-discards-archiver/nsq.go:136 +0x327
github.com/segmentio/integration-discards-archiver.(*nsqInput).Consume(0xc420145b90, 0x9ba560, 0xc420018f80, 0x0, 0x0)
    /go/src/github.com/segmentio/integration-discards-archiver/nsq.go:71 +0x1d4
main.main.func1(0xc420145b90, 0x9ba560, 0xc420018f80, 0xc42001d950)
    /go/src/github.com/segmentio/integration-discards-archiver/cmd/archiver.go:102 +0x43
created by main.main
    /go/src/github.com/segmentio/integration-discards-archiver/cmd/archiver.go:107 +0x48a

I can send a PR if needed

achille-roussel commented 7 years ago

I disagree, this is a programming error, the program is misusing the package so a panic seems appropriate. It's kind of like closing a channel twice, it panics because it doesn't make sense.

Also, it's documented.