ruby-amqp/bunny

nil recover_attempts (default) causes incorrect logging

Closed this issue · 1 comments

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

Please submit a PR.