Skip to content

Commit

Permalink
fix(API): Catastrophic 500 when media cannot be processed (#976)
Browse files Browse the repository at this point in the history
* Red Tests

* fix(API): Explore shows infinite loading

When an otherwise representable media attachment is processed for a
preview or image and errors, the request fails catastrophically
resulting in the Explore interface looking like it's loading forever.

It was determined that this was due to the underlying processing failure
and, as such, the check for `.representable?` was insufficient to catch
errors.

For now, we simply added a test that raises the error manually for
invariable images. See https://api.rubyonrails.org/classes/ActiveStorage/Blob/Representable.html#method-i-variant

Our code currently catches any ActiveStorage::Error to reduce friction
points since the Explore code has guardrails in place to display other
media when a preview thumbnail is unaavilable.

---------

Co-authored-by: Laura Mosher <[email protected]>
  • Loading branch information
lauramosher and lauramosher authored Oct 19, 2023
1 parent 0ddf326 commit d2d5b56
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 2 deletions.
2 changes: 1 addition & 1 deletion rails/app/models/media.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def blob_id
],
size: { less_than_or_equal_to: 200.megabytes }

delegate :content_type, :blob_id, :blob, :representable?, to: :media
delegate :attach, :content_type, :blob_id, :blob, :representable?, to: :media
end

# == Schema Information
Expand Down
2 changes: 2 additions & 0 deletions rails/app/models/story.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def media_preview_thumbnail
Rails.application.routes.url_helpers.rails_representation_url(
previewable_media.media.representation(resize_to_limit: [200, 200]).processed
)
rescue ActiveStorage::Error
nil
end

def self.export_sample_csv
Expand Down
6 changes: 6 additions & 0 deletions rails/spec/factories/media_factory.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FactoryBot.define do
factory :media do
story
media { Rack::Test::UploadedFile.new('spec/fixtures/media/terrastories.png', 'image/png') }
end
end
24 changes: 23 additions & 1 deletion rails/spec/models/story_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@
it "strips trailing whitespaces from Speaker_name and Place_name" do
described_class.import(file_fixture('story_with_trailing_whitespaces.csv'), community.id, mapped_headers)
story = described_class.last

expect(story.speakers.map(&:name)).to all(satisfy { |name| name == name.strip })
expect(story.places.map(&:name)).to all(satisfy { |name| name == name.strip })
end
Expand Down Expand Up @@ -242,4 +242,26 @@
expect(described_class.export_sample_csv).to eq("name,description,speakers,places,interview_location,date_interviewed,interviewer,language,media,permission_level\n")
end
end

describe '#media_preview_thumbnail' do
let(:story) { create(:story, :with_places, :with_speakers) }

it 'returns nil when story has no previewable media' do
expect(story.media_preview_thumbnail).to be_nil
end

it 'returns processed URL for media' do
media = create(:media, story: story)

expect(story.media_preview_thumbnail).to match(/^http(.*)\/terrastories.png$/)
end

it 'returns nil if media processing throws an error' do
create(:media, story: story)

expect_any_instance_of(ActiveStorage::Variant).to receive(:processed).and_raise(ActiveStorage::InvariableError)

expect(story.media_preview_thumbnail).to be_nil
end
end
end

0 comments on commit d2d5b56

Please sign in to comment.