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

nil recover_attempts (default) causes incorrect logging #569

Closed
olivierlacan opened this issue Oct 30, 2018 · 1 comment
Closed

nil recover_attempts (default) causes incorrect logging #569

olivierlacan opened this issue Oct 30, 2018 · 1 comment
Milestone

Comments

@olivierlacan
Copy link

olivierlacan commented Oct 30, 2018

When recover_attempts isn't set, it defaults to nil, which leads to an empty string being printed in the logs when recovery attempts left is logged inside of decrement_recovery_attempt_counter!.

13:40:35 -- Retrying connection on next host in line: rabbit3.domain.com:5672
13:40:35 -- Could not establish TCP connection to rabbit3.domain.com:5672: getaddrinfo: nodename nor servname provided, or not known
13:40:35 -- Will try to connect to the next endpoint in line: rabbit2.domain.com:5672
13:40:35 -- Could not establish TCP connection to rabbit2.domain.com:5672: getaddrinfo: nodename nor servname provided, or not known
13:40:35 -- Will try to connect to the next endpoint in line: rabbit1.domain.com:5672
13:40:35 -- Could not establish TCP connection to rabbit1.domain.com:5672: getaddrinfo: nodename nor servname provided, or not known
13:40:35 -- TCP connection failed, reconnecting in 5.0 seconds
13:40:35 --  recovery attempts left
13:40:35 -- Will recover from a network failure (no retry limit)...
13:40:40 -- Will attempt connection recovery...

It seems like the call to logger.debug should be conditional on the non-nil value of @recovery_attempts here:

bunny/lib/bunny/session.rb

Lines 774 to 778 in f29c8b7

def decrement_recovery_attemp_counter!
@recovery_attempts -= 1 if @recovery_attempts
@logger.debug "#{@recovery_attempts} recovery attempts left"
@recovery_attempts
end

Or at the very least it should depend on @max_recovery_attempts which is used to set @recovery_attempts to begin with:

bunny/lib/bunny/session.rb

Lines 173 to 174 in f29c8b7

@max_recovery_attempts = opts[:recovery_attempts]
@recovery_attempts = @max_recovery_attempts

@michaelklishin
Copy link
Member

Please submit a PR.

@michaelklishin michaelklishin added this to the 2.13.0 milestone Nov 7, 2018
michaelklishin added a commit that referenced this issue Dec 3, 2018
(cherry picked from commit 354ae54)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants