[ruby-core:108157] [Ruby master Feature#18675] Add new exception class for resolv timeouts

Issue #18675 has been updated by jeremyevans0 (Jeremy Evans).

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

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.

----------------------------------------
Feature #18675: Add new exception class for resolv timeouts
https://bugs.ruby-lang.org/issues/18675#change-97134

* Author: geffatpier64 (Geff Hanoain)
* Status: Open
* Priority: Normal
----------------------------------------
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,



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>