Merge pull request #131 from Shopify/handle-timeout-due-to-starvation

Handle timeout due to pool exhaustion

Author: tenderlove

History.txt

@@ -1,3 +1,9 @@
+=== 4.0.6 / 2024-12-04
+
+Bug fixes:
+
+* Allow ConnectionPool exceptions from checkout to bubble up to caller.
+
 === 4.0.5 / 2024-12-04
 
 Bug fixes:

lib/net/http/persistent.rb

@@ -181,7 +181,7 @@ class Net::HTTP::Persistent
   ##
   # The version of Net::HTTP::Persistent you are using
 
-  VERSION = '4.0.5'
+  VERSION = '4.0.6'
 
   ##
   # Error class for errors raised by Net::HTTP::Persistent.  Various
@@ -630,47 +630,49 @@ def connection_for uri
 
     connection = @pool.checkout net_http_args
 
-    http = connection.http
+    begin
+      http = connection.http
 
-    connection.ressl @ssl_generation if
-      connection.ssl_generation != @ssl_generation
+      connection.ressl @ssl_generation if
+        connection.ssl_generation != @ssl_generation
 
-    if not http.started? then
-      ssl   http if use_ssl
-      start http
-    elsif expired? connection then
-      reset connection
-    end
+      if not http.started? then
+        ssl   http if use_ssl
+        start http
+      elsif expired? connection then
+        reset connection
+      end
 
-    http.keep_alive_timeout = @idle_timeout  if @idle_timeout
-    http.max_retries        = @max_retries   if http.respond_to?(:max_retries=)
-    http.read_timeout       = @read_timeout  if @read_timeout
-    http.write_timeout      = @write_timeout if
-      @write_timeout && http.respond_to?(:write_timeout=)
+      http.keep_alive_timeout = @idle_timeout  if @idle_timeout
+      http.max_retries        = @max_retries   if http.respond_to?(:max_retries=)
+      http.read_timeout       = @read_timeout  if @read_timeout
+      http.write_timeout      = @write_timeout if
+        @write_timeout && http.respond_to?(:write_timeout=)
+
+      return yield connection
+    rescue Errno::ECONNREFUSED
+      if http.proxy?
+        address = http.proxy_address
+        port    = http.proxy_port
+      else
+        address = http.address
+        port    = http.port
+      end
 
-    return yield connection
-  rescue Errno::ECONNREFUSED
-    if http.proxy?
-      address = http.proxy_address
-      port    = http.proxy_port
-    else
-      address = http.address
-      port    = http.port
-    end
+      raise Error, "connection refused: #{address}:#{port}"
+    rescue Errno::EHOSTDOWN
+      if http.proxy?
+        address = http.proxy_address
+        port    = http.proxy_port
+      else
+        address = http.address
+        port    = http.port
+      end
 
-    raise Error, "connection refused: #{address}:#{port}"
-  rescue Errno::EHOSTDOWN
-    if http.proxy?
-      address = http.proxy_address
-      port    = http.proxy_port
-    else
-      address = http.address
-      port    = http.port
+      raise Error, "host down: #{address}:#{port}"
+    ensure
+      @pool.checkin net_http_args
     end
-
-    raise Error, "host down: #{address}:#{port}"
-  ensure
-    @pool.checkin net_http_args
   end
 
   ##

test/test_net_http_persistent.rb

@@ -280,6 +280,16 @@ def test_connection_for
     assert_same used, stored
   end
 
+  def test_connection_for_exhaustion
+    @http = Net::HTTP::Persistent.new pool_size: 0
+
+    assert_raises Timeout::Error do
+      @http.connection_for @uri do |c|
+        assert_same nil, c
+      end
+    end
+  end
+
   def test_connection_for_cached
     cached = basic_connection
     cached.http.start