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

Use Timeout.timeout for TCP connection #195

Merged
merged 1 commit into from
Mar 31, 2015

Conversation

zanker
Copy link
Contributor

@zanker zanker commented Mar 30, 2015

Unfortunately, getting a pure solution to timeouts is essentially impossible without rewriting the entire stack yourself. Which I might actually do for fun later, but for the time being. This is the simplest way of getting a connect timeout :(

@zanker
Copy link
Contributor Author

zanker commented Mar 31, 2015

@httprb/owners fyi if someone can 👍 quickly

else fail ArgumentError, "unsupported address class: #{addr.class}"
def connect_ssl
socket.connect_nonblock
rescue IO::WaitReadable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I missed this on the last pass, but as the SSL/TLS handshake makes many roundtrips, I believe it's possible for IO::WaitWritable to be raised here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops I broke Excon then!!

Sent from my iPhone

On Mar 31, 2015, at 08:40, Tony Arcieri notifications@github.com wrote:

In lib/http/timeout/per_operation.rb:

  •    case addr
    
  •    when Resolv::IPv4
    
  •      family = Socket::AF_INET
    
  •    when Resolv::IPv6
    
  •      family = Socket::AF_INET6
    
  •    else fail ArgumentError, "unsupported address class: #{addr.class}"
    
  •  def connect_ssl
    
  •    socket.connect_nonblock
    
  •  rescue IO::WaitReadable
    
    I guess I missed this on the last pass, but as the SSL/TLS handshake makes many roundtrips, I believe it's possible for IO::WaitWritable to be raised here too.


Reply to this email directly or view it on GitHub.

@tarcieri
Copy link
Member

This looks fine to me

@ixti
Copy link
Member

ixti commented Mar 31, 2015

👍 from me as well :D

sferik added a commit that referenced this pull request Mar 31, 2015
Use Timeout.timeout for TCP connection
@sferik sferik merged commit 7aff327 into httprb:master Mar 31, 2015
zanker referenced this pull request Apr 4, 2015
zanker pushed a commit to zanker/http.rb that referenced this pull request May 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants