Improve ReadTotalTimeout to start with reading

Initially, the timer for the ReadTotalTimeout started
counting on the beginning of the request. For large
uploads like LFS this could be problematic because the
timeout can be exceeded even before the server had a chance
to respond.

Changelog: fixed

lib/gitlab/http.rb

@@ -49,11 +49,12 @@ module Gitlab
         return httparty_perform_request(http_method, path, options_with_timeouts, &block)
       end
 
-      start_time = Gitlab::Metrics::System.monotonic_time
+      start_time = nil
       read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT)
       tracked_timeout_error = false
 
       httparty_perform_request(http_method, path, options_with_timeouts) do |fragment|
+        start_time ||= Gitlab::Metrics::System.monotonic_time
         elapsed = Gitlab::Metrics::System.monotonic_time - start_time
 
         if elapsed > read_total_timeout

spec/lib/gitlab/http_spec.rb

@@ -29,12 +29,40 @@ RSpec.describe Gitlab::HTTP do
 
   context 'when reading the response is too slow' do
     before do
+      # Override Net::HTTP to add a delay between sending each response chunk
+      mocked_http = Class.new(Net::HTTP) do
+        def request(*)
+          super do |response|
+            response.instance_eval do
+              def read_body(*)
+                @body.each do |fragment|
+                  sleep 0.002.seconds
+
+                  yield fragment if block_given?
+                end
+              end
+            end
+
+            yield response if block_given?
+
+            response
+          end
+        end
+      end
+
+      @original_net_http = Net.send(:remove_const, :HTTP)
+      Net.send(:const_set, :HTTP, mocked_http)
+
       stub_const("#{described_class}::DEFAULT_READ_TOTAL_TIMEOUT", 0.001.seconds)
 
       WebMock.stub_request(:post, /.*/).to_return do |request|
-        sleep 0.002.seconds
-        { body: 'I\'m slow', status: 200 }
+        { body: %w(a b), status: 200 }
+      end
     end
+
+    after do
+      Net.send(:remove_const, :HTTP)
+      Net.send(:const_set, :HTTP, @original_net_http)
     end
 
     let(:options) { {} }
@@ -51,7 +79,7 @@ RSpec.describe Gitlab::HTTP do
       end
 
       it 'still calls the block' do
-        expect { |b| described_class.post('http://example.org', **options, &b) }.to yield_with_args
+        expect { |b| described_class.post('http://example.org', **options, &b) }.to yield_successive_args('a', 'b')
       end
     end