Please don't panic

[silasdavis]

Note: rmq panics on Redis connection errors. Your producers and consumers will crash if Redis goes down. Please let us know if you would see this handled differently.

My producers and consumers are part of my main API - I don't want it to be taken down because there is a redis error.

I can understand a fail fast design for separate worker services but that is a decision that is better made by the application rather than one of its libraries.

The most idiomatic go would probably be to put errors everywhere, but that would be quite a big change to your API. One option would be to coalesce errors together and put the onus on the library user to check. This would work for me.

To make this a non-breaking change we could do the following:

• Make an option panicOnErrors with the default true, I like the following pattern for options (also non-breaking):

  
  // 
  type option func(*redisConnection)

function PanicOnError(value bool) option { ... }

func OpenConnection(tag, network, address string, db int, options... option) *redisConnection

- Exploit the fact that we are meant to be running `cleaner.Clean()` and it returns an error and return any aggregate (or first) error there so that the library user can take whatever recover action is appropriate

Alternatively maybe it is cleaner (no pun intended) to have a separate `Error() error` method.

Another possibility would be to have a `PanicHandler(func (error) error)` so that where you would panic you instead run the provided handler function. The semantics could be that if that itself errs then you _do_ panic. Hmm I think maybe I like that one the best - it means you can act immediately but keeps the API intact.

[wellle]

@silasdavis: Thanks, see #8 for previous discussion. I'll have another look at it though.

[gearintellix]

Please make logs only, or can be handle error don't do panic, this break my whole apps!

2020/05/03 21:06:00 rmq redis error is not nil write tcp 127.0.0.1:61947->127.0.0.1:6379: i/o timeout
panic: rmq redis error is not nil write tcp 127.0.0.1:61947->127.0.0.1:6379: i/o timeout

goroutine 61 [running]:
log.Panicf(0x51330ca, 0x1d, 0xc00050fe98, 0x1, 0x1)
    /usr/local/go/src/log/log.go:340 +0xd8
github.com/adjust/rmq.checkErr(0x54244c0, 0xc0008c1c20, 0xc0008c1c00)
    /Users/gearintellix/Sites/go/src/github.com/adjust/rmq/redis_wrapper.go:96 +0x29c
github.com/adjust/rmq.RedisWrapper.Set(0xc0000be4c0, 0xc00027c400, 0x35, 0x5112ab8, 0x1, 0xdf8475800, 0x0)
    /Users/gearintellix/Sites/go/src/github.com/adjust/rmq/redis_wrapper.go:15 +0xe6
github.com/adjust/rmq.(*redisConnection).updateHeartbeat(0xc0003e8230, 0x543b200)
    /Users/gearintellix/Sites/go/src/github.com/adjust/rmq/connection.go:155 +0x88
github.com/adjust/rmq.(*redisConnection).heartbeat(0xc0003e8230)
    /Users/gearintellix/Sites/go/src/github.com/adjust/rmq/connection.go:141 +0x2f
created by github.com/adjust/rmq.openConnectionWithRedisClient
    /Users/gearintellix/Sites/go/src/github.com/adjust/rmq/connection.go:60 +0x412

[wellle]

Hey, I actually started working on this again just a few days ago. :v:

[gearintellix]

@wellle that sound great, thank you

[wellle]

Done in #88.