ssi-anik / amqp

php-amqplib wrapper that eases the consumption of RabbitMQ. A painless way of using RabbitMQ
https://packagist.org/packages/anik/amqp
MIT License
134 stars 23 forks source link

Support for AMQPStreamConnection #13

Closed ryanhalliday closed 3 years ago

ryanhalliday commented 3 years ago

Not all AMQP / RabbitMQ servers have SSL setup (eg default Ubuntu apt install rabbitmq-server), so this package will fail with the vague error message of stream_socket_client(): unable to connect to ssl://localhost:5672.

Changing out AMQPSSLConnection for AMQPStreamConnection resolves this error - but there is no way to do this via configuration.

The RabbitMQ server logs don't help much either when debugging this, with only logs like 2020-XX-XX XX:XX:XX.XXX [error] <0.596.0> closing AMQP connection <0.596.0> (127.0.0.1:44504 -> 127.0.0.1:5672): {bad_header,<<22,3,1,2,0,1,0,1>>}

I suggest we include an option in the connection settings like:

'ssl' => env('AMQP_SSL', true),

Then we need to add a condition to AmqpManager->connect() to create either AMQPSSLConnection or AMQPStreamConnection.

If that sounds good to you @ssi-anik then I can make these changes and send a PR?

ssi-anik commented 3 years ago

Hello, Seems like passing ssl_protocol null will do the job. AMQPSSLConnection extends the AMQPStreamConnection. So, can you try null and if it works then send a PR?

ssi-anik commented 3 years ago

Hello, Seems like passing ssl_protocol null will do the job. AMQPSSLConnection extends the AMQPStreamConnection. So, can you try null and if it works then send a PR?

Setting null will not work because of the null coalescing operator. Can you please

Waiting for the PR. 👍

ryanhalliday commented 3 years ago

It seems the issue is actually with php-amqlib and they should be releasing a fix soon. The cause is these lines: https://github.com/php-amqplib/php-amqplib/blob/v2.12.0/PhpAmqpLib/Connection/AMQPSSLConnection.php#L26-L28

v2.12.0 introduced a bug which has been reverted in v2.12.1 which has yet to be released: https://github.com/php-amqplib/php-amqplib/pull/829

As a temporary fix, I have included specifically "php-amqplib/php-amqplib": "2.11.3", in my projects composer.json as that satisfies this packages and resolves the issue.

We could either set the exact the version in this packages composer.json and update it when v2.12.1 comes out or leave it and wait for it to be fixed.

Is there anything you would like me to do?

ssi-anik commented 3 years ago

Nice find out. When I developed, I used without SSL and it worked fine. That's why I was worried about, why it's not working now.

Anyway, if you can kindly add the above-mentioned points and send a PR would be great. And it should not break the code I guess too.

ryanhalliday commented 3 years ago

I don't think this change is needed anymore for non-SSL to work, but I have submitted it anyway as it gives people better control over the variable and means we rely less on the upstream php-amqplib's handling.

Also excluded their broken version in composer.json

ssi-anik commented 3 years ago

Merged and tagged a version. Thanks.