(PUP-11853) Wait for request completion before closing

Before this commit, puppet would attempt to `finish` a
connection in the block passed to an `http.request` function. This
will not work because `finish` sets the `@socket` to nil, and the
net/http request assumes that the `@socket` is set for the duration
of that function. With this change, we mark some state during the
request function that indicates the connection should be closed
after the completion of the `http.request`.

lib/puppet/http/client.rb

@@ -367,6 +367,7 @@ def execute_streaming(request, options: {}, &block)
         apply_auth(request, basic_auth) if redirects.zero?
 
         # don't call return within the `request` block
+        close_connection_after_request = false
         http.request(request) do |nethttp|
           response = Puppet::HTTP::ResponseNetHTTP.new(request.uri, nethttp)
           begin
@@ -381,8 +382,7 @@ def execute_streaming(request, options: {}, &block)
               retries += 1
               if interval
                 if http.started?
-                  Puppet.debug("Closing connection for #{Puppet::HTTP::Site.from_uri(request.uri)}")
-                  http.finish
+                  close_connection_after_request = true
                 end
                 Puppet.warning(_("Sleeping for %{interval} seconds before retrying the request") % { interval: interval })
                 ::Kernel.sleep(interval)
@@ -404,6 +404,13 @@ def execute_streaming(request, options: {}, &block)
 
           done = true
         end
+      ensure
+        # Connections cannot be closed inside a block passed to http.request,
+        # so close it after the function has been completed.
+        if close_connection_after_request
+          Puppet.debug("Closing connection for #{Puppet::HTTP::Site.from_uri(request.uri)}")
+          http.finish
+        end
       end
     end
 

spec/unit/http/client_spec.rb

@@ -862,8 +862,8 @@ def retry_after(datetime)
       # The "with_connection" method is required to yield started connections
       allow(pool).to receive(:with_connection).and_yield(http1).and_yield(http2)
 
-      expect(http1).to receive(:finish).ordered
       expect(::Kernel).to receive(:sleep).with(42).ordered
+      expect(http1).to receive(:finish).ordered
 
       client.get(uri)
     end

spec/unit/network/http/connection_spec.rb

@@ -515,8 +515,8 @@ def retry_after(datetime)
 
         allow(pool).to receive(:with_connection).and_yield(http1).and_yield(http2)
 
-        expect(http1).to receive(:finish).ordered
         expect(::Kernel).to receive(:sleep).with(42).ordered
+        expect(http1).to receive(:finish).ordered
 
         subject.get('/foo')
       end