Retry exceptions during BulkImports::FileDownloadService

Changelog: other

app/services/bulk_imports/file_download_service.rb

@@ -58,6 +58,13 @@ def download_file
         http_client.stream(relative_url) do |chunk|
           next if bytes_downloaded == 0 && [301, 302, 303, 307, 308].include?(chunk.code)
 
+          if BulkImports::NetworkError::RETRIABLE_HTTP_CODES.include?(chunk.code)
+            raise BulkImports::NetworkError.new(
+              "Error downloading file from #{relative_url}. Error code: #{chunk.code}",
+              response: chunk.http_response
+            )
+          end
+
           @response_code = chunk.code
           @response_headers ||= Gitlab::HTTP::Response::Headers.new(chunk.http_response.to_hash)
           @last_chunk_context = chunk.to_s.truncate(LAST_CHUNK_CONTEXT_CHAR_LIMIT)

lib/bulk_imports/network_error.rb

@@ -18,7 +18,7 @@ class NetworkError < Error
       Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ENETUNREACH
     ].freeze
 
-    RETRIABLE_HTTP_CODES = [429].freeze
+    RETRIABLE_HTTP_CODES = [429, 500, 502, 503, 504].freeze
 
     DEFAULT_RETRY_DELAY_SECONDS = 30
 
@@ -57,7 +57,7 @@ def retriable_exception?
     end
 
     def retriable_http_code?
-      RETRIABLE_HTTP_CODES.include?(response&.code)
+      RETRIABLE_HTTP_CODES.include?(response&.code.to_i)
     end
 
     def increment(object)

spec/lib/bulk_imports/pipeline/runner_spec.rb

@@ -322,7 +322,7 @@ def load(context, data); end
         end
 
         context 'when exception is not retriable' do
-          let(:reponse_status_code) { 503 }
+          let(:reponse_status_code) { 505 }
 
           it_behaves_like 'failed pipeline', 'BulkImports::NetworkError', 'Net::ReadTimeout'
         end

spec/services/bulk_imports/file_download_service_spec.rb

@@ -149,15 +149,26 @@
     end
 
     context 'when chunk code is not 200' do
-      let(:chunk_code) { 500 }
+      let(:chunk_code) { 404 }
 
       it 'raises an error' do
         expect { subject.execute }.to raise_error(
           described_class::ServiceError,
-          'File download error 500'
+          'File download error 404'
         )
       end
 
+      context 'when chunk code is retriable' do
+        let(:chunk_code) { 502 }
+
+        it 'raises a retriable error' do
+          expect { subject.execute }.to raise_error(
+            BulkImports::NetworkError,
+            'Error downloading file from /test. Error code: 502'
+          )
+        end
+      end
+
       context 'when chunk code is redirection' do
         let(:chunk_code) { 303 }