Merge pull request #2477 from basecamp/fix-crash-during-export

Fix crash during export

app/jobs/account/data_import_job.rb

@@ -2,7 +2,7 @@ class Account::DataImportJob < ApplicationJob
   include ActiveJob::Continuable
 
   queue_as :backend
-  discard_on Account::DataTransfer::RecordSet::IntegrityError
+  discard_on Account::DataTransfer::RecordSet::IntegrityError, ZipFile::InvalidFileError
 
   def perform(import)
     step :check do |step|

app/models/account/data_transfer/action_text_rich_text_record_set.rb

@@ -57,11 +57,17 @@ class Account::DataTransfer::ActionTextRichTextRecordSet < Account::DataTransfer
 
       content.send(:attachment_nodes).each do |node|
         sgid = SignedGlobalID.parse(node["sgid"], for: ActionText::Attachable::LOCATOR_NAME)
-        record = sgid&.find
-        next if record&.account_id != account.id
 
-        node["gid"] = record.to_global_id.to_s
-        node.remove_attribute("sgid")
+        record = begin
+          sgid&.find
+        rescue ActiveRecord::RecordNotFound
+          nil
+        end
+
+        if record&.account_id == account.id
+          node["gid"] = record.to_global_id.to_s
+          node.remove_attribute("sgid")
+        end
       end
 
       content.fragment.source.to_html
@@ -74,11 +80,12 @@ class Account::DataTransfer::ActionTextRichTextRecordSet < Account::DataTransfer
 
       fragment.css("action-text-attachment[gid]").each do |node|
         gid = GlobalID.parse(node["gid"])
-        next unless gid
 
-        record = gid.find
-        node["sgid"] = record.attachable_sgid
-        node.remove_attribute("gid")
+        if gid
+          record = gid.find
+          node["sgid"] = record.attachable_sgid
+          node.remove_attribute("gid")
+        end
       end
 
       fragment.to_html

app/models/account/data_transfer/blob_file_record_set.rb

@@ -9,11 +9,11 @@ class Account::DataTransfer::BlobFileRecordSet < Account::DataTransfer::RecordSe
     end
 
     def export_record(blob)
-      zip.add_file("storage/#{blob.key}", compress: false) do |out|
-        blob.download { |chunk| out.write(chunk) }
+      if blob.service.exist?(blob.key)
+        zip.add_file("storage/#{blob.key}", compress: false) do |out|
+          blob.download { |chunk| out.write(chunk) }
+        end
       end
-    rescue ActiveStorage::FileNotFoundError
-      # Skip blobs where the file is missing from storage
     end
 
     def files

app/models/zip_file.rb

@@ -1,4 +1,6 @@
 class ZipFile
+  class InvalidFileError < StandardError; end
+
   class << self
     def create_for(attachment, filename:)
       raise ArgumentError, "No block given" unless block_given?
@@ -57,6 +59,12 @@ class ZipFile
 
         blob.update!(byte_size: writer.byte_size, checksum: writer.checksum)
         attachment.attach(blob)
+      rescue Aws::S3::MultipartUploadError => e
+        if e.errors.any?
+          raise e.errors.first
+        else
+          raise e
+        end
       end
 
       def create_for_disk(attachment, filename:)

app/models/zip_file/reader.rb

@@ -2,6 +2,8 @@ class ZipFile::Reader
   def initialize(io)
     @io = io
     @reader = ZipKit::FileReader.read_zip_structure(io: io)
+  rescue ZipKit::FileReader::InvalidStructure => e
+    raise ZipFile::InvalidFileError, e.message
   end
 
   def read(file_path)

test/models/account/export_test.rb

@@ -59,4 +59,21 @@ class Account::ExportTest < ActiveSupport::TestCase
       assert entry, "Expected blob file in zip"
     end
   end
+
+  test "build succeeds when rich text references missing blob" do
+    blob = ActiveStorage::Blob.create_and_upload!(
+      io: file_fixture("moon.jpg").open,
+      filename: "moon.jpg",
+      content_type: "image/jpeg"
+    )
+    card = cards(:logo)
+    card.update!(description: "<action-text-attachment sgid=\"#{blob.attachable_sgid}\"></action-text-attachment>")
+    ActiveStorage::Blob.where(id: blob.id).delete_all
+
+    export = Account::Export.create!(account: Current.account, user: users(:david))
+
+    export.build
+
+    assert export.completed?
+  end
 end