Skip to content

Commit 1a70da9

Browse files
jason-raitzjason-raitz
committed
Add timeout support to URIs request methods and update tests
 - also makes some rubocop and coverage related changes
1 parent 367d787 commit 1a70da9

10 files changed

Lines changed: 71 additions & 35 deletions

File tree

‎.gitignore‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ Gemfile.lock
3535
/test/tmp/
3636
/test/version_tmp/
3737
/tmp/
38+
/vendor
3839

3940
# Used by dotenv library to load environment variables.
4041
# .env
@@ -218,7 +219,8 @@ flycheck_*.el
218219
.LSOverride
219220

220221
# Icon must end with two \r
221-
Icon
222+
Icon
223+
222224

223225
# Thumbnails
224226
._*

‎.ruby-version‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
~> 3.3
1+
3.3

‎lib/berkeley_library/util.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Dir.glob(File.expand_path('util/*.rb', __dir__)).sort.each(&method(:require))
1+
Dir.glob(File.expand_path('util/*.rb', __dir__)).each(&method(:require))

‎lib/berkeley_library/util/arrays.rb‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ def count_while(values:)
6363
# @yieldparam target [Object] the value to compare against
6464
# @return [Array<Integer>, nil] the indices in `in_array` of each value in `for_array`,
6565
# or `nil` if not all values could be found
66-
def find_indices(for_array:, in_array:, &block)
67-
return find_indices_matching(for_array, in_array, &block) if block_given?
66+
def find_indices(for_array:, in_array:, &)
67+
return find_indices_matching(for_array, in_array, &) if block_given?
6868

6969
find_all_indices(for_array, in_array)
7070
end
@@ -89,10 +89,10 @@ def find_indices(for_array:, in_array:, &block)
8989
# @param in_array [Array] the array to search
9090
# @param start_index [Integer] the index to start with
9191
# @return [Enumerator] a new enumerator
92-
def find_index(*args, in_array:, start_index: 0, &block)
92+
def find_index(*args, in_array:, start_index: 0, &)
9393
raise ArgumentError, "wrong number of arguments (given #{args.length}, expected 0..1" if args.size > 1
9494
return Enumerator.new { |y| find_index(in_array: in_array, start_index: start_index, &y) } if args.empty? && !block_given?
95-
return unless (relative_index = in_array[start_index..].find_index(*args, &block))
95+
return unless (relative_index = in_array[start_index..].find_index(*args, &))
9696

9797
relative_index + start_index
9898
end

‎lib/berkeley_library/util/uris.rb‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ def append(uri, *elements)
3434
# @param log [Boolean] whether to log each request URL and response code
3535
# @return [String] the body as a string.
3636
# @raise [RestClient::Exception] in the event of an unsuccessful request.
37-
def get(uri, params: {}, headers: {}, log: true)
38-
Requester.get(uri, params: params, headers: headers, log: log)
37+
def get(uri, params: {}, headers: {}, log: true, timeout: Requester::DEFAULT_TIMEOUT_SECONDS)
38+
Requester.get(uri, params: params, headers: headers, log: log, timeout: timeout)
3939
end
4040

4141
# Performs a HEAD request and returns the response status as an integer.
@@ -47,8 +47,8 @@ def get(uri, params: {}, headers: {}, log: true)
4747
# @param headers [Hash] the request headers.
4848
# @param log [Boolean] whether to log each request URL and response code
4949
# @return [Integer] the response code as an integer.
50-
def head(uri, params: {}, headers: {}, log: true)
51-
Requester.head(uri, params: params, headers: headers, log: log)
50+
def head(uri, params: {}, headers: {}, log: true, timeout: Requester::DEFAULT_TIMEOUT_SECONDS)
51+
Requester.head(uri, params: params, headers: headers, log: log, timeout: timeout)
5252
end
5353

5454
# Performs a GET request and returns the response, even in the event of
@@ -59,8 +59,8 @@ def head(uri, params: {}, headers: {}, log: true)
5959
# @param headers [Hash] the request headers.
6060
# @param log [Boolean] whether to log each request URL and response code
6161
# @return [RestClient::Response] the response
62-
def get_response(uri, params: {}, headers: {}, log: true)
63-
Requester.get_response(uri, params: params, headers: headers, log: log)
62+
def get_response(uri, params: {}, headers: {}, log: true, timeout: Requester::DEFAULT_TIMEOUT_SECONDS)
63+
Requester.get_response(uri, params: params, headers: headers, log: log, timeout: timeout)
6464
end
6565

6666
# Performs a HEAD request and returns the response, even in the event of
@@ -71,8 +71,8 @@ def get_response(uri, params: {}, headers: {}, log: true)
7171
# @param headers [Hash] the request headers.
7272
# @param log [Boolean] whether to log each request URL and response code
7373
# @return [RestClient::Response] the response
74-
def head_response(uri, params: {}, headers: {}, log: true)
75-
Requester.head_response(uri, params: params, headers: headers, log: log)
74+
def head_response(uri, params: {}, headers: {}, log: true, timeout: Requester::DEFAULT_TIMEOUT_SECONDS)
75+
Requester.head_response(uri, params: params, headers: headers, log: log, timeout: timeout)
7676
end
7777

