Only retry idempotent requests

lib/net/http/pipeline.rb

@@ -181,7 +181,7 @@ def pipeline requests, &block # :yields: response
           requests.unshift request
         end
 
-        raise if retried
+        raise if retried or not idempotent? requests.first
 
         retried = true
 
@@ -248,6 +248,14 @@ def pipeline_end_transport res
     end
   end
 
+  ##
+  # Closes the connection and rescues any IOErrors this may cause
+
+  def pipeline_finish
+    finish
+  rescue IOError
+  end
+
   if Net::HTTPResponse.allocate.respond_to? :connection_close? then
     ##
     # Checks for an connection close header
@@ -287,26 +295,23 @@ def pipeline_receive in_flight, responses
     responses
   rescue Timeout::Error, EOFError, Errno::ECONNABORTED, Errno::ECONNRESET,
          Errno::EPIPE, Net::HTTPBadResponse => e
+    pipeline_finish
+
     raise ResponseError.new(e, in_flight, responses)
   end
 
   ##
   # Resets this connection
 
   def pipeline_reset requests, responses
-    begin
-      finish
-    rescue IOError
-    end
-
-    begin
-      start
-    rescue Errno::ECONNREFUSED
-      raise Error.new("connection refused: #{address}:#{port}", requests,
-                      responses)
-    rescue Errno::EHOSTDOWN
-      raise Error.new("host down: #{address}:#{port}", requests, responses)
-    end
+    pipeline_finish
+
+    start
+  rescue Errno::ECONNREFUSED
+    raise Error.new("connection refused: #{address}:#{port}", requests,
+                    responses)
+  rescue Errno::EHOSTDOWN
+    raise Error.new("host down: #{address}:#{port}", requests, responses)
   end
 
   ##

test/test_net_http_pipeline.rb

@@ -63,8 +63,8 @@ def write data
   end
 
   class ErrorBuffer < Buffer
-    def initialize exception
-      @readline = false
+    def initialize exception, immediate = false
+      @readline = immediate
       @exception = exception
       super()
     end
@@ -88,6 +88,7 @@ def edit_path path
   end
 
   def finish
+    @socket.close
   end
 
   def http_get
@@ -284,10 +285,6 @@ def start
       @socket = @good_socket
     end
 
-    def finish
-      @socket.close
-    end
-
     requests = [@get1, @get2, @get3]
 
     responses = pipeline requests
@@ -309,6 +306,27 @@ def finish
     assert_empty requests
   end
 
+  def test_pipeline_retry_post
+    self.pipelining = true
+
+    @socket = ErrorBuffer.new Errno::ECONNRESET, true
+    @socket.start
+
+    requests = [@post]
+
+    e = assert_raises Net::HTTP::Pipeline::ResponseError do
+      pipeline requests
+    end
+
+    @socket.finish
+
+    assert_equal http_post, @socket.write_io.read
+
+    assert_empty e.responses
+
+    assert_equal [@post], e.requests
+  end
+
   def test_pipeline_retry_fail
     @error_socket = ErrorBuffer.new Errno::ECONNRESET
     @error_socket.read_io.write http_response('Worked 1!')
@@ -324,10 +342,6 @@ def start
       @socket = @error_socket2
     end
 
-    def finish
-      @socket.close
-    end
-
     requests = [@get1, @get2, @get3]
 
     e = assert_raises Net::HTTP::Pipeline::ResponseError do
@@ -341,7 +355,7 @@ def finish
     assert @error_socket.closed?
 
     assert_equal http_get * 2, @error_socket2.write_io.read
-    refute @error_socket2.closed?
+    assert @error_socket2.closed?
 
     responses = e.responses
     assert_equal 'Worked 1!', responses.shift.body
@@ -490,6 +504,8 @@ def test_pipeline_receive
 
     @socket.finish
 
+    refute @socket.closed?
+
     assert_equal 'Worked 1!', responses.first.body
     assert_equal 'Worked 2!', responses.last.body
 
@@ -510,6 +526,8 @@ def test_pipeline_receive_bad_response
 
     @socket.finish
 
+    assert @socket.closed?
+
     assert_equal [@get2], e.requests
     assert_equal 1, e.responses.length
     assert_equal 'Worked 1!', e.responses.first.body
@@ -531,6 +549,8 @@ def test_pipeline_receive_timeout
 
     @socket.finish
 
+    assert @socket.closed?
+
     assert_equal [@get2], e.requests
     assert_equal 1, e.responses.length
     assert_equal 'Worked 1!', e.responses.first.body