Skip to content

Commit

Permalink
Use separate exceptions for open_timeout, timeout
Browse files Browse the repository at this point in the history
Explicitly track which state we're in with a given connection so that
we know whether we've successfully established the TCP connection.

For timeouts occurring before we enter the body of the Net::HTTP#start
block, assume the request failed establishing the connection, and
raise the new RestClient::ConnectTimeout exception (which, for
compatibility, is a subclass of RestClient::RequestTimeout).

Unfortunately, this is a bit difficult to test without taking
advantage of actual network latency - it seems impossible to force
Linux to delay SYN/ACKing an incoming connection (even if you don't
accept() it and you have listen() backlog set to 0). However, testing
with a sufficiently low open_timeout and a sufficiently distant server
seems to work.

Conflicts:
    lib/restclient/request.rb
  • Loading branch information
ebroder authored and ab committed Dec 5, 2014
1 parent 08480eb commit 5a3b760
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
7 changes: 7 additions & 0 deletions lib/restclient/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,13 @@ def initialize(message)
self.message = message
end
end

class ConnectTimeout < RequestTimeout
def initialize(message='Timed out connecting to server')
super nil, nil
self.message = message
end
end
end

class RestClient::Request
Expand Down
11 changes: 9 additions & 2 deletions lib/restclient/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ def print_verify_callback_warnings
end

def transmit uri, req, payload, & block
established_connection = false

setup_credentials req

net = net_http_class.new(uri.hostname, uri.port)
Expand Down Expand Up @@ -428,6 +430,8 @@ def transmit uri, req, payload, & block


net.start do |http|
established_connection = true

if @block_response
net_http_do_request(http, req, payload ? payload.to_s : nil,
&@block_response)
Expand All @@ -441,8 +445,11 @@ def transmit uri, req, payload, & block
rescue EOFError
raise RestClient::ServerBrokeConnection
rescue Timeout::Error, Errno::ETIMEDOUT
raise RestClient::RequestTimeout

if established_connection
raise RestClient::RequestTimeout
else
raise RestClient::ConnectTimeout
end
rescue OpenSSL::SSL::SSLError => error
# TODO: deprecate and remove RestClient::SSLCertificateNotVerified and just
# pass through OpenSSL::SSL::SSLError directly.
Expand Down

0 comments on commit 5a3b760

Please sign in to comment.