Fix webfinger retries (#4275)

* Do not raise unretryable exceptions in ResolveRemoteAccountService

* Removed fatal exceptions from ResolveRemoteAccountService

Exceptions that cannot be retried should not be raised. New exception
class for those that can be retried (Mastodon::UnexpectedResponseError)

6 files changed, 18 insertions, 23 deletions

diff --git a/app/services/fetch_remote_account_service.rb b/app/services/fetch_remote_account_service.rb
index 1efac365b..8eed0d454 100644
--- a/app/services/fetch_remote_account_service.rb
+++ b/app/services/fetch_remote_account_service.rb
@@ -32,8 +32,5 @@ class FetchRemoteAccountService < BaseService
   rescue Nokogiri::XML::XPath::SyntaxError
     Rails.logger.debug 'Invalid XML or missing namespace'
     nil
-  rescue Goldfinger::NotFoundError, Goldfinger::Error
-    Rails.logger.debug 'Exceptions related to Goldfinger occurs'
-    nil
   end
 end
diff --git a/app/services/fetch_remote_status_service.rb b/app/services/fetch_remote_status_service.rb
index 6ac31e4d8..b9f5f97b1 100644
--- a/app/services/fetch_remote_status_service.rb
+++ b/app/services/fetch_remote_status_service.rb
@@ -33,9 +33,6 @@ class FetchRemoteStatusService < BaseService
   rescue Nokogiri::XML::XPath::SyntaxError
     Rails.logger.debug 'Invalid XML or missing namespace'
     nil
-  rescue Goldfinger::NotFoundError, Goldfinger::Error
-    Rails.logger.debug 'Exceptions related to Goldfinger occurs'
-    nil
   end
 
   def confirmed_domain?(domain, account)
diff --git a/app/services/process_interaction_service.rb b/app/services/process_interaction_service.rb
index 584a109ad..cc99cde03 100644
--- a/app/services/process_interaction_service.rb
+++ b/app/services/process_interaction_service.rb
@@ -47,7 +47,7 @@ class ProcessInteractionService < BaseService
         reflect_unblock!(account, target_account)
       end
     end
-  rescue Goldfinger::Error, HTTP::Error, OStatus2::BadSalmonError, Mastodon::NotPermittedError
+  rescue HTTP::Error, OStatus2::BadSalmonError, Mastodon::NotPermittedError
     nil
   end
 
diff --git a/app/services/resolve_remote_account_service.rb b/app/services/resolve_remote_account_service.rb
index c948243ec..e0e2ebc83 100644
--- a/app/services/resolve_remote_account_service.rb
+++ b/app/services/resolve_remote_account_service.rb
@@ -23,18 +23,19 @@ class ResolveRemoteAccountService < BaseService
 
     @webfinger = Goldfinger.finger("acct:#{uri}")
 
-    raise Goldfinger::Error, 'Missing resource links' if links_missing?
-
     confirmed_username, confirmed_domain = @webfinger.subject.gsub(/\Aacct:/, '').split('@')
 
     if confirmed_username.casecmp(@username).zero? && confirmed_domain.casecmp(@domain).zero?
       @username = confirmed_username
       @domain   = confirmed_domain
+    elsif redirected.nil?
+      return call("#{confirmed_username}@#{confirmed_domain}", update_profile, true)
     else
-      return call("#{confirmed_username}@#{confirmed_domain}", update_profile, true) if redirected.nil?
-      raise Goldfinger::Error, 'Requested and returned acct URIs do not match'
+      Rails.logger.debug 'Requested and returned acct URIs do not match'
+      return
     end
 
+    return if links_missing?
     return Account.find_local(@username) if TagManager.instance.local_domain?(@domain)
 
     RedisLock.acquire(lock_options) do |lock|
@@ -49,6 +50,9 @@ class ResolveRemoteAccountService < BaseService
     end
 
     @account
+  rescue Goldfinger::Error => e
+    Rails.logger.debug "Webfinger query for #{uri} unsuccessful: #{e}"
+    nil
   end
 
   private
@@ -57,7 +61,9 @@ class ResolveRemoteAccountService < BaseService
     @webfinger.link('http://schemas.google.com/g/2010#updates-from').nil? ||
       @webfinger.link('salmon').nil? ||
       @webfinger.link('http://webfinger.net/rel/profile-page').nil? ||
-      @webfinger.link('magic-public-key').nil?
+      @webfinger.link('magic-public-key').nil? ||
+      canonical_uri.nil? ||
+      hub_url.nil?
   end
 
   def webfinger_update_due?
@@ -123,19 +129,14 @@ class ResolveRemoteAccountService < BaseService
       author_uri = owner.at_xpath('./xmlns:uri') unless owner.nil?
     end
 
-    raise Goldfinger::Error, 'Author URI could not be found' if author_uri.nil?
-
-    @canonical_uri = author_uri.content
+    @canonical_uri = author_uri.nil? ? nil : author_uri.content
   end
 
   def hub_url
     return @hub_url if defined?(@hub_url)
 
-    hubs = atom.xpath('//xmlns:link[@rel="hub"]')
-
-    raise Goldfinger::Error, 'No PubSubHubbub hubs found' if hubs.empty? || hubs.first['href'].nil?
-
-    @hub_url = hubs.first['href']
+    hubs     = atom.xpath('//xmlns:link[@rel="hub"]')
+    @hub_url = hubs.empty? || hubs.first['href'].nil? ? nil : hubs.first['href']
   end
 
   def atom_body
@@ -143,7 +144,7 @@ class ResolveRemoteAccountService < BaseService
 
     response = Request.new(:get, atom_url).perform
 
-    raise Goldfinger::Error, "Feed attempt failed for #{atom_url}: HTTP #{response.code}" unless response.code == 200
+    raise Mastodon::UnexpectedResponseError, response unless response.code == 200
 
     @atom_body = response.to_s
   end
diff --git a/app/services/send_interaction_service.rb b/app/services/send_interaction_service.rb
index ab0d3aeed..c11813abc 100644
--- a/app/services/send_interaction_service.rb
+++ b/app/services/send_interaction_service.rb
@@ -14,7 +14,7 @@ class SendInteractionService < BaseService
 
     delivery = build_request.perform
 
-    raise "Delivery failed for #{target_account.salmon_url}: HTTP #{delivery.code}" unless delivery.code > 199 && delivery.code < 300
+    raise Mastodon::UnexpectedResponseError, delivery unless delivery.code > 199 && delivery.code < 300
   end
 
   private
diff --git a/app/services/subscribe_service.rb b/app/services/subscribe_service.rb
index c1c0a4c8b..d3e41e691 100644
--- a/app/services/subscribe_service.rb
+++ b/app/services/subscribe_service.rb
@@ -18,7 +18,7 @@ class SubscribeService < BaseService
     else
       # The response was either a 429 rate limit, or a 5xx error.
       # We need to retry at a later time. Fail loudly!
-      raise "Subscription attempt failed for #{@account.acct} (#{@account.hub_url}): HTTP #{@response.code}"
+      raise Mastodon::UnexpectedResponseError, @response
     end
   end