phpinnacle / ridge Goto Github PK

if PHPinnacle\Ridge\Client receive PHPinnacle\Ridge\Protocol\ConnectionCloseFrame it will never chnage state to STATE_NOT_CONNECTED. There is no way to detect/subscribe on canceled connection from user code.

Maybe neccesary to change state here?

• $this->state = self::STATE_NOT_CONNECTED;

If some reason clent not able to send heartbeats (broken network etc) RabbitMQ server will close connection automaticly on his own side, but consumer still connected to server. Clinet is waiting of new messages but they will never come, because rabbitmq dosn`t have connection (and consumer) any more .

There is code in PHPinnacle\Ridge\Connection for send heartbeats, but no code for receive it and broke connection if heartbeats didn't came at proper time.

Mybe good idea to pass callback to heartbeat init function and call it if last read was too later.

public function heartbeat(int $interval, ?callable $connectionLost = null): void

amphp 3 is stable and was released more than 1 year ago. Are there any plans to support this version with the next major update?

I try subscribe to channel with:

yield $channel->consume('onMessage', 'queue', 'tag');
yield $channel->consume('onMessage', 'queue', 'tag');

For first request it working, but for the second it remains pending for a long time.

In rabbitmq I see:

operation basic.consume caused a connection exception not_allowed: "attempt to reuse consumer tag 'tag'"

This also applies to the queueDeclare method with different parameters.
I think need to throw an exception when the server raises an error.

When we pass an invalid vhost, the broker sends a ConnectionCloseFrame and terminates the connection, but Ridge does not capture this frame and does not throw an exception.
Example:

use Amp\Loop;
use PHPinnacle\Ridge\Client;

Loop::run(function () {
    $client = Client::create('amqp://guest:guest@localhost:5672/unknown-vhost');

    yield $client->connect();

    echo "Execution will never get here. Nor will an exception be thrown.";

    // (...)
});

This code:

asyncCall(function () {
    yield $this->await(Protocol\ConnectionCloseFrame::class);

    // (...)
});

should be in front of:

Additionally, the status change to STATE_DISCONNECTING and STATE_NOT_CONNECTED is missing in the code below.

Version 1.3.0

Simple code:

\Amp\Loop::run(function() {
	$client = Client::create("amqp://guest:guest@localhost:5672/?timeout=5000");
	yield $client->connect();
	print_r('Connected: ' . $client->isConnected() . PHP_EOL);
	\Amp\Loop::repeat(1000, fn() => print_r('Connected: ' . $client->isConnected() . PHP_EOL));
});

And see in log:

Connected: 1
Connected: 1
Connected: 
Connected: 
Connected: 

But I see active connection in rabbit console:

Bunny is based on ReactPHP, however since Amphp does support React promises, bunny should be able to work with Amphp. What are the reasons for this library then? You mentioned "some strange bugs" but that's way too abstract. What are those bugs? Are there any other advantages and/or differences from bunny?

Detailed description

Fatal error received after missed heartbeats

rabbitmq | missed heartbeats from client, timeout: 60s
backend | Fatal error (Uncaught ErrorException)
Notice: Undefined property: PHPinnacle\Ridge\Connection::$socket 
in /var/www/vendor/phpinnacle/ridge/src/Connection.php:200

Your environment

Docker images based on php:7.2-cli-alpine and rabbitmq:3.7.2-management-alpine

When we provide an incorrect login or password, the broker closes the connection (without sending the ConnectionCloseFrame, it just terminates the socket connection) and Ridge does not inform about it in any way. I think an exception should be thrown informing about this fact.

Example:

use Amp\Loop;
use PHPinnacle\Ridge\Client;

Loop::run(function () {
    $client = Client::create('amqp://user:wrongpass@localhost:5672');

    yield $client->connect();

    echo "Execution will never get here. Nor will an exception be thrown.";

    // (...)
});

RabbitMQ logs:

2020-09-10 08:39:02.330 [info] <0.13118.7> accepting AMQP connection <0.13118.7> (192.168.5.23:33952 -> 172.17.0.3:5672)
2020-09-10 08:39:02.332 [error] <0.13118.7> Error on AMQP connection <0.13118.7> (192.168.5.23:33952 -> 172.17.0.3:5672, state: starting):
AMQPLAIN login refused: user 'user' - invalid credentials
2020-09-10 08:39:05.332 [info] <0.13118.7> closing AMQP connection <0.13118.7> (192.168.5.23:33952 -> 172.17.0.3:5672)

Any plans to support PHP 8?

Detailed description

When connecting to RMQ fails ConnectionCloseFrame is handled but no exception in case of error is thrown.

DSN

amqp://test:test@localhost:5672/test

ConnectionCloseFrame

    object(PHPinnacle\Ridge\Protocol\ConnectionCloseFrame)#866 (10) {
      ["replyCode"]=>
      int(530)
      ["replyText"]=>
      string(81) "NOT_ALLOWED - access to vhost '/test' refused for user 'test'"
      ["closeClassId"]=>
      int(10)
      ["closeMethodId"]=>
      int(40)
      ["classId"]=>
      int(10)
      ["methodId"]=>
      int(50)
      ["type"]=>
      int(1)
      ["channel"]=>
      int(0)
      ["size"]=>
      int(92)
      ["payload"]=>
      NULL
    }

Context

The change is important for all users using non-default vhost.

Possible implementation

Possible fix should add \trim($url['path'], '/') in Config::parse

Environment

• Version used 0.2.0
• PHP Version used 7.2.4
• Operating system: any

Version 1.3.0 with fixed PR #33

Example code:

\Amp\Loop::run(function() {
	$client = Client::create("amqp://guest:guest@localhost:5672/?timeout=5000&heartbeat=2000");
	yield $client->connect();
	print_r('Connected: ' . $client->isConnected() . PHP_EOL);
	\Amp\Loop::repeat(1000, fn() => print_r('Connected: ' . $client->isConnected() . PHP_EOL));
});

I add debug lines before write, before check condition and add log in close method

print_r('write' . PHP_EOL);
yield $this->write((new Buffer)
...
print_r('check' . PHP_EOL);
if (

Run script and see in log:

Connected: 1
Connected: 1
write
check
Connected: 1
Connected: 1
write
check
Connected: 1
Connected: 1

Checking heartbeat correctly. But I simulate RabbitMQ server down with command brew services restart rabbitmq I see in log:

Connected: 1
Connected: 1
write
check
Connected: 1
Connected: 1
write
check
Connected: 1
Connected: 1
close
Connected: 1
Connected: 1
Connected: 1
Connected: 1
Connected: 1

In the configuration, the default timeout is written as milliseconds: