From 0cd4040db684b8d85b7a2e53417adf1fe536c31b Mon Sep 17 00:00:00 2001 From: Oneric Date: Wed, 4 Dec 2024 02:09:49 +0100 Subject: [PATCH] Error out earlier on missing mandatory reference This is the only user of fetch_actor_and_object which previously just always preteneded to be successful. For all the activity types handled here, we absolutely need the referenced object to be able to process it (other than Announce whether or not processing those activity types for unknown remote objects is desirable in the first place is up for debate) All other users of the similar fetch_actor already properly check success. Note, this currently lumps all reolv failure reasons together, so even e.g. boosts of MRF rejected posts will still exhaust all retries. The following commit improves on this. --- lib/pleroma/web/activity_pub/object_validator.ex | 9 ++++++--- lib/pleroma/web/activity_pub/transmogrifier.ex | 5 ++++- lib/pleroma/workers/receiver_worker.ex | 7 +++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index cb0cc9ed7..e44f2fdee 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validator.ex @@ -253,9 +253,12 @@ def fetch_actor(object) do end def fetch_actor_and_object(object) do - fetch_actor(object) - Object.normalize(object["object"], fetch: true) - :ok + with {:ok, %User{}} <- fetch_actor(object), + %Object{} <- Object.normalize(object["object"], fetch: true) do + :ok + else + _ -> :error + end end defp for_each_history_item( diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 251ba6540..02253a2c8 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -519,10 +519,13 @@ defp handle_incoming_normalised( defp handle_incoming_normalised(%{"type" => type} = data, _options) when type in ~w{Like EmojiReact Announce Add Remove} do - with :ok <- ObjectValidator.fetch_actor_and_object(data), + with {_, :ok} <- {:link, ObjectValidator.fetch_actor_and_object(data)}, {:ok, activity, _meta} <- Pipeline.common_pipeline(data, local: false) do {:ok, activity} else + {:link, _} -> + {:error, :link_resolve_failed} + e -> {:error, e} end diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 3eccba812..4cde93503 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -33,6 +33,13 @@ def perform(%Job{args: %{"op" => "incoming_ap_doc", "params" => params}}) do Logger.info("Received invalid AP document: (2e) #{inspect(issue)}") {:discard, :invalid} + # failed to resolve a necessary referenced remote AP object; + # might be temporary server/network trouble thus reattempt + {:error, :link_resolve_failed} = e -> + # TODO: lower to debug for PR! + Logger.info("Failed to resolve AP link; may retry: #{inspect(params)}") + e + {:error, _} = e -> Logger.error("Unexpected AP doc error: #{inspect(e)} from #{inspect(params)}") e