Skip to content

Commit

Permalink
Replace Timeout.timeout in Net:HTTP#connect
Browse files Browse the repository at this point in the history
Use Socket.tcp's connect_timeout option instead
  • Loading branch information
mohamedhafez committed Jan 29, 2021
1 parent d9ba437 commit 753cae3
Showing 1 changed file with 7 additions and 8 deletions.
15 changes: 7 additions & 8 deletions lib/net/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -982,14 +982,13 @@ def connect
end

D "opening connection to #{conn_addr}:#{conn_port}..."
s = Timeout.timeout(@open_timeout, Net::OpenTimeout) {
begin
TCPSocket.open(conn_addr, conn_port, @local_host, @local_port)
rescue => e
raise e, "Failed to open TCP connection to " +
"#{conn_addr}:#{conn_port} (#{e.message})"
end
}
begin
s = Socket.tcp conn_addr, conn_port, @local_host, @local_port, connect_timeout: @open_timeout
rescue => e
e = Net::OpenTimeout.new(e) if e.is_a?(Errno::ETIMEDOUT) #for compatibility with previous versions
raise e, "Failed to open TCP connection to " +
"#{conn_addr}:#{conn_port} (#{e.message})"
end
s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
D "opened"
if use_ssl?
Expand Down

3 comments on commit 753cae3

@danielwaterworth
Copy link

Choose a reason for hiding this comment

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

Doesn't this change the meaning of open_timeout: 0?

With Timeout.timeout, 0 is equivalent to nil, but with Socket.tcp, 0 means 0.

@mohamedhafez
Copy link
Contributor Author

@mohamedhafez mohamedhafez commented on 753cae3 Feb 7, 2023

Choose a reason for hiding this comment

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

This change was reverted in 98caa38, not sure why. If there are edge cases like, it would have been nice to just take them into account and preserve the behavior, but it's a moot point now

@mohamedhafez
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct though, I'll update my the monkeypatch I'm using, thanks!

Please sign in to comment.