From 42947ec6f122dce654c954c2cd9bc1b4e9244b36 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 23 Sep 2022 12:42:07 +0100 Subject: [PATCH] FIX: Handle failed download when calculating image dominant color (#18342) This can happen when the upload size exceeds the maximum upload size, or there is a network issue during download --- app/models/upload.rb | 9 +++++++-- spec/models/upload_spec.rb | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index be5deaaa6bb..f218a9c741f 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -340,10 +340,15 @@ class Upload < ActiveRecord::Base if local? Discourse.store.path_for(self) else - Discourse.store.download(self).path + Discourse.store.download(self)&.path end - color = begin + if local_path.nil? + # Download failed. Could be too large to download, or file could be missing in s3 + color = "" + end + + color ||= begin data = Discourse::Utils.execute_command( "nice", "-n", diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 2ce566ed264..34862ff23ce 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -690,6 +690,15 @@ RSpec.describe Upload do expect(invalid_image.dominant_color).to eq(nil) end + it "correctly handles download failures" do + white_image.stubs(:local?).returns(true) + Discourse.store.stubs(:download).returns(nil) + + 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)