amqp-node / amqplib

AMQP 0-9-1 library and client for Node.JS
https://amqp-node.github.io/amqplib/
Other
3.69k stars 474 forks source link

Connection: forcibly close stream if close() is called while the connection is blocked #744

Open SimonWoolf opened 1 year ago

SimonWoolf commented 1 year ago

Because of how rabbit's connection.blocked mode works, if the connection is blocked, the client will not receive a close-ok unless/until the connection becomes unblocked, which may be some time away, or never. Current behaviour just leaves the connection open indefinitely. This instead closes it immediately.

cressie176 commented 1 year ago

Hi @SimonWoolf, Thanks for the PR. I see other clients have suffer from the same challenge.

@michaelklishin, it was a while back but do you remember whether and how this issue was address with the Java client? https://groups.google.com/g/rabbitmq-users/c/7cQXglAnWS8

Is there a defacto standard approach that amqplib should follow?

cressie176 commented 11 months ago

I've been thinking about this some more and wondering whether supplying an optional boolean "force" to the connection close method might be better. @SimonWoolf if you have time to update the PR then I'll merge. Otherwise I'll take a look, but probably not for a little while.

SimonWoolf commented 11 months ago

I've been thinking about this some more and wondering whether supplying an optional boolean "force" to the connection close method might be better.

to be clear, are you asking for that force arg to apply to the 'destroy() after end()' behaviour change, or to the 'close immediately if blocked' change? (or both?)

cressie176 commented 11 months ago

I was thinking of adding the option to connection.close, then passing this through. The function also takes an optional callback which would make this a bit clumsy. Also, something I hadn't considered before, is that this wouldn't help for any non-user invoked closes.

cressie176 commented 9 months ago

Some interesting reading here... https://rabbitmq-users.narkive.com/DSpWxIxq/rabbitmq-discuss-blocking-queue-close-connection-by-client

cressie176 commented 9 months ago

php-amqplib took a timeout approach - https://github.com/php-amqplib/php-amqplib/pull/609

kibertoad commented 5 months ago

@SimonWoolf Ping? Are you still open to update the PR?