Downloads::File#http_get_streaming: clean up retry logic.

Replace handrolled retry logic with retriable gem (already pulled in by another gem).

Gemfile

@@ -50,6 +50,7 @@ gem 'activemodel-serializers-xml'
 gem 'ptools'
 gem 'jquery-rails'
 gem 'webpacker', '>= 4.0.x'
+gem 'retriable'
 
 # needed for looser jpeg header compat
 gem 'ruby-imagespec', :require => "image_spec", :git => "https://github.com/r888888888/ruby-imagespec.git", :branch => "exif-fixes"

Gemfile.lock

@@ -489,6 +489,7 @@ DEPENDENCIES
   rakismet
   recaptcha
   responders
+  retriable
   ruby-imagespec!
   ruby-prof
   ruby-vips

app/logical/downloads/file.rb

@@ -2,6 +2,8 @@ module Downloads
   class File
     class Error < Exception ; end
 
+    RETRIABLE_ERRORS = [Errno::ECONNRESET, Errno::ETIMEDOUT, Errno::EIO, Errno::EHOSTUNREACH, Errno::ECONNREFUSED, Timeout::Error, IOError]
+
     attr_reader :data, :options
     attr_accessor :source, :referer
 
@@ -52,16 +54,15 @@ module Downloads
       end
     end
 
-    def download!
+    def download!(tries: 3)
       strategy = Sources::Strategies.find(source, referer)
       output_file = Tempfile.new(binmode: true)
+      url = self.class.uncached_url(strategy.file_url, strategy.headers)
       @data = strategy.data
 
-      http_get_streaming(
+      Retriable.retriable(on: RETRIABLE_ERRORS, tries: tries, base_interval: 0) do
-        self.class.uncached_url(strategy.file_url, strategy.headers), 
+        http_get_streaming(url, output_file, strategy.headers)
-        output_file, 
+      end
-        strategy.headers
-      )
 
       [output_file, strategy]
     end
@@ -74,17 +75,14 @@ module Downloads
     end
 
     def http_get_streaming(src, file, headers = {}, max_size: Danbooru.config.max_file_size)
-      tries = 0
       url = URI.parse(src)
 
-      while true
       unless url.is_a?(URI::HTTP) || url.is_a?(URI::HTTPS)
         raise Error.new("URL must be HTTP or HTTPS")
       end
 
       validate_local_hosts(url)
 
-        begin
       size = 0
       options = { stream_body: true, timeout: 10, headers: headers }
 
@@ -101,15 +99,6 @@ module Downloads
       else
         raise Error.new("HTTP error code: #{res.code} #{res.message}")
       end
-        rescue Errno::ECONNRESET, Errno::ETIMEDOUT, Errno::EIO, Errno::EHOSTUNREACH, Errno::ECONNREFUSED, Timeout::Error, IOError => x
-          tries += 1
-          if tries < 3
-            retry
-          else
-            raise
-          end
-        end
-      end # while
     end # def
   end
 end

test/unit/downloads/file_test.rb

@@ -10,14 +10,9 @@ module Downloads
       end
 
       context "that fails" do
-        setup do
+        should "retry three times before giving up" do
-          HTTParty.stubs(:get).raises(Errno::ETIMEDOUT)
+          HTTParty.expects(:get).times(3).raises(Errno::ETIMEDOUT)
-        end
+          assert_raises(Errno::ETIMEDOUT) { @download.download! }
-
-        should "retry three times" do
-          assert_raises(Errno::ETIMEDOUT) do
-            @download.http_get_streaming(@source, @tempfile)
-          end
         end
       end