Inconsistent retry mechanism for nil and empty array bodies.

[binarycode]

Request retry mechanism is working inconstently based on wheither the request body is nil or empty array.

Consider following spec:

require_relative 'server_context'

require 'async/http/client'
require 'async/http/endpoint'

RSpec.describe 'inconsistent retry behaviour' do
    include_context Async::HTTP::Server

    let(:delay) { 0.1 }
    let(:retries) { 2 }
    let(:protocol) { Async::HTTP::Protocol::HTTP1 }

    let(:server) do
        Async::HTTP::Server.for(endpoint, protocol) do |request|
            Async::Task.current.sleep(delay)
            Protocol::HTTP::Response[200, {}, []]
        end
    end

    def make_request(body)
        Async::Task.current.with_timeout(delay / 2) do
            client.get('/', {}, body)
        end
    end

    specify 'nil body' do
        make_request(nil)
    end

    specify 'empty array body' do
        make_request([])
    end
end

The nil body spec will pass, as the request will be retried after timeout, while the empty array body spec will fail as there will be no retries (actual requests being made will be completely identical and empty in both cases).

Looks like this is happening because the request is considered to be non-idempotent in the empty array body case, here is the chain of events:

1. Protocol::HTTP1::Connection#write_empty_body sends the request and closes the body:

   # protocol-http1 gem, lib/protocol/http1/connection.rb
   247       def write_empty_body(body)
   248         @stream.write("content-length: 0\r\n\r\n")
   249         @stream.flush
   250
   251         body&.close
   252       end

2. Protocol::HTTP::Body::Buffered#close appends nil to body request chunks:

   # protocol-http gem, lib/protocol/http/body/buffered.rb
   85         def close(error = nil)
   86           @chunks << nil
   87         end

3. Once the exception is raised, rescue handler tries to determine if it's safe to retry it, and checks idempotency of the request:

   # protocol-http gem, lib/protocol/http/request.rb
   70       def idempotent?
   71         @method != Methods::POST && (@body.nil? || @body.empty?)
   72       end

4. @body.empty? returns false, as there is a single chunk after step 2, the request is considered to be non-idempotent and is not retried

   # protocol-http gem, lib/protocol/http/body/buffered.rb
   69         def empty?
   70           @index >= @chunks.length
   71         end

[ioquatix]

Thanks for the detailed analysis. This looks like a bug.

[ioquatix]

This should have been fixed in https://github.com/socketry/protocol-http/commit/4902a8fd22f1040b6e3074cf0dc9d3608d97b6ab#diff-22b9fd4a82daf26e77d916d9a099aded5baf5c14ab03f0da9776ad6dba794c14