Skip to content
Permalink
Browse files

Simplify setting timeouts

- `.timeout(<Numeric>)` sets a global timeout value.
- `.timeout(connect: x, write: y, read: z)` sets per operation timeouts.

The previous style: `.timeout(:global|:per_operation, hash_of_options)` doesn't work anymore.
  • Loading branch information
mikegee committed Dec 31, 2017
1 parent 4d6c357 commit 11e158060e710680c011b5c57ad05a9f205a6fd3
Showing with 102 additions and 134 deletions.
  1. +14 −19 lib/http/chainable.rb
  2. +12 −14 lib/http/timeout/global.rb
  3. +5 −7 lib/http/timeout/per_operation.rb
  4. +11 −30 spec/lib/http_spec.rb
  5. +60 −64 spec/support/http_handling_shared.rb
@@ -77,29 +77,24 @@ def request(verb, uri, options = {}) # rubocop:disable Style/OptionHash
end

# @overload timeout(options = {})
# Syntax sugar for `timeout(:per_operation, options)`
# @overload timeout(klass, options = {})
# Adds a timeout to the request.
# @param [#to_sym] klass
# either :null, :global, or :per_operation
# Adds per operation timeouts to the request
# @param [Hash] options
# @option options [Float] :read Read timeout
# @option options [Float] :write Write timeout
# @option options [Float] :connect Connect timeout
def timeout(klass, options = {}) # rubocop:disable Style/OptionHash
if klass.is_a? Hash
options = klass
klass = :per_operation
end

klass = case klass.to_sym
when :null then HTTP::Timeout::Null
when :global then HTTP::Timeout::Global
when :per_operation then HTTP::Timeout::PerOperation
else raise ArgumentError, "Unsupported Timeout class: #{klass}"
end

%i[read write connect].each do |k|
# @overload timeout(global_timeout)
# Adds a global timeout to the full request
# @param [Numeric] global_timeout
def timeout(options)
klass, options = case options
when Numeric then [HTTP::Timeout::Global, {:global => options}]
when Hash then [HTTP::Timeout::PerOperation, options]
when :null then [HTTP::Timeout::Null, {}]
else raise ArgumentError, "Use `.timeout(global_timeout_in_seconds)` or `.timeout(connect: x, write: y, read: z)`."

end

%i[global read write connect].each do |k|
next unless options.key? k
options["#{k}_timeout".to_sym] = options.delete k
end
@@ -3,27 +3,25 @@
require "timeout"
require "io/wait"

require "http/timeout/per_operation"
require "http/timeout/null"

module HTTP
module Timeout
class Global < PerOperation
attr_reader :time_left, :total_timeout

class Global < Null
def initialize(*args)
super
reset_counter

@timeout = @time_left = options.fetch(:global_timeout)
end

# To future me: Don't remove this again, past you was smarter.
def reset_counter
@time_left = connect_timeout + read_timeout + write_timeout
@total_timeout = time_left
@time_left = @timeout
end

def connect(socket_class, host, port, nodelay = false)
reset_timer
::Timeout.timeout(time_left, TimeoutError) do
::Timeout.timeout(@time_left, TimeoutError) do
@socket = socket_class.open(host, port)
@socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay
end
@@ -37,11 +35,11 @@ def connect_ssl
begin
@socket.connect_nonblock
rescue IO::WaitReadable
IO.select([@socket], nil, nil, time_left)
IO.select([@socket], nil, nil, @time_left)
log_time
retry
rescue IO::WaitWritable
IO.select(nil, [@socket], nil, time_left)
IO.select(nil, [@socket], nil, @time_left)
log_time
retry
end
@@ -105,13 +103,13 @@ def perform_io

# Wait for a socket to become readable
def wait_readable_or_timeout
@socket.to_io.wait_readable(time_left)
@socket.to_io.wait_readable(@time_left)
log_time
end

# Wait for a socket to become writable
def wait_writable_or_timeout
@socket.to_io.wait_writable(time_left)
@socket.to_io.wait_writable(@time_left)
log_time
end

@@ -123,8 +121,8 @@ def reset_timer

def log_time
@time_left -= (Time.now - @started)
if time_left <= 0
raise TimeoutError, "Timed out after using the allocated #{total_timeout} seconds"
if @time_left <= 0
raise TimeoutError, "Timed out after using the allocated #{@timeout} seconds"
end

