cleanup so gravatar download failures are consistent

previously we would ignore socket error, but this would mean that
there could be conditions where we would keep trying to download
gravatars forever (in an hourly job)
This commit is contained in:
Sam 2018-10-19 12:51:34 +11:00
parent 85ef8e5a9f
commit 9bfc939692
2 changed files with 6 additions and 9 deletions

View File

@ -13,7 +13,7 @@ class UserAvatar < ActiveRecord::Base
def update_gravatar! def update_gravatar!
DistributedMutex.synchronize("update_gravatar_#{user_id}") do DistributedMutex.synchronize("update_gravatar_#{user_id}") do
begin begin
self.update_columns(last_gravatar_download_attempt: Time.now) self.update!(last_gravatar_download_attempt: Time.now)
max = Discourse.avatar_sizes.max max = Discourse.avatar_sizes.max
email_hash = user_id == Discourse::SYSTEM_USER_ID ? User.email_hash("info@discourse.org") : user.email_hash email_hash = user_id == Discourse::SYSTEM_USER_ID ? User.email_hash("info@discourse.org") : user.email_hash
@ -47,17 +47,12 @@ class UserAvatar < ActiveRecord::Base
end end
gravatar_upload&.destroy! gravatar_upload&.destroy!
self.gravatar_upload = upload self.update!(gravatar_upload: upload)
save!
end end
end end
end end
rescue OpenURI::HTTPError
save!
rescue SocketError
# skip saving, we are not connected to the net
ensure ensure
tempfile.try(:close!) tempfile&.close!
end end
end end
end end

View File

@ -74,7 +74,9 @@ describe UserAvatar do
FileHelper.expects(:download).raises(SocketError) FileHelper.expects(:download).raises(SocketError)
expect { avatar.update_gravatar! }.to_not change { Upload.count } expect do
expect { avatar.update_gravatar! }.to raise_error(SocketError)
end.to_not change { Upload.count }
expect(avatar.last_gravatar_download_attempt).to eq(Time.now) expect(avatar.last_gravatar_download_attempt).to eq(Time.now)
end end