Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing client notification when recovery fails (after specified recovery attempts) #665

Closed
Schmitze333 opened this issue Jun 9, 2023 · 1 comment

Comments

@Schmitze333
Copy link
Contributor

Schmitze333 commented Jun 9, 2023

Bunny: latest
RabbitMQ: 3.11.16
Ruby: 3.2.2

Overview

The Bunny gem conveniently allows to specify a recovery_attempts option limiting the number of connection recovery retries.
However, when running into the situation that all recovery attempts are used up and a recovery was not successful, the application has no chance to be informed about that state.

Example to reproduce

require "bunny"

error = false

connection = Bunny.new(recovery_attempts: 2)
connection.start

channel = connection.create_channel
channel.on_uncaught_exception { error = true }
queue = channel.queue("hello")

consumer_thread =
  Thread.new do
    consumer = Bunny::Consumer.new(channel, queue)
    consumer.on_delivery { |_di, _m, payload| puts payload }
    queue.subscribe_with(consumer)
  end

consumer_thread.abort_on_exception = true

begin
  sleep 1 while !error
rescue Interrupt => _
  connection.close

  exit(0)
rescue => e
  pp e

  exit(1)
end

Logs when interrupting the connection to RMQ:

W, [2023-06-09T14:28:37.811469 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Recovering from connection.close (CONNECTION_FORCED - broker forced connection closure with reason 'shutdown')
W, [2023-06-09T14:28:37.811907 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Will recover from a network failure (2 out of 2 left)...
W, [2023-06-09T14:28:47.813447 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Retrying connection on next host in line: 127.0.0.1:5672
W, [2023-06-09T14:28:47.814373 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Could not establish TCP connection to 127.0.0.1:5672: Connection refused - connect(2) for 127.0.0.1:5672
W, [2023-06-09T14:28:47.814446 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: TCP connection failed, reconnecting in 5.0 seconds
W, [2023-06-09T14:28:47.814474 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Will recover from a network failure (1 out of 2 left)...
W, [2023-06-09T14:28:52.818248 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Retrying connection on next host in line: 127.0.0.1:5672
W, [2023-06-09T14:28:52.819027 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Could not establish TCP connection to 127.0.0.1:5672: Connection refused - connect(2) for 127.0.0.1:5672
W, [2023-06-09T14:28:52.819096 #6429]  WARN -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: TCP connection failed, reconnecting in 5.0 seconds
E, [2023-06-09T14:28:52.819771 #6429] ERROR -- #<Bunny::Session:0x11d0 guest@127.0.0.1:5672, vhost=/, addresses=[127.0.0.1:5672]>: Ran out of recovery attempts (limit set to 2), giving up

Expectation

In line with other libraries that offer a retry mechanism (e.g. Faraday) I would expect the client code to be informed about that state so that it can be handled. This could be either done by:

  • (Re-)Raising an exception (like Faraday) - It's even logged out as ERROR at this point here.
  • Providing a hook. Seeing a hook for the start of every recovery attempt and one for a successful recovery makes me wonder why there is none for a failed recovery.

I appreciate the Bunny gem providing such a convenience feature (in contrast to other client implementations), but the lag of a appropriate feedback mechanism here makes it feel slightly inconsistent.

@michaelklishin
Copy link
Member

Addressed in #666.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants