Feature #18675: Add new exception class for resolv timeouts

• Status: Open
• Assignee: -
• Target version: -

[geffatpier64 (Geff Hanoain)]

I have 2.7.4 but I also checked 3.0, same code in resolv.rb.

to reproduce:

irb(main):001:0> require 'resolv'
=> true
irb(main):002:0> Resolv::DNS.new(nameserver: ['127.0.0.5']).getaddress("google.com")
Traceback (most recent call last):
        5: from /usr/bin/irb:23:in `<main>'
        4: from /usr/bin/irb:23:in `load'
        3: from /usr/share/gems/gems/irb-1.2.6/exe/irb:11:in `<top (required)>'
        2: from (irb):2
        1: from /usr/share/ruby/resolv.rb:379:in `getaddress'
Resolv::ResolvError (DNS result has no information for google.com)
irb(main):003:0>

Ideal Expectation:

#<Resolv::ResolvTimeout: Resolv::ResolvTimeout>

suggested fix:

class Resolv
  class DNS
    class Config
      def resolv(name)
        candidates = generate_candidates(name)
        timeouts = @timeouts || generate_timeouts
        begin
          candidates.each {|candidate|
            begin
              timeouts.each {|tout|
                @nameserver_port.each {|nameserver, port|
                  begin
                    yield candidate, tout, nameserver, port
                  rescue ResolvTimeout
                  end
                }
              }
              #raise ResolvError.new("DNS resolv timeout: #{name}")
              raise ResolvTimeout
            rescue NXDomain
            end
          }
          #rescue ResolvError
        end
      end
    end
  end
end

I have commented out the code that seemed to arbitrarily swallow any raised exception.

https://github.com/ruby/ruby/blob/ruby_2_7/lib/resolv.rb - line 1125

        rescue ResolvError

I would prefer raising ResolvTimeout to ResolvError.new("DNS resolv timeout: #{name}")

But as long is it does something that isn't:

DNS result has no information for google.com

I'd be okay with it. I'm happy to help with this via a pull request in github, a diff patch or what ever is needed. I just didn't want to spend too much time "fixing" it the "wrong" way. I'm not intimately familiar with the code in resolv.rb so I'm not 100% sure my "fix" wouldn't break other stuff. I did some minor testing of my minor change.

Separately - This also highlights that a timeout condition isn't in or doesn't work in unit tests.

Thanks so much,

[jeremyevans0 (Jeremy Evans)]

• Tracker changed from Bug to Feature
• Subject changed from Resolv :: Timeout doesn't seem to work to Add new exception class for resolv timeouts
• ruby -v deleted (ruby 2.7.4p191 (2021-07-07 revision a21a3b7d23) [x86_64-linux])
• Backport deleted (2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN)

From reviewing the code, Resolv::ResolvTimeout is an exception class used for internal control flow, and not expected to be rescued externally. It's raised in 4 places in request, and rescued in one place by resolv (resolv yields to a block that calls request). Had resolv been developed after Ruby 1.9, it probably would be a private constant.

So the current behavior is expected, and this is not a bug. You cannot switch the current ResolvError to ResolvTimeout externally without breaking backwards compatibility, as ResolvTimeout subclasses Timeout::Error, not ResolvError. So we would need to add a new subclass of ResolvError specific to timeouts. That seems a reasonable feature, so I'm changing the subject and switching this to a feature request.