isConnected reports true while it is not

[xorock]

If it is not a bug then maybe You can show me right path how to handle this. I have a long-lived consumer that parses incoming messages (text documents) and then publisher sends a reply to another queue. Now, if heartbeat is set to 10s, producer connection is killed after that time but isConnected() method always reports true.

public function reconnect(): void
    {
        var_dump($this->client->isConnected()); // always true even if connection was closed
        $this->client->disconnect();
        var_dump($this->client->isConnected()); // this is also true :)
        if (!$this->client->isConnected()) { // never executed
            $this->client->connect();
        }
    }

After that time I get Broken pipe or closed connection exception.

As a workaround, I set heartbeat option to 5 minutes, then changed $channel->run to $bunny->run(5 * 60) and the supervisord daemon starts the process again.

[jakubkulhan]

Hello, isConnected checks only internal state of the client connection, it doesn't try to do reach to the broker over network to assume that the connection is actually still ok. This is intended behavior.

As PHP is single-threaded, processing of messages cannot take you more than heartbeat timeout, otherwise client doesn't get a chance to send something to the broker and the broker forcefully closes the connection. If you connect to the broker over local network, it's ok to use large heartbeat timeouts. One production consumer I created needed to work for more than 10 minutes, so the heartbeat was set to 15 minutes without a problem.

When you call sync client's Channel::run() or Client::run(), it starts a loop that periodically sends heartbeats (as per heartbeat client option) to the broker if there is no chatter over the connection. You shouldn't need to pass $maxSeconds argument to Client::run() as high as heartbeat timeout. If the consumer (i.e. the callback passed to consume method) completes its work within heartbeat timeout and you still get heartbeat timeouts, I suggest you check that the TCP connection can stay open at least for heartbeat timeout (e.g. if there was a loadbalancer in-between the consumer and the broker, loadbalancer must have idle timeout set long enough).

[brandonsimpson]

I have the same issue. Regardless of the heartbeat timeout, a connection could have been closed from the rabbitMQ server side and we need to know when to re-connect. The only time I can detect the connection was closed is after the write to the channel fails because isConnected always returns true, and the client state is always 2. I have also seen the channel publish() response return true, even when the connection is gone. On the next publish with the same connection, then we see the client was disconnected.

I see Client::run() also uses isConnected and doesn't attempt re-connecting on it's own. That may keep a connection open with heartbeats, but does not help recover disconnected clients.

I'm at a loss for how to make this library check for a reliable connection.

[brandonsimpson]

@xorock take a look at the tests here: https://github.com/jakubkulhan/bunny/blob/master/test/Bunny/ClientTest.php

I dug in deep, as this wasn't documented. I used the channel->confirmSelect with the MethodBasicAckFrame callback, and I was able to catch connection issues and restart the connection when the publish and ack failed. The isConnected value was also updated correctly when that connection failed.

Hope that helps.

[PatUlm]

Hi, I have the same issue and I found out, that the Broken pipe exception is thrown in the destructor. I don't know why, but the clean shutdown is only implemented in Client::__destruct(). The destructor explicit call Client::run(), which makes sense. With that in mind a clean shutdown could be

$bunny->disconnect()->done(function () use ($bunny) {
  $bunny->stop();
});
if ($bunny->isConnected()) {
  $bunny->run();
}
var_dump($bunny->isConnected());

Now the client is really disconnect but I don't think it was meant to be so complicated. Shouldn't there be a clean shutdown? I think the destructor should only call disconnect() (like in async client) and disconnect() should do the hole job currently implemented in the destructor. What do you think?

The broken pipe exception is really bad in that case, because it's triggered by the garbage collector and could not be catched. And the fix should be easy to implement.