fix: Recover Hutch worker when consumer channel is closed due to delivery_ack_timeout?

[eglitobias]

Summary

This MR fixes the case where RabbitMQ closes the consumer channel and Hutch gets stuck because subsequent ack/nack calls raise ChannelAlreadyClosed.

When that happens today, Hutch can stay alive but stop consuming new messages until the process is restarted. That is the behavior described in issue #364.

What changed

• expose ChannelAlreadyClosed from both adapters:
  • Hutch::Adapter::Bunny::ChannelAlreadyClosed
  • Hutch::Adapter::MarchHare::ChannelAlreadyClosed
• update Hutch::Worker to:
  • rescue ChannelAlreadyClosed when acknowledge_error tries to ack/nack
  • trigger channel recovery instead of letting the worker remain stuck
• add guarded recovery logic using a mutex so only one recovery runs at a time
• recover by:
  • reopening the AMQP channel on the existing connection
  • re-running queue setup / consumer subscriptions
• log recovery success or failure
• add specs for:
  • closed-channel ack not raising
  • closed-channel nack after consumer error not raising
  • end-to-end recovery after the consumer channel is closed

[michaelklishin]

Re-opening channels that were closed by the server is OK in some cases (e.g. due to a delivery acknowledgement timeout)
but not others.

This reinvents a portion of connection recovery in Bunny, ignores Bunny::Channel#open?, existing eventual assertion helpers.

[michaelklishin]

One thing that this PR does get right: it recovers in one specific place instead of using Bunny::Channel#on_error. This is acceptable and such recover should succeed in most cases in practice (assuming that only the channel was closed).

I'm working on adding a couple of Bunny::Channel and Bunny::Session methods to Bunny 3.0 for such cases.

[michaelklishin]

With [1][2], the implementation of this PR can shrink significantly, coming down to something like this:

ch.on_error do |channel, channel_close_method|
  if channel_close_method.delivery_ack_timeout?
    channel.reopen
    connection.recover_channel_topology(channel)
  end
end

1. ruby-amqp/amq-protocol@dea05d4
2. ruby-amqp/bunny@4b2fa43

[eglitobias]

I only used this for local testing. As soon as bunny 3 is released we can change this in the hutch.gemspec

[eglitobias]

Thank you very much for the quick review.

I’ve updated the PR accordingly. Most of the work went into building an integration test that reliably reproduces the issue.

One thing I’m still unsure about is whether reopening the channel from a separate thread is the best approach here. I wasn’t able to find a cleaner way to make channel.reopen succeed in this setup, so I’d be glad to hear if there is a better alternative.

I also opened a small follow-up PR in amq-protocol because delivery_ack_timeout? does not return a boolean value:
ruby-amqp/amq-protocol#85

[michaelklishin]

If the channel is already closed by the server, there won't be any activity on it, so all scenarios where reopening the channel would entail a concurrency hazard come down to the channel ID allocator safety.

All key methods of the allocator are synchronized.

[michaelklishin]

@eglitobias voilà, Bunny 3.0.0 is here. Happens to be on Apr 1st in Europe but I am dead serious :)

[michaelklishin]

Hutch main now depends on Bunny 3.0 #415

[eglitobias]

Rebased on top of Bunny 3.0 and updated the branch.
@michaelklishin Thanks again for the super fast follow-up and for the very convincing "totally not an April 1st joke" release.
I was briefly worried Bunny 3.0 would ship with an April-1st-only channel allocator mode.

[eglitobias]

Not sure why the pipeline is failing now with Ruby 3.3.
But the error in the log is related to the deprecated ddtrace Gem.
I opened this MR #416: Switch to the new gem (datadog) and also add Ruby 3.4 and Ruby 4 pipelines.

[michaelklishin]

@eglitobias having considered the "offloading" thread workaround you have ended up with, I think another change to Bunny can be justified: the Bunny::Channel#on_error callback can be invoked in a new thread by Bunny itself.

Otherwise the entire feature set would require every single user to start a new thread.

Bunny's I/O loop thread is different from the application thread (let's ignore the single threaded mode, virtually no one even knows about it even though it was contributed around 2012). Which means using a yet another separate thread won't make much difference for the end user.

[michaelklishin]

@eglitobias I have released Bunny 3.1.0 and refactored this PR to not use a separate thread.

We can still introduce more tweaks in follow-up PRs.