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

Introduce recovery_failed callback #666

Conversation

Schmitze333
Copy link
Contributor

It’s possible to specify the recovery_attempts option on the Bunny::Session.
Hitting the recovery attempts limit without a successful recovery leads to a cleanup of connection, etc.
However, the client is not informed about this state and has no chance to react upon it.

This PR introduces a callback which is executed in exactly the above described situation enabling client code to react to a failed recovery process.

Relates to: #665

It’s possible to specify the recovery_attempts option.
Hitting the recovery attempts limit without a successful recovery
leads to a cleanup of connection, etc.
However, the client is not informed about this state and has no chance
to react upon it.

Here we introduce a callback which is called in exactly the above
described situation enabling client code to react to a failed recovery
process.
Copy link
Member

@michaelklishin michaelklishin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking the time co contribute.

"recovery_failed" to me suggests that this callback will be used for every unsuccessful recovery attempts. If that's not the case, we should rename it "recovery_attemtps_exhausted" or something that clearly suggests that this callable will be invoked only once
for a series for unsuccessful recovery attempts.

@Schmitze333
Copy link
Contributor Author

Yeah, I agree. That name is much better!
I'll make the necessary changes.

The former name implied that the hook would be executed on every
recovery attempt failing. Renaming it to ‘recovery_attempts_exhausted’
will make things much clearer.
@michaelklishin michaelklishin merged commit d8ff73d into ruby-amqp:main Jun 9, 2023
@michaelklishin
Copy link
Member

@Schmitze333 thank you for contributing to Bunny!

@Schmitze333
Copy link
Contributor Author

I did not provide a Changelog entry. Will that happen on the release or shall I add one?

@michaelklishin
Copy link
Member

michaelklishin commented Jun 12, 2023

I update the change log before every release if necessary. Don't worry about it.

@michaelklishin
Copy link
Member

2.22.0 is up on rubygems.org.

@michaelklishin michaelklishin modified the milestones: 2.21.0, 2.22.0 Jul 1, 2024
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

Successfully merging this pull request may close these issues.

2 participants