Ignore error if mentioned account was not processable (#29215)
Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
		
					parent
					
						
							
								f91f077985
							
						
					
				
			
			
				commit
				
					
						66b2bc1c84
					
				
			
		
					 4 changed files with 122 additions and 0 deletions
				
			
		|  | @ -42,6 +42,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity | |||
|   def process_status | ||||
|     @tags                 = [] | ||||
|     @mentions             = [] | ||||
|     @unresolved_mentions  = [] | ||||
|     @silenced_account_ids = [] | ||||
|     @params               = {} | ||||
| 
 | ||||
|  | @ -55,6 +56,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity | |||
|     end | ||||
| 
 | ||||
|     resolve_thread(@status) | ||||
|     resolve_unresolved_mentions(@status) | ||||
|     fetch_replies(@status) | ||||
|     distribute | ||||
|     forward_for_reply | ||||
|  | @ -197,6 +199,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity | |||
|     return if account.nil? | ||||
| 
 | ||||
|     @mentions << Mention.new(account: account, silent: false) | ||||
|   rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError | ||||
|     @unresolved_mentions << tag['href'] | ||||
|   end | ||||
| 
 | ||||
|   def process_emoji(tag) | ||||
|  | @ -301,6 +305,12 @@ class ActivityPub::Activity::Create < ActivityPub::Activity | |||
|     ThreadResolveWorker.perform_async(status.id, in_reply_to_uri, { 'request_id' => @options[:request_id] }) | ||||
|   end | ||||
| 
 | ||||
|   def resolve_unresolved_mentions(status) | ||||
|     @unresolved_mentions.uniq.each do |uri| | ||||
|       MentionResolveWorker.perform_in(rand(30...600).seconds, status.id, uri, { 'request_id' => @options[:request_id] }) | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   def fetch_replies(status) | ||||
|     collection = @object['replies'] | ||||
|     return if collection.blank? | ||||
|  |  | |||
							
								
								
									
										37
									
								
								app/workers/mention_resolve_worker.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										37
									
								
								app/workers/mention_resolve_worker.rb
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,37 @@ | |||
| # frozen_string_literal: true | ||||
| 
 | ||||
| class MentionResolveWorker | ||||
|   include Sidekiq::Worker | ||||
|   include ExponentialBackoff | ||||
|   include JsonLdHelper | ||||
| 
 | ||||
|   sidekiq_options queue: 'pull', retry: 7 | ||||
| 
 | ||||
|   def perform(status_id, uri, options = {}) | ||||
|     status = Status.find_by(id: status_id) | ||||
|     return if status.nil? | ||||
| 
 | ||||
|     account = account_from_uri(uri) | ||||
|     account = ActivityPub::FetchRemoteAccountService.new.call(uri, request_id: options[:request_id]) if account.nil? | ||||
| 
 | ||||
|     return if account.nil? | ||||
| 
 | ||||
|     status.mentions.create!(account: account, silent: false) | ||||
|   rescue ActiveRecord::RecordNotFound | ||||
|     # Do nothing | ||||
|   rescue Mastodon::UnexpectedResponseError => e | ||||
|     response = e.response | ||||
| 
 | ||||
|     if response_error_unsalvageable?(response) | ||||
|       # Give up | ||||
|     else | ||||
|       raise e | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   private | ||||
| 
 | ||||
|   def account_from_uri(uri) | ||||
|     ActivityPub::TagManager.instance.uri_to_resource(uri, Account) | ||||
|   end | ||||
| end | ||||
|  | @ -63,6 +63,24 @@ RSpec.describe ActivityPub::Activity::Create do | |||
|       } | ||||
|     end | ||||
| 
 | ||||
|     let(:invalid_mention_json) do | ||||
|       { | ||||
|         id: [ActivityPub::TagManager.instance.uri_for(sender), 'post2'].join('/'), | ||||
|         type: 'Note', | ||||
|         to: [ | ||||
|           'https://www.w3.org/ns/activitystreams#Public', | ||||
|           ActivityPub::TagManager.instance.uri_for(follower), | ||||
|         ], | ||||
|         content: '@bob lorem ipsum', | ||||
|         published: 1.hour.ago.utc.iso8601, | ||||
|         updated: 1.hour.ago.utc.iso8601, | ||||
|         tag: { | ||||
|           type: 'Mention', | ||||
|           href: 'http://notexisting.dontexistingtld/actor', | ||||
|         }, | ||||
|       } | ||||
|     end | ||||
| 
 | ||||
|     def activity_for_object(json) | ||||
|       { | ||||
|         '@context': 'https://www.w3.org/ns/activitystreams', | ||||
|  | @ -117,6 +135,25 @@ RSpec.describe ActivityPub::Activity::Create do | |||
|       # Creates two notifications | ||||
|       expect(Notification.count).to eq 2 | ||||
|     end | ||||
| 
 | ||||
|     it 'ignores unprocessable mention', :aggregate_failures do | ||||
|       stub_request(:get, invalid_mention_json[:tag][:href]).to_raise(HTTP::ConnectionError) | ||||
|       # When receiving the post that contains an invalid mention… | ||||
|       described_class.new(activity_for_object(invalid_mention_json), sender, delivery: true).perform | ||||
| 
 | ||||
|       # NOTE: Refering explicitly to the workers is a bit awkward | ||||
|       DistributionWorker.drain | ||||
|       FeedInsertWorker.drain | ||||
| 
 | ||||
|       # …it creates a status | ||||
|       status = Status.find_by(uri: invalid_mention_json[:id]) | ||||
| 
 | ||||
|       # Check the process did not crash | ||||
|       expect(status.nil?).to be false | ||||
| 
 | ||||
|       # It has queued a mention resolve job | ||||
|       expect(MentionResolveWorker).to have_enqueued_sidekiq_job(status.id, invalid_mention_json[:tag][:href], anything) | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe '#perform' do | ||||
|  |  | |||
							
								
								
									
										38
									
								
								spec/workers/mention_resolve_worker_spec.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										38
									
								
								spec/workers/mention_resolve_worker_spec.rb
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,38 @@ | |||
| # frozen_string_literal: true | ||||
| 
 | ||||
| require 'rails_helper' | ||||
| 
 | ||||
| RSpec.describe MentionResolveWorker do | ||||
|   let(:status_id) { -42 } | ||||
|   let(:uri) { 'https://example.com/users/unknown' } | ||||
| 
 | ||||
|   describe '#perform' do | ||||
|     subject { described_class.new.perform(status_id, uri, {}) } | ||||
| 
 | ||||
|     context 'with a non-existent status' do | ||||
|       it 'returns nil' do | ||||
|         expect(subject).to be_nil | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'with a valid user' do | ||||
|       let(:status) { Fabricate(:status) } | ||||
|       let(:status_id) { status.id } | ||||
| 
 | ||||
|       let(:service_double) { instance_double(ActivityPub::FetchRemoteAccountService) } | ||||
| 
 | ||||
|       before do | ||||
|         allow(ActivityPub::FetchRemoteAccountService).to receive(:new).and_return(service_double) | ||||
| 
 | ||||
|         allow(service_double).to receive(:call).with(uri, anything) { Fabricate(:account, domain: 'example.com', uri: uri) } | ||||
|       end | ||||
| 
 | ||||
|       it 'resolves the account and adds a new mention', :aggregate_failures do | ||||
|         expect { subject } | ||||
|           .to change { status.reload.mentions }.from([]).to(a_collection_including(having_attributes(account: having_attributes(uri: uri), silent: false))) | ||||
| 
 | ||||
|         expect(service_double).to have_received(:call).once | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue