Skip to content

Commit 86232d6

Browse files
committed
open: Never call Timeout.timeout in rescue clause
The try-open_timeout-then-fallback-to-timeout introduced in 1903ced works well, but when it errors due to any reason in Rubies which do not support `open_timeout`, it spits the rescued ArgumentError that is unrelated to user code and not actionable. Net::HTTP.start('foo.bar', 80) /.../net-http-0.8.0/lib/net/http.rb:1691:in 'TCPSocket#initialize': Failed to open TCP connection to foo.bar:80 (getaddrinfo(3): nodename nor servname provided, or not known) (Socket::ResolutionError) from /.../net-http-0.8.0/lib/net/http.rb:1691:in 'IO.open' from /.../net-http-0.8.0/lib/net/http.rb:1691:in 'block in Net::HTTP#connect' from /.../timeout-0.4.4/lib/timeout.rb:188:in 'block in Timeout.timeout' from /.../timeout-0.4.4/lib/timeout.rb:195:in 'Timeout.timeout' from /.../net-http-0.8.0/lib/net/http.rb:1690:in 'Net::HTTP#connect' from /.../net-http-0.8.0/lib/net/http.rb:1655:in 'Net::HTTP#do_start' from /.../net-http-0.8.0/lib/net/http.rb:1635:in 'Net::HTTP#start' from /.../net-http-0.8.0/lib/net/http.rb:1064:in 'Net::HTTP.start' (snip) /.../net-http-0.8.0/lib/net/http.rb:1682:in 'TCPSocket#initialize': unknown keyword: :open_timeout (ArgumentError) sock = TCPSocket.open(conn_addr, conn_port, @local_host, @local_port, open_timeout: @open_timeout) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ from /.../net-http-0.8.0/lib/net/http.rb:1682:in 'IO.open' from /.../net-http-0.8.0/lib/net/http.rb:1682:in 'Net::HTTP#connect' from /.../net-http-0.8.0/lib/net/http.rb:1655:in 'Net::HTTP#do_start' from /.../net-http-0.8.0/lib/net/http.rb:1635:in 'Net::HTTP#start' from /.../net-http-0.8.0/lib/net/http.rb:1064:in 'Net::HTTP.start' (snip) ... 8 levels... This patch suppresses the ArgumentError by moving the retry out of the rescue clause.
1 parent 9c2c2f4 commit 86232d6

File tree

1 file changed

+24
-24
lines changed

1 file changed

+24
-24
lines changed

lib/net/http.rb

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,30 +1674,7 @@ def connect
16741674

16751675
debug "opening connection to #{conn_addr}:#{conn_port}..."
16761676
begin
1677-
s =
1678-
case @tcpsocket_supports_open_timeout
1679-
when nil, true
1680-
begin
1681-
# Use built-in timeout in TCPSocket.open if available
1682-
sock = TCPSocket.open(conn_addr, conn_port, @local_host, @local_port, open_timeout: @open_timeout)
1683-
@tcpsocket_supports_open_timeout = true
1684-
sock
1685-
rescue ArgumentError => e
1686-
raise if !(e.message.include?('unknown keyword: :open_timeout') || e.message.include?('wrong number of arguments (given 5, expected 2..4)'))
1687-
@tcpsocket_supports_open_timeout = false
1688-
1689-
# Fallback to Timeout.timeout if TCPSocket.open does not support open_timeout
1690-
Timeout.timeout(@open_timeout, Net::OpenTimeout) {
1691-
TCPSocket.open(conn_addr, conn_port, @local_host, @local_port)
1692-
}
1693-
end
1694-
when false
1695-
# The current Ruby is known to not support TCPSocket(open_timeout:).
1696-
# Directly fall back to Timeout.timeout to avoid performance penalty incured by rescue.
1697-
Timeout.timeout(@open_timeout, Net::OpenTimeout) {
1698-
TCPSocket.open(conn_addr, conn_port, @local_host, @local_port)
1699-
}
1700-
end
1677+
s = timeouted_connect(conn_addr, conn_port)
17011678
rescue => e
17021679
e = Net::OpenTimeout.new(e) if e.is_a?(Errno::ETIMEDOUT) # for compatibility with previous versions
17031680
raise e, "Failed to open TCP connection to " +
@@ -1795,6 +1772,29 @@ def connect
17951772
end
17961773
private :connect
17971774

1775+
def timeouted_connect(conn_addr, conn_port)
1776+
if @tcpsocket_supports_open_timeout == nil || @tcpsocket_supports_open_timeout == true
1777+
# Try to use built-in open_timeout in TCPSocket.open if:
1778+
# - The current Ruby runtime is known to support it, or
1779+
# - It is unknown whether the current Ruby runtime supports it (so we'll try).
1780+
begin
1781+
sock = TCPSocket.open(conn_addr, conn_port, @local_host, @local_port, open_timeout: @open_timeout)
1782+
@tcpsocket_supports_open_timeout = true
1783+
return sock
1784+
rescue ArgumentError => e
1785+
raise if !(e.message.include?('unknown keyword: :open_timeout') || e.message.include?('wrong number of arguments (given 5, expected 2..4)'))
1786+
@tcpsocket_supports_open_timeout = false
1787+
end
1788+
end
1789+
1790+
# This Ruby runtime is known not to support `TCPSocket(open_timeout:)`.
1791+
# Directly fall back to Timeout.timeout to avoid performance penalty incured by rescue.
1792+
Timeout.timeout(@open_timeout, Net::OpenTimeout) {
1793+
TCPSocket.open(conn_addr, conn_port, @local_host, @local_port)
1794+
}
1795+
end
1796+
private :timeouted_connect
1797+
17981798
def on_connect
17991799
end
18001800
private :on_connect

0 commit comments

Comments
 (0)