From c403c3695b1943882bf88afa9caf55bd8c6acc2f Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Tue, 11 May 2021 21:19:22 +0900 Subject: [PATCH] Fix to be able to redownload avatar and header (#16190) * Fix to reset if header and avatar download fails * Add RedownloadAvatarWorker and RedownloadHeaderWorker --- app/models/concerns/account_avatar.rb | 2 +- app/models/concerns/account_header.rb | 2 +- app/models/concerns/remotable.rb | 2 ++ .../activitypub/process_account_service.rb | 12 ++++++-- app/workers/redownload_avatar_worker.rb | 29 +++++++++++++++++++ app/workers/redownload_header_worker.rb | 29 +++++++++++++++++++ 6 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 app/workers/redownload_avatar_worker.rb create mode 100644 app/workers/redownload_header_worker.rb diff --git a/app/models/concerns/account_avatar.rb b/app/models/concerns/account_avatar.rb index 2d5ebfca3..1af53ed23 100644 --- a/app/models/concerns/account_avatar.rb +++ b/app/models/concerns/account_avatar.rb @@ -21,7 +21,7 @@ module AccountAvatar has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '-strip' }, processors: [:lazy_thumbnail] validates_attachment_content_type :avatar, content_type: IMAGE_MIME_TYPES validates_attachment_size :avatar, less_than: LIMIT - remotable_attachment :avatar, LIMIT + remotable_attachment :avatar, LIMIT, suppress_errors: false end def avatar_original_url diff --git a/app/models/concerns/account_header.rb b/app/models/concerns/account_header.rb index 067e166eb..72a3d0566 100644 --- a/app/models/concerns/account_header.rb +++ b/app/models/concerns/account_header.rb @@ -22,7 +22,7 @@ module AccountHeader has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '-strip' }, processors: [:lazy_thumbnail] validates_attachment_content_type :header, content_type: IMAGE_MIME_TYPES validates_attachment_size :header, less_than: LIMIT - remotable_attachment :header, LIMIT + remotable_attachment :header, LIMIT, suppress_errors: false end def header_original_url diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb index 56b9c0164..ffe8a7565 100644 --- a/app/models/concerns/remotable.rb +++ b/app/models/concerns/remotable.rb @@ -28,9 +28,11 @@ module Remotable end rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" + public_send("#{attachment_name}=", nil) if public_send("#{attachment_name}_file_name").present? raise e unless suppress_errors rescue Paperclip::Errors::NotIdentifiedByImageMagickError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Paperclip::Error, Mastodon::DimensionsValidationError, Mastodon::StreamValidationError => e Rails.logger.debug "Error fetching remote #{attachment_name}: #{e}" + public_send("#{attachment_name}=", nil) if public_send("#{attachment_name}_file_name").present? end nil diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index bb2e8f665..7e268f4d4 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -106,8 +106,16 @@ class ActivityPub::ProcessAccountService < BaseService end def set_fetchable_attributes! - @account.avatar_remote_url = image_url('icon') || '' unless skip_download? - @account.header_remote_url = image_url('image') || '' unless skip_download? + begin + @account.avatar_remote_url = image_url('icon') || '' unless skip_download? + rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError + RedownloadAvatarWorker.perform_in(rand(30..600).seconds, @account.id) + end + begin + @account.header_remote_url = image_url('image') || '' unless skip_download? + rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError + RedownloadHeaderWorker.perform_in(rand(30..600).seconds, @account.id) + end @account.statuses_count = outbox_total_items if outbox_total_items.present? @account.following_count = following_total_items if following_total_items.present? @account.followers_count = followers_total_items if followers_total_items.present? diff --git a/app/workers/redownload_avatar_worker.rb b/app/workers/redownload_avatar_worker.rb new file mode 100644 index 000000000..df17b7718 --- /dev/null +++ b/app/workers/redownload_avatar_worker.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class RedownloadAvatarWorker + include Sidekiq::Worker + include ExponentialBackoff + include JsonLdHelper + + sidekiq_options queue: 'pull', retry: 7 + + def perform(id) + account = Account.find(id) + + return if account.suspended? || DomainBlock.rule_for(account.domain)&.reject_media? + return if account.avatar_remote_url.blank? || account.avatar_file_name.present? + + account.reset_avatar! + account.save! + rescue ActiveRecord::RecordNotFound + # Do nothing + rescue Mastodon::UnexpectedResponseError => e + response = e.response + + if response_error_unsalvageable?(response) + # Give up + else + raise e + end + end +end diff --git a/app/workers/redownload_header_worker.rb b/app/workers/redownload_header_worker.rb new file mode 100644 index 000000000..3b142ec5f --- /dev/null +++ b/app/workers/redownload_header_worker.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class RedownloadHeaderWorker + include Sidekiq::Worker + include ExponentialBackoff + include JsonLdHelper + + sidekiq_options queue: 'pull', retry: 7 + + def perform(id) + account = Account.find(id) + + return if account.suspended? || DomainBlock.rule_for(account.domain)&.reject_media? + return if account.header_remote_url.blank? || account.header_file_name.present? + + account.reset_header! + account.save! + rescue ActiveRecord::RecordNotFound + # Do nothing + rescue Mastodon::UnexpectedResponseError => e + response = e.response + + if response_error_unsalvageable?(response) + # Give up + else + raise e + end + end +end