Fixes: #3507@1.5h Retries & Error Handling in WorkbenchImportWorker

3 files changed, 52 insertions, 26 deletions

diff --git a/app/services/http_service.rb b/app/services/http_service.rb
index 37f1621d5..b01e11d6f 100644
--- a/app/services/http_service.rb
+++ b/app/services/http_service.rb
@@ -1,5 +1,7 @@
 module HTTPService extend self
 
+  Timeout = Faraday::TimeoutError
+
   def get_resource(host:, path:, token: nil, params: {}, parse_json: false) 
     Faraday.new(url: host) do |c|
       c.headers['Authorization'] = "Token token=#{token.inspect}" if token
diff --git a/app/workers/workbench_import_worker.rb b/app/workers/workbench_import_worker.rb
index 2973d97c5..b7668c976 100644
--- a/app/workers/workbench_import_worker.rb
+++ b/app/workers/workbench_import_worker.rb
@@ -3,16 +3,17 @@ class WorkbenchImportWorker
   include Rails.application.routes.url_helpers
   include Configurable
 
+  RETRY_DELAYS = [3, 5, 8]
 
   # Workers
   # =======
 
   def perform(import_id)
-    @import = Import.find(import_id)
-    @downloaded = nil
-    download
-    @zip_service = ZipService.new(@zipfile_data)
-    upload
+    @import     = Import.find(import_id)
+    @response   = nil
+    downloaded  = download
+    zip_service = ZipService.new(downloaded)
+    upload zip_service
   end
 
   def download
@@ -21,25 +22,10 @@ class WorkbenchImportWorker
       host: import_host,
       path: import_path,
       params: {token: @import.token_download})
-
-    # TODO: Delete me after stable implementation of #1726
-    # path = File.join(config.dir, import.name.gsub(%r{\s+}, '-'))
-    # unique_path = FileService.unique_filename path
-    # Dir.mkdir unique_path
-    # @downloaded = File.join(unique_path, import.name)
-    # File.open(downloaded, 'wb') do | file |
-    #   file.write zipfile_data
-    # end
-  end
-
-
-  def upload
-    @zip_service.entry_group_streams.each(&method(:upload_entry_group))
   end
 
-  def upload_entry_group key_pair
-    eg_name, eg_stream = key_pair
-    logger.warn  "HTTP POST #{export_url} (for #{complete_entry_group_name(eg_name)})"
+  def execute_post eg_name, eg_stream
+    logger.info  "HTTP POST #{export_url} (for #{complete_entry_group_name(eg_name)})"
     HTTPService.post_resource(
       host: export_host,
       path: export_path,
@@ -49,6 +35,39 @@ class WorkbenchImportWorker
       upload: {file: [eg_stream, 'application/zip', eg_name]})
   end
 
+  def log_failure reason, count
+    logger.info "HTTP POST failed with #{reason}, count = #{count}, response=#{@response}"
+  end
+
+  def try_again
+    raise RetryService::Retry
+  end
+
+  def try_upload_entry_group eg_name, eg_stream
+    result = execute_post eg_name, eg_stream
+    return result if result.status < 400
+    @response = result.body
+    try_again
+  end
+
+  def upload zip_service
+    zip_service.entry_group_streams.each(&method(:upload_entry_group))
+  end
+
+  def upload_entry_group key_pair
+    retry_service = RetryService.new(delay: RETRY_DELAYS, rescue_from: HTTPService::Timeout, &method(:log_failure)) 
+    retry_service.execute(&upload_entry_group_proc(key_pair))
+  end
+
+  def upload_entry_group_proc key_pair
+    eg_name, eg_stream = key_pair
+    # This should be fn.try_upload_entry_group(eg_name, eg_stream) ;(
+    -> do
+      try_upload_entry_group(eg_name, eg_stream)
+    end
+  end
+
+
 
   # Queries
   # =======
diff --git a/spec/services/retry_service_spec.rb b/spec/services/retry_service_spec.rb
index 0ab9ddef9..22957b565 100644
--- a/spec/services/retry_service_spec.rb
+++ b/spec/services/retry_service_spec.rb
@@ -96,19 +96,24 @@ RSpec.describe RetryService do
 
   context 'failure callback in constructor' do
     subject do
-      described_class.new(delays: [1]){ @failures += 1}
+      described_class.new(delays: [1, 2], &method(:add2failures))
     end
     before do
-      @failures = 0
+      @failures = []
       @count    = 0
       expect( subject ).to receive(:sleep).with(1)
+      expect( subject ).to receive(:sleep).with(2)
     end
     it 'succeeds the second time and calls the failure_callback once' do
-      subject.execute{ succeed_later(RetryService::Retry){ 42 } }
-      expect( @failures ).to eq(1)
+      subject.execute{ succeed_later(RetryService::Retry, count: 2){ 42 } }
+      expect( @failures ).to eq([1,2])
     end
   end
 
+  def add2failures( e, c)
+    @failures << c 
+  end
+
   def succeed_later error, count: 1, &blk
     return blk.() unless @count < count
     @count += 1