7878
# Returns the specified URL as a URI, or `nil` if the URL is `nil`.

‎lib/berkeley_library/util/uris/head_check.rb‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ def perform_request
1313
headers['Authorization'] = "Basic #{Base64.strict_encode64("#{user}:#{password}")}"
1414
end
1515

16-
URIs.head_response(url, headers: headers, log: false)
16+
options = { headers: headers, log: false }
17+
options[:timeout] = request_timeout.to_i if request_timeout
18+
19+
URIs.head_response(url, **options)
1720
rescue StandardError => e
18-
raise OkComputer::HttpCheck::ConnectionFailed, e.message
21+
raise OkComputer::HttpCheck::ConnectionFailed, e
1922
end
2023
end
2124
end

‎lib/berkeley_library/util/uris/requester.rb‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@ class Requester
2020
RETRY_STATUSES = [429, 503].freeze
2121
MAX_RETRY_DELAY_SECONDS = 10
2222
MAX_RETRIES = 3
23+
DEFAULT_TIMEOUT_SECONDS = 10
2324

2425
# ------------------------------------------------------------
2526
# Attributes
2627

27-
attr_reader :method, :url_str, :headers, :log, :max_retries, :max_retry_delay
28+
attr_reader :method, :url_str, :headers, :log, :max_retries, :max_retry_delay, :timeout
2829

2930
# ------------------------------------------------------------
3031
# Initializer
@@ -38,9 +39,11 @@ class Requester
3839
# @param log [Boolean] whether to log each request URL and response code
3940
# @param max_retries [Integer] the maximum number of times to retry after a 429 or 503 with Retry-After
4041
# @param max_retry_delay [Integer] the maximum retry delay (in seconds) to accept in a Retry-After header
42+
# @param timeout [Integer] the request timeout in seconds (RestClient will use this to set both open and read timeouts)
4143
# @raise URI::InvalidURIError if the specified URL is invalid
4244
# rubocop:disable Metrics/ParameterLists
43-
def initialize(method, url, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS)
45+
def initialize(method, url, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS,
46+
timeout: DEFAULT_TIMEOUT_SECONDS)
4447
raise ArgumentError, "#{method} not supported" unless SUPPORTED_METHODS.include?(method)
4548
raise ArgumentError, 'url cannot be nil' unless (uri = Validator.uri_or_nil(url))
4649

@@ -50,6 +53,7 @@ def initialize(method, url, params: {}, headers: {}, log: true, max_retries: MAX
5053
@log = log
5154
@max_retries = max_retries
5255
@max_retry_delay = max_retry_delay
56+
@timeout = timeout
5357
end
5458

5559
# rubocop:enable Metrics/ParameterLists
@@ -73,7 +77,7 @@ def make_request
7377
private
7478

7579
def log_response(response)
76-
return unless log
80+
return unless log && response&.code
7781

7882
logger.info("#{method.to_s.upcase} #{url_str} returned #{response.code}")
7983
end
@@ -90,6 +94,8 @@ def url_str_with_params(uri, params)
9094

9195
def execute_request(retries_remaining = max_retries)
9296
try_execute_request
97+
rescue RestClient::Exceptions::Timeout
98+
raise
9399
rescue RestClient::Exception => e
94100
response = e.response
95101
raise unless (retry_delay = retry_delay_from(response))
@@ -99,7 +105,7 @@ def execute_request(retries_remaining = max_retries)
99105
end
100106

101107
def try_execute_request
102-
RestClient::Request.execute(method: method, url: url_str, headers: headers).tap do |response|
108+
RestClient::Request.execute(method: method, url: url_str, headers: headers, timeout: timeout).tap do |response|
103109
# Not all failed RestClient requests throw exceptions
104110
raise(exception_for(response)) unless response.code == 200
105111
end

‎lib/berkeley_library/util/uris/requester/class_methods.rb‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ module ClassMethods
1313
# @param log [Boolean] whether to log each request URL and response code
1414
# @param max_retries [Integer] the maximum number of times to retry after a 429 or 503 with Retry-After
1515
# @param max_retry_delay [Integer] the maximum retry delay (in seconds) to accept in a Retry-After header
16+
# @param timeout [Integer] the request timeout in seconds (RestClient will use this to set both open and read timeouts)
1617
# @raise [RestClient::Exception] in the event of an unsuccessful request.
17-
def get(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS)
18-
resp = make_request(:get, uri, params, headers, log, max_retries, max_retry_delay)
18+
def get(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS,
19+
timeout: DEFAULT_TIMEOUT_SECONDS)
20+
resp = make_request(:get, uri, params, headers, log, max_retries, max_retry_delay, timeout)
1921
resp.body
2022
end
2123

@@ -28,8 +30,10 @@ def get(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_r
2830
# @param headers [Hash] the request headers.
2931
# @param log [Boolean] whether to log each request URL and response code
3032
# @return [Integer] the response code as an integer.
31-
def head(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS)
32-
head_response(uri, params: params, headers: headers, log: log, max_retries: max_retries, max_retry_delay: max_retry_delay).code
33+
def head(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS,
34+
timeout: DEFAULT_TIMEOUT_SECONDS)
35+
head_response(uri, params: params, headers: headers, log: log, max_retries: max_retries, max_retry_delay: max_retry_delay,
36+
timeout: timeout).code
3337
end
3438

3539
# Performs a GET request and returns the response, even in the event of
@@ -40,10 +44,13 @@ def head(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_
4044
# @param headers [Hash] the request headers.
4145
# @param log [Boolean] whether to log each request URL and response code
4246
# @return [RestClient::Response] the response
43-
def get_response(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS)
44-
make_request(:get, uri, params, headers, log, max_retries, max_retry_delay)
47+
def get_response(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS,
48+
timeout: DEFAULT_TIMEOUT_SECONDS)
49+
make_request(:get, uri, params, headers, log, max_retries, max_retry_delay, timeout)
4550
rescue RestClient::Exception => e
46-
e.response
51+
return e.response if e.response
52+
53+
raise
4754
end
4855

4956
# Performs a HEAD request and returns the response, even in the event of
@@ -54,23 +61,27 @@ def get_response(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRI
5461
# @param headers [Hash] the request headers.
5562
# @param log [Boolean] whether to log each request URL and response code
5663
# @return [RestClient::Response] the response
57-
def head_response(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS)
58-
make_request(:head, uri, params, headers, log, max_retries, max_retry_delay)
64+
def head_response(uri, params: {}, headers: {}, log: true, max_retries: MAX_RETRIES, max_retry_delay: MAX_RETRY_DELAY_SECONDS,
65+
timeout: DEFAULT_TIMEOUT_SECONDS)
66+
make_request(:head, uri, params, headers, log, max_retries, max_retry_delay, timeout)
5967
rescue RestClient::Exception => e
60-
e.response
68+
return e.response if e.response
69+
70+
raise
6171
end
6272

6373
private
6474

65-
def make_request(method, url, params, headers, log, max_retries, max_retry_delay)
75+
def make_request(method, url, params, headers, log, max_retries, max_retry_delay, timeout)
6676
Requester.new(
6777
method,
6878
url,
6979
params: params,
7080
headers: headers,
7181
log: log,
7282
max_retries: max_retries,
73-
max_retry_delay: max_retry_delay
83+
max_retry_delay: max_retry_delay,
84+
timeout: timeout
7485
).make_request
7586
end
7687

‎spec/berkeley_library/util/uris/head_check_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ module Util
2323

2424
it 'calls URIs.head_response with the correct URL' do
2525
check.perform_request
26-
expect(BerkeleyLibrary::Util::URIs).to have_received(:head_response).with(URI(url), headers: {}, log: false)
26+
expect(BerkeleyLibrary::Util::URIs).to have_received(:head_response).with(URI(url), headers: {}, log: false, timeout: 5)
2727
end
2828
end
2929

@@ -40,7 +40,7 @@ module Util
4040
expected_headers = { 'Authorization' => "Basic #{Base64.strict_encode64("#{user}:#{password}")}" }
4141

4242
check.perform_request
43-
expect(BerkeleyLibrary::Util::URIs).to have_received(:head_response).with(URI(url), headers: expected_headers, log: false)
43+
expect(BerkeleyLibrary::Util::URIs).to have_received(:head_response).with(URI(url), headers: expected_headers, log: false, timeout: 5)
4444
end
4545
end
4646

‎spec/berkeley_library/util/uris/requester_spec.rb‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,13 @@ module URIs
305305
requester = Requester.new(:get, url)
306306
expect { requester.make_request }.to raise_error(RestClient::ServiceUnavailable)
307307
end
308+
309+
it "raises #{RestClient::Exceptions::Timeout} when the request times out" do
310+
url = 'http://example.edu/timeout'
311+
stub_request(:get, url).to_raise(RestClient::Exceptions::Timeout)
312+
313+
expect { Requester.get(url, timeout: 10) }.to raise_error(RestClient::Exceptions::Timeout)
314+
end
308315
end
309316
end
310317
end
@@ -374,6 +381,13 @@ module URIs
374381
end
375382
end
376383

384+
it "raises #{RestClient::Exceptions::Timeout} when the request times out" do
385+
url = 'http://example.edu/timeout'
386+
stub_request(:head, url).to_raise(RestClient::Exceptions::Timeout)
387+
388+
expect { Requester.head(url, timeout: 10) }.to raise_error(RestClient::Exceptions::Timeout)
389+
end
390+
377391
it 'handles redirects' do
378392
url1 = 'https://example.org/'
379393
url2 = 'https://example.edu/'

0 commit comments

Comments
 (0)