Ignore error if mentioned account was not processable (#29215)
Co-authored-by: Claire <claire.github-309c@sitedethib.com>
This commit is contained in:
parent
c91e06bcad
commit
b8fdffe824
4 changed files with 122 additions and 0 deletions
|
@ -42,6 +42,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
|
||||||
def process_status
|
def process_status
|
||||||
@tags = []
|
@tags = []
|
||||||
@mentions = []
|
@mentions = []
|
||||||
|
@unresolved_mentions = []
|
||||||
@silenced_account_ids = []
|
@silenced_account_ids = []
|
||||||
@params = {}
|
@params = {}
|
||||||
|
|
||||||
|
@ -55,6 +56,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
|
||||||
end
|
end
|
||||||
|
|
||||||
resolve_thread(@status)
|
resolve_thread(@status)
|
||||||
|
resolve_unresolved_mentions(@status)
|
||||||
fetch_replies(@status)
|
fetch_replies(@status)
|
||||||
distribute
|
distribute
|
||||||
forward_for_reply
|
forward_for_reply
|
||||||
|
@ -197,6 +199,8 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
|
||||||
return if account.nil?
|
return if account.nil?
|
||||||
|
|
||||||
@mentions << Mention.new(account: account, silent: false)
|
@mentions << Mention.new(account: account, silent: false)
|
||||||
|
rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError
|
||||||
|
@unresolved_mentions << tag['href']
|
||||||
end
|
end
|
||||||
|
|
||||||
def process_emoji(tag)
|
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] })
|
ThreadResolveWorker.perform_async(status.id, in_reply_to_uri, { 'request_id' => @options[:request_id] })
|
||||||
end
|
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)
|
def fetch_replies(status)
|
||||||
collection = @object['replies']
|
collection = @object['replies']
|
||||||
return if collection.blank?
|
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
|
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)
|
def activity_for_object(json)
|
||||||
{
|
{
|
||||||
'@context': 'https://www.w3.org/ns/activitystreams',
|
'@context': 'https://www.w3.org/ns/activitystreams',
|
||||||
|
@ -117,6 +135,25 @@ RSpec.describe ActivityPub::Activity::Create do
|
||||||
# Creates two notifications
|
# Creates two notifications
|
||||||
expect(Notification.count).to eq 2
|
expect(Notification.count).to eq 2
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
describe '#perform' do
|
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…
Reference in a new issue