Skip to content
This repository was archived by the owner on Dec 7, 2018. It is now read-only.

[DNSResolver] Raise SocketError when there’s no connection. - #171

Open
alloy wants to merge 1 commit into
celluloid:masterfrom
alloy:handle-socket-error
Open

[DNSResolver] Raise SocketError when there’s no connection.#171
alloy wants to merge 1 commit into
celluloid:masterfrom
alloy:handle-socket-error

Conversation

@alloy

@alloy alloy commented Jan 28, 2016

Copy link
Copy Markdown

In short, what I tried to fix is the following behaviour when trying to resolve a hostname when offline:

irb(main):001:0> require 'celluloid/current'
=> true
irb(main):002:0> require 'celluloid/io'
=> true
irb(main):003:0> Celluloid::IO::TCPSocket.new("www.google.com", 80)
NoMethodError: undefined method `"\u0000\u0002\u0001\u0000\u0000\u0001\u0000\u0000\u0000\u0000\u0000\u0000\u0003www\u0006google\u0003com\u0000\u0000\u0001\u0000\u0001"' for nil:NilClass
    from /Users/eloy/.gem/ruby/2.3.0/gems/celluloid-io-0.17.3/lib/celluloid/io/dns_resolver.rb:44:in `resolve'
    from /Users/eloy/.gem/ruby/2.3.0/gems/celluloid-io-0.17.3/lib/celluloid/io/tcp_socket.rb:101:in `create_socket'
    from /Users/eloy/.gem/ruby/2.3.0/gems/celluloid-io-0.17.3/lib/celluloid/io/tcp_socket.rb:53:in `initialize'
    from /Users/eloy/.gem/ruby/2.3.0/gems/celluloid-io-0.17.3/lib/celluloid/io/socket.rb:39:in `new'
    from /Users/eloy/.gem/ruby/2.3.0/gems/celluloid-io-0.17.3/lib/celluloid/io/socket.rb:39:in `new'
    from (irb):3
    from /Users/eloy/.rubies/ruby-2.3.0/bin/irb:11:in `<main>'

What happens is that @socket remains nil here which means that you end up sending the hostname as a NilClass#send message here.


With stdlib it raises the following exception, although that one is actually not raised by the name resolver, but by Socket:

irb(main):001:0> require 'socket'
=> false
irb(main):002:0> TCPSocket.new("www.google.com", 80)
SocketError: getaddrinfo: nodename nor servname provided, or not known
    from (irb):2:in `initialize'
    from (irb):2:in `new'
    from (irb):2
    from /Users/eloy/.rubies/ruby-2.3.0/bin/irb:11:in `<main>'

The one raised by Resolv is:

irb(main):001:0> Resolv.getaddress("www.google.com")
Resolv::ResolvError: no address for www.google.com
    from /Users/eloy/.rubies/ruby-2.3.0/lib/ruby/2.3.0/resolv.rb:95:in `getaddress'
    from /Users/eloy/.rubies/ruby-2.3.0/lib/ruby/2.3.0/resolv.rb:45:in `getaddress'
    from (irb):1
    from /Users/eloy/.rubies/ruby-2.3.0/bin/irb:11:in `<main>'

I’m not sure which one I should use, any thoughts?

@ioquatix

Copy link
Copy Markdown
Contributor

probably the one from socket?

@ioquatix

Copy link
Copy Markdown
Contributor

perhaps a new exception would make sense... SocketNameResolutionError < SocketError?

@alloy

alloy commented Jan 28, 2016

Copy link
Copy Markdown
Author

Sure, I can do that, and keep the same error message, yeah?

@alloy
alloy force-pushed the handle-socket-error branch from 369b515 to af08ced Compare January 28, 2016 20:39
@alloy

alloy commented Jan 28, 2016

Copy link
Copy Markdown
Author

@ioquatix Like so?

@ioquatix

Copy link
Copy Markdown
Contributor

Yeah that seems fine.

@ioquatix

Copy link
Copy Markdown
Contributor

@tarcieri still waiting for travis issues to be fixed, boop :)

@tarcieri

Copy link
Copy Markdown
Member

@ioquatix that was on nio4r. The test failures look like the same on master (we never got celluloid-io back to green)

@ioquatix

Copy link
Copy Markdown
Contributor

ah okay I guess I'll take a look at the issues here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants