make request_timeout reset on returned response, rather than response callback

this makes it not reset on redirect or retried responses, and effectively makes it act as a max-time for individual transactions/requests
This commit is contained in:
HoneyryderChuck 2024-07-19 12:15:13 +01:00
parent fafe7c140c
commit b8db28abd2
7 changed files with 57 additions and 19 deletions

View File

@ -666,25 +666,37 @@ module HTTPX
end
def set_request_timeouts(request)
write_timeout = request.write_timeout
set_request_write_timeout(request)
set_request_read_timeout(request)
set_request_request_timeout(request)
end
def set_request_read_timeout(request)
read_timeout = request.read_timeout
return if read_timeout.nil? || read_timeout.infinite?
set_request_timeout(request, read_timeout, :done, :response) do
read_timeout_callback(request, read_timeout)
end
end
def set_request_write_timeout(request)
write_timeout = request.write_timeout
return if write_timeout.nil? || write_timeout.infinite?
set_request_timeout(request, write_timeout, :headers, %i[done response]) do
write_timeout_callback(request, write_timeout)
end
end
def set_request_request_timeout(request)
request_timeout = request.request_timeout
unless write_timeout.nil? || write_timeout.infinite?
set_request_timeout(request, write_timeout, :headers, %i[done response]) do
write_timeout_callback(request, write_timeout)
end
end
unless read_timeout.nil? || read_timeout.infinite?
set_request_timeout(request, read_timeout, :done, :response) do
read_timeout_callback(request, read_timeout)
end
end
return if request_timeout.nil? || request_timeout.infinite?
set_request_timeout(request, request_timeout, :headers, :response) do
set_request_timeout(request, request_timeout, :headers, :complete) do
read_timeout_callback(request, request_timeout, RequestTimeoutError)
end
end

View File

@ -187,6 +187,16 @@ module HTTPX
@options.max_redirects || MAX_REDIRECTS
end
end
module ConnectionMethods
private
def set_request_request_timeout(request)
return unless request.root_request.nil?
super
end
end
end
register_plugin :follow_redirects, FollowRedirects
end

View File

@ -76,7 +76,7 @@ module HTTPX
module InstanceMethods
def max_retries(n)
with(max_retries: n.to_i)
with(max_retries: n)
end
private

View File

@ -261,6 +261,7 @@ module HTTPX
return responses unless request
catch(:coalesced) { pool.next_tick } until (response = fetch_response(request, connections, request.options))
request.emit(:complete, response)
responses << response
requests.shift
@ -274,7 +275,9 @@ module HTTPX
# opportunity to traverse the requests, hence we're returning only a fraction of the errors
# we were supposed to. This effectively fetches the existing responses and return them.
while (request = requests.shift)
responses << fetch_response(request, connections, request.options)
response = fetch_response(request, connections, request.options)
request.emit(:complete, response) if response
responses << response
end
break
end

View File

@ -127,6 +127,12 @@ module HTTPX
def set_request_timeouts: (Request request) -> void
def set_request_read_timeout: (Request request) -> void
def set_request_write_timeout: (Request request) -> void
def set_request_request_timeout: (Request request) -> void
def write_timeout_callback: (Request request, Numeric write_timeout) -> void
def read_timeout_callback: (Request request, Numeric read_timeout, ?singleton(RequestTimeoutError) error_type) -> void

View File

@ -111,6 +111,13 @@ module Requests
assert total_time >= 2, "request didn't take as expected to redirect (#{total_time} secs)"
end
def test_plugin_follow_redirects_retry_after_with_request_timeout
session = HTTPX.plugin(SessionWithMockResponse, mock_status: 302, mock_headers: { "retry-after" => "2" }).plugin(:follow_redirects)
timeout_response = session.get(max_redirect_uri(2), timeout: { request_timeout: 1 })
verify_error_response(timeout_response, HTTPX::RequestTimeoutError)
end
def test_plugin_follow_insecure_no_insecure_downgrade
return unless origin.start_with?("https")

View File

@ -79,7 +79,7 @@ module Requests
after_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second)
total_time = after_time - before_time
verify_error_response(retries_response)
verify_error_response(retries_response, HTTPX::RequestTimeoutError)
verify_execution_delta(3 + 2 + 3, total_time, 1)
end
@ -94,7 +94,7 @@ module Requests
after_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second)
total_time = after_time - before_time
verify_error_response(retries_response)
verify_error_response(retries_response, HTTPX::RequestTimeoutError)
verify_execution_delta(3 + 2 + 1 + 3, total_time, 1)
end
@ -111,7 +111,7 @@ module Requests
after_time = Process.clock_gettime(Process::CLOCK_MONOTONIC, :second)
total_time = after_time - before_time
verify_error_response(retries_response)
verify_error_response(retries_response, HTTPX::RequestTimeoutError)
verify_execution_delta(3 + 2 + 3 + 4 + 3, total_time, 1)
end