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.
This commit is contained in:
@@ -94,7 +94,7 @@ module Downloads
|
|||||||
else
|
else
|
||||||
raise Error.new("HTTP error code: #{res.code} #{res.message}")
|
raise Error.new("HTTP error code: #{res.code} #{res.message}")
|
||||||
end
|
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
|
tries += 1
|
||||||
if tries < 3
|
if tries < 3
|
||||||
retry
|
retry
|
||||||
|
|||||||
@@ -165,25 +165,11 @@ class Upload < ApplicationRecord
|
|||||||
end
|
end
|
||||||
|
|
||||||
def process!(force = false)
|
def process!(force = false)
|
||||||
@tries ||= 0
|
|
||||||
return if !force && status =~ /processing|completed|error/
|
|
||||||
|
|
||||||
process_upload
|
process_upload
|
||||||
post = create_post_from_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
|
rescue Exception => x
|
||||||
update_attributes(:status => "error: #{x.class} - #{x.message}", :backtrace => x.backtrace.join("\n"))
|
update_attributes(:status => "error: #{x.class} - #{x.message}", :backtrace => x.backtrace.join("\n"))
|
||||||
nil
|
nil
|
||||||
|
|
||||||
ensure
|
ensure
|
||||||
file.try(:close!)
|
file.try(:close!)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -305,5 +305,15 @@ class UploadTest < ActiveSupport::TestCase
|
|||||||
assert_nothing_raised {@upload.process!}
|
assert_nothing_raised {@upload.process!}
|
||||||
end
|
end
|
||||||
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
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user