fix: do not close request right after sending it, assume it may have to be retried

with the retries plugin, the request payload will be rewinded, and that may not be possible if already closed. this was never detected so far because no request body transcoder internally closes, but the faraday multipart adapter does

the request is therefore closed alongside the response (when the latter is closed)

Fixes https://github.com/HoneyryderChuck/httpx/issues/75\#issuecomment-2731219586
This commit is contained in:
HoneyryderChuck 2025-03-19 16:57:03 +00:00
parent 8287a55b95
commit 4653b48602
6 changed files with 38 additions and 6 deletions

View File

@ -57,6 +57,7 @@ group :test do
gem "aws-sdk-s3"
gem "faraday"
gem "faraday-multipart"
gem "idnx"
gem "oga"

View File

@ -167,7 +167,7 @@ module HTTPX
unless response.headers.key?("accept-ranges") &&
response.headers["accept-ranges"] == "bytes" && # there's nothing else supported though...
(original_body = response.body)
response.close if response.respond_to?(:close)
response.body.close
return
end

View File

@ -50,6 +50,9 @@ module HTTPX
# will be +true+ when request body has been completely flushed.
def_delegator :@body, :empty?
# closes the body
def_delegator :@body, :close
# initializes the instance with the given +verb+ (an upppercase String, ex. 'GEt'),
# an absolute or relative +uri+ (either as String or URI::HTTP object), the
# request +options+ (instance of HTTPX::Options) and an optional Hash of +params+.
@ -273,7 +276,6 @@ module HTTPX
when :done
return if @state == :expect
@body.close
end
@state = nextstate
emit(@state, self)

View File

@ -52,9 +52,6 @@ module HTTPX
# copies the response body to a different location.
def_delegator :@body, :copy_to
# closes the body.
def_delegator :@body, :close
# the corresponding request uri.
def_delegator :@request, :uri
@ -74,6 +71,12 @@ module HTTPX
@content_type = nil
end
# closes the respective +@request+ and +@body+.
def close
@request.close
@body.close
end
# merges headers defined in +h+ into the response headers.
def merge_headers(h)
@headers = @headers.merge(h)
@ -264,7 +267,7 @@ module HTTPX
# closes the error resources.
def close
@response.close if @response && @response.respond_to?(:close)
@response.close if @response
end
# always true for error responses.

View File

@ -28,6 +28,10 @@ module HTTPX
def initialize: (Symbol | String verb, generic_uri uri, Options options, ?request_params params) -> untyped
def empty?: () -> bool
def close: () -> void
def interests: () -> (:r | :w)
def merge_headers: (_Each[[String, headers_value]]) -> void

View File

@ -2,6 +2,7 @@
require_relative "../test_helper"
require "faraday"
require "faraday/multipart"
require "forwardable"
require "httpx/adapters/faraday"
require_relative "../support/http_helpers"
@ -237,6 +238,27 @@ class FaradayTest < Minitest::Test
# TODO: test that request has been proxied
end
def test_adapter_multipart_retried
request = nil
check_error = lambda { |response|
request = response.instance_variable_get(:@request)
(response.is_a?(HTTPX::ErrorResponse) && response.error.is_a?(HTTPX::TimeoutError)) || response.status == 405
}
session = nil
faraday_conn = faraday_connection(request: { read_timeout: 2 }) do |http|
session = http.with(max_retries: 1, retry_on: check_error)
end
adapter_handler = faraday_conn.builder.handlers.last
faraday_conn.builder.insert_before adapter_handler, Faraday::Multipart::Middleware
response = faraday_conn.post(build_path("/delay/4")) do |req|
req.body = {
image: Faraday::UploadIO.new(StringIO.new(File.read(fixture_file_path)), "image/jpeg"),
}
end
verify_status(response, 405)
assert request.retries.zero?, "expect request to have exhausted retries"
end
private
def origin(orig = httpbin)