diff --git a/app/services/bulk_imports/file_download_service.rb b/app/services/bulk_imports/file_download_service.rb index 321e6b3783e01eed23867610e52a0b08c293c590..8f72c35a94cd176e65aa3c6eac124e261b7d7bb6 100644 --- a/app/services/bulk_imports/file_download_service.rb +++ b/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) diff --git a/lib/bulk_imports/network_error.rb b/lib/bulk_imports/network_error.rb index b21889adcb3e67e712361ccec2dbca76d7442e30..b49733962f4bec827b52ab74d1683784340e4035 100644 --- a/lib/bulk_imports/network_error.rb +++ b/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) diff --git a/spec/lib/bulk_imports/pipeline/runner_spec.rb b/spec/lib/bulk_imports/pipeline/runner_spec.rb index 8c1a6a5b6c82153282d556fd792b9ad73440424d..a88e8fb50d3f159a34a84f1461caa997e4a56c4a 100644 --- a/spec/lib/bulk_imports/pipeline/runner_spec.rb +++ b/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 diff --git a/spec/services/bulk_imports/file_download_service_spec.rb b/spec/services/bulk_imports/file_download_service_spec.rb index accaa31dd50623cc6535c397c3709e5751b21823..0d9fe96bb640c217625b84c2c50b676e28cb566f 100644 --- a/spec/services/bulk_imports/file_download_service_spec.rb +++ b/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 }