postwait / node-amqp

[UNMAINTAINED] node-amqp is an AMQP client for nodejs
MIT License
1.69k stars 357 forks source link

missed heartbeats do not seem to terminate connection #416

Open jclulow opened 8 years ago

jclulow commented 8 years ago

If I set the heartbeat option (e.g. to 5), the client appears to correctly negotiate the heartbeat interval with RabbitMQ. The "heartbeat" event is emitted at the correct interval when the server is ending heartbeats. If I disable the server while watching this output, the heartbeats (obviously) stop arriving but the client does not signal an error until I try to send a message -- even if this is many intervals later.

I'm not yet completely familiar with the relatively esoteric socket state tracking being used in this library, but I don't see how this logic can be correct:

Connection.prototype._inboundHeartbeatTimerReset = function () {
...
    this._inboundHeartbeatTimer = setTimeout(function () {
      if(self.socket.readable)
        self.emit('error', new Error('no heartbeat or data in last ' + gracePeriod + ' seconds'));
    }, gracePeriod * 1000);
...

Specifically: if no data has arrived (heartbeats or otherwise), why would the socket be readable?

chriswiggins commented 8 years ago

I'm in the same position. Unfortunately I don't think this library is being actively maintained so we might need to find a solution ourselves

siboulet commented 8 years ago

I'm also running in a similar issue. Depending of situation heartbeat does sometime seems to kick in ie. I did get "no heartbeat or data in last xx seconds" exception once, but most of the time I don't.

Listening for the "close" event I can see connection being closed between the 2 heartbeat, so I suspect the problem is with the if(self.socket.readable) check which no longer check heartbeat after socket is closed.

@jclulow @chriswiggins did you find a solution to this problem?

jclulow commented 8 years ago

@siboulet I'm working around it in my library -- https://github.com/joyent/node-urclient/blob/master/lib/mq.js#L362-L375

siboulet commented 8 years ago

Anyone tried the new heartbeatForceReconnect option introduced in the latest 0.2.6 release?

https://github.com/postwait/node-amqp/commit/4a382125d18d74fdffa450afe5f46c0cff1aef57

DJMit-alt commented 5 years ago

4a38212 this fix is also failing and the issue is the same