Merge pull request #7121 from betagouv/US/retry-once-when-call-to-script-fails

feat(ArchiveUploader.upload_with_chunking_wrapper): expect to retry call to wrapper once

app/services/archive_uploader.rb

@@ -48,11 +48,11 @@ class ArchiveUploader
     params =  blob_default_params(filepath).merge(byte_size: File.size(filepath),
                                                   checksum: Digest::SHA256.file(filepath).hexdigest)
     blob = ActiveStorage::Blob.create_before_direct_upload!(**params)
-    if syscall_to_custom_uploader(blob)
+    if retryable_syscall_to_custom_uploader(blob)
       return blob
     else
       blob.purge
-      fail "custom archive attachment failed, should it be retried ?"
+      fail "custom archive attachment failed twice, retry later"
     end
   end
 
@@ -73,6 +73,18 @@ class ArchiveUploader
     @namespaced_object_key ||= "archives/#{Date.today.strftime("%Y-%m-%d")}/#{SecureRandom.uuid}"
   end
 
+  def retryable_syscall_to_custom_uploader(blob)
+    limit_to_retry = 1
+    begin
+      syscall_to_custom_uploader(blob)
+    rescue
+      if limit_to_retry > 0
+        limit_to_retry = limit_to_retry - 1
+        retry
+      end
+    end
+  end
+
   def syscall_to_custom_uploader(blob)
     system(ENV.fetch('ACTIVE_STORAGE_BIG_FILE_UPLOADER_WITH_ENCRYPTION_PATH').to_s, filepath, blob.key, exception: true)
   end

spec/services/archive_uploader_spec.rb

@@ -57,14 +57,33 @@ describe ProcedureArchiveService do
     let(:fake_blob_bytesize) { 100.gigabytes }
 
     before do
-      expect(uploader).to receive(:syscall_to_custom_uploader).and_return(true)
       expect(File).to receive(:size).with(file.path).and_return(fake_blob_bytesize)
       expect(Digest::SHA256).to receive(:file).with(file.path).and_return(double(hexdigest: fake_blob_checksum.hexdigest))
     end
 
-    it 'creates a blob' do
+    context 'when it just works' do
-      expect { uploader.send(:upload_with_chunking_wrapper) }
+      it 'creates a blob' do
-        .to change { ActiveStorage::Blob.where(checksum: fake_blob_checksum.hexdigest, byte_size: fake_blob_bytesize).count }.by(1)
+        expect(uploader).to receive(:syscall_to_custom_uploader).and_return(true)
+        expect { uploader.send(:upload_with_chunking_wrapper) }
+          .to change { ActiveStorage::Blob.where(checksum: fake_blob_checksum.hexdigest, byte_size: fake_blob_bytesize).count }.by(1)
+      end
+    end
+
+    context 'when it fails once (DS proxy a bit flacky with archive ±>20Go, fails once, accept other call' do
+      it 'retries' do
+        expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).once.and_raise(StandardError, "BOOM")
+        expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).once.and_return(true)
+        expect { uploader.send(:upload_with_chunking_wrapper) }
+          .to change { ActiveStorage::Blob.where(checksum: fake_blob_checksum.hexdigest, byte_size: fake_blob_bytesize).count }.by(1)
+      end
+    end
+
+    context 'when it fails twice' do
+      it 'does not retry more than once' do
+        expect(uploader).to receive(:syscall_to_custom_uploader).with(anything).twice.and_raise(StandardError, "BOOM")
+        expect { uploader.send(:upload_with_chunking_wrapper) }
+          .to raise_error(RuntimeError, "custom archive attachment failed twice, retry later")
+      end
     end
   end
 end