♻️ Re-arrange method structure in hopes of not swallowing exception

We are observing that this job is raising an exception, which is then
swallowed (and "pooped out" to the mailboxer notifications for the
submitting user; which is not helpful when tracking down the
particulars).

Confounding part is that the reported errors, don't seem to be affecting
the attachment of files.  In other words, we get a notification of
failure but it worked (the majority of the time).

Co-authored-by: LaRita Robinson <[email protected]>

app/jobs/import_url_job_decorator.rb

@@ -1,28 +1,48 @@
 # frozen_string_literal: true
 
 # OVERRIDE in v2.9.6 of Hyrax
+# <2023-11-22 Wed> Aim is to hopefully not swallow exceptions, and get meaningful info regarding
+#                  the observed problem.
 module ImportUrlJobDecorator
-  # OVERRIDE to gain further insight into the StandardError that was reported but hidden.
-  def copy_remote_file(uri, name, headers = {})
+  def perform_af
+    name = file_set.label
+
+    copy_remote_file(name) do |f|
+      # reload the FileSet once the data is copied since this is a long running task
+      file_set.reload
+
+      # FileSetActor operates synchronously so that this tempfile is available.
+      # If asynchronous, the job might be invoked on a machine that did not have this temp file on its file system!
+      # NOTE: The return status may be successful even if the content never attaches.
+      log_import_status(f)
+    end
+  rescue => e
+    message = "ImportUrlJob: #{file_set.class} ID=#{file_set&.id} to_param=#{file_set&.to_param} with " \
+              "Parent #{file_set&.parent&.inspect} ID=#{file_set.parent&.id&.inspect} had error on URI #{uri.inspect}." \
+              "Error: #{e.class}.  Message: #{e.message}.  Backtrace:\n#{e.backtrace.join("\n")}"
+    Raven.capture_exception(e)
+    Hyrax.logger.error(message)
+    # Does send_error swallow the exception?
+    send_error(message)
+    raise e
+  end
+
+  # Download file from uri, yields a block with a file in a temporary directory.
+  # It is important that the file on disk has the same file name as the URL,
+  # because when the file in added into Fedora the file name will get persisted in the
+  # metadata.
+  # @param name [String] the human-readable name of the file
+  # @yield [IO] the stream to write to
+  def copy_remote_file(name)
     filename = File.basename(name)
     dir = Dir.mktmpdir
-    Rails.logger.debug("ImportUrlJob: Copying <#{uri}> to #{dir}")
+    Hyrax.logger.debug("ImportUrlJob: Copying <#{uri}> to #{dir}")
 
     File.open(File.join(dir, filename), 'wb') do |f|
-      begin
-        write_file(uri, f, headers)
-        yield f
-      rescue StandardError => e
-        # OVERRIDE adding Rails.logger.error call
-        Rails.logger.error(
-          %(ImportUrlJob: Error copying <#{uri}> to #{dir} with #{e.message}.  #{e.backtrace.join("\n")})
-        )
-        send_error(e.message)
-        # TODO: Should we re-raise the exception?  As written this copy_remote_file has a false
-        # success.
-      end
+      write_file(f)
+      yield f
+      Hyrax.logger.debug("ImportUrlJob: Closing #{File.join(dir, filename)}")
     end
-    Rails.logger.debug("ImportUrlJob: Copying <#{uri}> to #{dir}, closing #{File.join(dir, filename)}")
   end
 
   # OVERRIDE there are calls to send_error that send two arguments.