Merge branch 'sh-geo-fix-file-download-errors' into 'master'

Geo: Properly check return value of file download return codes

Closes #3749

See merge request gitlab-org/gitlab-ee!3143

app/services/geo/file_download_service.rb

@@ -6,7 +6,7 @@ module Geo
       try_obtain_lease do |lease|
         start_time = Time.now
         bytes_downloaded = downloader.execute
-        success = bytes_downloaded && bytes_downloaded >= 0
+        success = (bytes_downloaded.present? && bytes_downloaded >= 0)
         log_info("File download",
                  success: success,
                  bytes_downloaded: bytes_downloaded,

spec/services/geo/file_download_service_spec.rb

@@ -30,6 +30,12 @@ describe Geo::FileDownloadService do
 
         expect { execute! }.to change { Geo::FileRegistry.failed.count }.by(1)
       end
+
+      it 'registers when the download fails with some other error' do
+        stub_transfer(Gitlab::Geo::FileTransfer, nil)
+
+        expect { execute! }.to change { Geo::FileRegistry.failed.count }.by(1)
+      end
     end
 
     context 'group avatar' do

spec/workers/geo/file_download_dispatch_worker_spec.rb

@@ -62,6 +62,7 @@ describe Geo::FileDownloadDispatchWorker, :postgresql do
     it 'attempts to load a new batch without pending downloads' do
       stub_const('Geo::BaseSchedulerWorker::DB_RETRIEVE_BATCH_SIZE', 5)
       stub_const('Geo::BaseSchedulerWorker::MAX_CAPACITY', 2)
+      allow_any_instance_of(::Gitlab::Geo::Transfer).to receive(:download_from_primary).and_return(100)
 
       avatar = fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'))
       create_list(:lfs_object, 2, :with_file)