From c584ca5b19392a6072a24179d3e768f74619e54b Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sat, 5 May 2018 11:42:40 -0500
Subject: [PATCH] Fix uploads getting stuck in 'processing' state (fix #3659).

Bug: if an upload timed out while downloading the file, Upload#process!
would catch the error and attempt to retry, but since the upload was
already in the 'processing' state, on the second try `process!` would
bail out immediately and leave the upload stuck in the 'processing' state.

Fix: remove the retry logic from Upload#process!. Let Downloads::File#download!
(which had its own retry logic) handle it instead.
---
 app/logical/downloads/file.rb |  2 +-
 app/models/upload.rb          | 14 --------------
 test/unit/upload_test.rb      | 10 ++++++++++
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/app/logical/downloads/file.rb b/app/logical/downloads/file.rb
index ab1cf63ae..ca6250080 100644
--- a/app/logical/downloads/file.rb
+++ b/app/logical/downloads/file.rb
@@ -94,7 +94,7 @@ 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, IOError => x
+        rescue Errno::ECONNRESET, Errno::ETIMEDOUT, Errno::EIO, Errno::EHOSTUNREACH, Errno::ECONNREFUSED, Timeout::Error, IOError => x
           tries += 1
           if tries < 3
             retry
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 6a4a9a18b..d5dd087ae 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -165,25 +165,11 @@ class Upload < ApplicationRecord
     end
 
     def process!(force = false)
-      @tries ||= 0
-      return if !force && status =~ /processing|completed|error/
-
       process_upload
       post = create_post_from_upload
-
-    rescue Timeout::Error, Net::HTTP::Persistent::Error => x
-      if @tries > 3
-        update_attributes(:status => "error: #{x.class} - #{x.message}", :backtrace => x.backtrace.join("\n"))
-      else
-        @tries += 1
-        retry
-      end
-      nil
-
     rescue Exception => x
       update_attributes(:status => "error: #{x.class} - #{x.message}", :backtrace => x.backtrace.join("\n"))
       nil
-
     ensure
       file.try(:close!)
     end
diff --git a/test/unit/upload_test.rb b/test/unit/upload_test.rb
index 87dc77b75..b5e278ed1 100644
--- a/test/unit/upload_test.rb
+++ b/test/unit/upload_test.rb
@@ -305,5 +305,15 @@ class UploadTest < ActiveSupport::TestCase
         assert_nothing_raised {@upload.process!}
       end
     end
+
+    context "on timeout errors" do
+      should "leave the upload in an error state" do
+        HTTParty.stubs(:get).raises(Net::ReadTimeout)
+        @upload = FactoryBot.create(:source_upload)
+        @upload.process!
+
+        assert_match(/\Aerror/, @upload.status)
+      end
+    end
   end
 end