FIX: Improve error handling for calculate_dominant_color! (#18503)

These errors tend to indicate that the upload is missing on the remote store. This is bad, but we don't want it to block the dominant-color calculation process. This commit catches errors when there is an HTTP error, and fixes the `base_store.rb` implementation when `FileHelper.download` returns nil.

app/models/upload.rb

@@ -373,6 +373,10 @@ class Upload < ActiveRecord::Base
         raise "Calculated dominant color but unable to parse output:\n#{data}" if color.nil?
 
         color
+      rescue OpenURI::HTTPError => e
+        # Some issue with downloading the image from a remote store.
+        # Assume the upload is broken and save an empty string to prevent re-evaluation
+        ""
       rescue Discourse::Utils::CommandError => e
         # Timeout or unable to parse image
         # This can happen due to bad user input - ignore and save

lib/file_store/base_store.rb

@@ -116,6 +116,8 @@ module FileStore
             follow_redirect: true
           )
 
+          return nil if file.nil?
+
           cache_file(file, filename)
           file = get_from_cache(filename)
         end

spec/models/upload_spec.rb

@@ -703,7 +703,7 @@ RSpec.describe Upload do
       expect(invalid_image.dominant_color).to eq(nil)
     end
 
-    it "correctly handles download failures" do
+    it "correctly handles error when file is too large to download" do
       white_image.stubs(:local?).returns(true)
       Discourse.store.stubs(:download).returns(nil)
 
@@ -712,6 +712,15 @@ RSpec.describe Upload do
       expect(invalid_image.dominant_color).to eq("")
     end
 
+    it "correctly handles error when file has HTTP error" do
+      white_image.stubs(:local?).returns(true)
+      Discourse.store.stubs(:download).raises(OpenURI::HTTPError)
+
+      expect(invalid_image.dominant_color).to eq(nil)
+      expect(invalid_image.dominant_color(calculate_if_missing: true)).to eq("")
+      expect(invalid_image.dominant_color).to eq("")
+    end
+
     it "is validated for length" do
       u = Fabricate(:upload)