reset_timer
@@ -11,8 +11,6 @@ class PerOperation < Null
WRITE_TIMEOUT = 0.25
READ_TIMEOUT = 0.25

attr_reader :read_timeout, :write_timeout, :connect_timeout

def initialize(*args)
super

@@ -22,7 +20,7 @@ def initialize(*args)
end

def connect(socket_class, host, port, nodelay = false)
::Timeout.timeout(connect_timeout, TimeoutError) do
::Timeout.timeout(@connect_timeout, TimeoutError) do
@socket = socket_class.open(host, port)
@socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) if nodelay
end
@@ -67,7 +65,7 @@ def readpartial(size)
return :eof if result.nil?
return result if result != :wait_readable

raise TimeoutError, "Read timed out after #{read_timeout} seconds" if timeout
raise TimeoutError, "Read timed out after #{@read_timeout} seconds" if timeout
# marking the socket for timeout. Why is this not being raised immediately?
# it seems there is some race-condition on the network level between calling
# #read_nonblock and #wait_readable, in which #read_nonblock signalizes waiting
@@ -78,7 +76,7 @@ def readpartial(size)
# timeout. Else, the first timeout was a proper timeout.
# This hack has to be done because io/wait#wait_readable doesn't provide a value for when
# the socket is closed by the server, and HTTP::Parser doesn't provide the limit for the chunks.
timeout = true unless @socket.to_io.wait_readable(read_timeout)
timeout = true unless @socket.to_io.wait_readable(@read_timeout)
end
end

@@ -89,9 +87,9 @@ def write(data)
result = @socket.write_nonblock(data, :exception => false)
return result unless result == :wait_writable

raise TimeoutError, "Write timed out after #{write_timeout} seconds" if timeout
raise TimeoutError, "Write timed out after #{@write_timeout} seconds" if timeout

timeout = true unless @socket.to_io.wait_writable(write_timeout)
timeout = true unless @socket.to_io.wait_writable(@write_timeout)
end
end

@@ -49,12 +49,12 @@
end

context "with a large request body" do
%w[global null per_operation].each do |timeout|
context "with a #{timeout} timeout" do
[:null, 6, {:read => 2, :write => 2, :connect => 2}].each do |timeout|
context "with `.timeout(#{timeout.inspect})`" do
[16_000, 16_500, 17_000, 34_000, 68_000].each do |size|
[0, rand(0..100), rand(100..1000)].each do |fuzzer|
context "with a #{size} body and #{fuzzer} of fuzzing" do
let(:client) { HTTP.timeout(timeout, :read => 2, :write => 2, :connect => 2) }
let(:client) { HTTP.timeout(timeout) }

let(:characters) { ("A".."Z").to_a }
let(:request_body) do
@@ -299,31 +299,17 @@
end

describe ".timeout" do
context "without timeout type" do
subject(:client) { HTTP.timeout :read => 123 }

it "sets timeout_class to PerOperation" do
expect(client.default_options.timeout_class).
to be HTTP::Timeout::PerOperation
end

it "sets given timeout options" do
expect(client.default_options.timeout_options).
to eq :read_timeout => 123
end
end

context "with :null type" do
subject(:client) { HTTP.timeout :null, :read => 123 }
context "specifying a null timeout" do
subject(:client) { HTTP.timeout :null }

it "sets timeout_class to Null" do
expect(client.default_options.timeout_class).
to be HTTP::Timeout::Null
end
end

context "with :per_operation type" do
subject(:client) { HTTP.timeout :per_operation, :read => 123 }
context "specifying per operation timeouts" do
subject(:client) { HTTP.timeout :read => 123 }

it "sets timeout_class to PerOperation" do
expect(client.default_options.timeout_class).
@@ -336,24 +322,19 @@
end
end

context "with :global type" do
subject(:client) { HTTP.timeout :global, :read => 123 }
context "specifying a global timeout" do
subject(:client) { HTTP.timeout 123 }

it "sets timeout_class to Global" do
expect(client.default_options.timeout_class).
to be HTTP::Timeout::Global
end

it "sets given timeout options" do
it "sets given timeout option" do
expect(client.default_options.timeout_options).
to eq :read_timeout => 123
to eq :global_timeout => 123
end
end

it "fails with unknown timeout type" do
expect { HTTP.timeout(:foobar, :read => 123) }.
to raise_error(ArgumentError, /foobar/)
end
end

describe ".cookies" do

0 comments on commit 11e1580

Please sign in to comment.
You can’t perform that action at this time.