From e509519db4fe6c9c0c3942ebd9ccb540439fba18 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sat, 20 Jul 2024 21:21:21 -0400 Subject: [PATCH 1/6] Publisher jobs will not retry if the error received is a 400 --- changelog.d/oban-cancel-badreq.change | 1 + lib/pleroma/web/activity_pub/publisher.ex | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog.d/oban-cancel-badreq.change diff --git a/changelog.d/oban-cancel-badreq.change b/changelog.d/oban-cancel-badreq.change new file mode 100644 index 000000000..c7951735c --- /dev/null +++ b/changelog.d/oban-cancel-badreq.change @@ -0,0 +1 @@ +Publisher jobs will not retry if the error received is a 400 diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 90c4274f2..c8bdf2250 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -123,6 +123,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do Logger.error("Publisher failed to inbox #{inbox} with status #{code}") case response do + %{status: 400} -> {:cancel, :bad_request} %{status: 403} -> {:cancel, :forbidden} %{status: 404} -> {:cancel, :not_found} %{status: 410} -> {:cancel, :not_found} From b8503f1ad412df72af2dc2d975d29e8810143667 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sat, 20 Jul 2024 21:24:13 -0400 Subject: [PATCH 2/6] PollWorker jobs will not retry if the activity no longer exists. --- changelog.d/oban-cancel-poll-result.change | 1 + lib/pleroma/workers/poll_worker.ex | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 changelog.d/oban-cancel-poll-result.change diff --git a/changelog.d/oban-cancel-poll-result.change b/changelog.d/oban-cancel-poll-result.change new file mode 100644 index 000000000..b51c460a7 --- /dev/null +++ b/changelog.d/oban-cancel-poll-result.change @@ -0,0 +1 @@ +PollWorker jobs will not retry if the activity no longer exists. diff --git a/lib/pleroma/workers/poll_worker.ex b/lib/pleroma/workers/poll_worker.ex index 3fcac9bc3..af8997e70 100644 --- a/lib/pleroma/workers/poll_worker.ex +++ b/lib/pleroma/workers/poll_worker.ex @@ -17,6 +17,9 @@ defmodule Pleroma.Workers.PollWorker do with %Activity{} = activity <- find_poll_activity(activity_id), {:ok, notifications} <- Notification.create_poll_notifications(activity) do Notification.stream(notifications) + else + {:error, :poll_activity_not_found} = e -> {:cancel, e} + e -> {:error, e} end end From d62a9afed3fe08aeaf9d2d9f0ca79736a1f24578 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sat, 20 Jul 2024 21:52:57 -0400 Subject: [PATCH 3/6] Improved detecting unrecoverable errors for incoming federation jobs --- changelog.d/oban-cancel-receiverworker.change | 1 + lib/pleroma/workers/receiver_worker.ex | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 changelog.d/oban-cancel-receiverworker.change diff --git a/changelog.d/oban-cancel-receiverworker.change b/changelog.d/oban-cancel-receiverworker.change new file mode 100644 index 000000000..70ad22d60 --- /dev/null +++ b/changelog.d/oban-cancel-receiverworker.change @@ -0,0 +1 @@ +Improved detecting unrecoverable errors for incoming federation jobs diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index 8b2052c23..cf1269a1f 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -47,11 +47,13 @@ defmodule Pleroma.Workers.ReceiverWorker do case errors do {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} {:error, :already_present} -> {:cancel, :already_present} - {:error, {:validate_object, reason}} -> {:cancel, reason} - {:error, {:error, {:validate, reason}}} -> {:cancel, reason} - {:error, {:reject, reason}} -> {:cancel, reason} + {:error, {:validate_object, _} = reason} -> {:cancel, reason} + {:error, {:validate, _} = reason} -> {:cancel, reason} + {:error, {:reject, _} = reason} -> {:cancel, reason} {:signature, false} -> {:cancel, :invalid_signature} - {:error, {:error, reason = "Object has been deleted"}} -> {:cancel, reason} + {:error, "Object has been deleted"} = reason -> {:cancel, reason} + {:error, {:side_effects, {:error, :no_object_actor}} = reason} -> {:cancel, reason} + {:error, :not_found} = reason -> {:cancel, reason} {:error, _} = e -> e e -> {:error, e} end From fdeb8616e8452d78060fb41622688b7143687058 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sat, 20 Jul 2024 22:00:34 -0400 Subject: [PATCH 4/6] Increase timeout for background, remote fetcher, and user refresh jobs --- changelog.d/oban-timeouts.skip | 0 lib/pleroma/web/activity_pub/side_effects.ex | 2 +- lib/pleroma/workers/background_worker.ex | 2 +- lib/pleroma/workers/remote_fetcher_worker.ex | 2 +- lib/pleroma/workers/user_refresh_worker.ex | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 changelog.d/oban-timeouts.skip diff --git a/changelog.d/oban-timeouts.skip b/changelog.d/oban-timeouts.skip new file mode 100644 index 000000000..e69de29bb diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 2a141b0f5..cc1c7a0af 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -453,7 +453,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do ) do orig_object_ap_id = updated_object["id"] orig_object = Object.get_by_ap_id(orig_object_ap_id) - orig_object_data = orig_object.data + orig_object_data = Map.get(orig_object, :data) updated_object = if meta[:local] do diff --git a/lib/pleroma/workers/background_worker.ex b/lib/pleroma/workers/background_worker.ex index d74cc08f1..870aef3c6 100644 --- a/lib/pleroma/workers/background_worker.ex +++ b/lib/pleroma/workers/background_worker.ex @@ -40,5 +40,5 @@ defmodule Pleroma.Workers.BackgroundWorker do end @impl Oban.Worker - def timeout(_job), do: :timer.seconds(5) + def timeout(_job), do: :timer.seconds(15) end diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 1c8874b59..60096e14b 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -31,5 +31,5 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do end @impl Oban.Worker - def timeout(_job), do: :timer.seconds(10) + def timeout(_job), do: :timer.seconds(15) end diff --git a/lib/pleroma/workers/user_refresh_worker.ex b/lib/pleroma/workers/user_refresh_worker.ex index 0c04fb237..fb90e9c9c 100644 --- a/lib/pleroma/workers/user_refresh_worker.ex +++ b/lib/pleroma/workers/user_refresh_worker.ex @@ -13,5 +13,5 @@ defmodule Pleroma.Workers.UserRefreshWorker do end @impl Oban.Worker - def timeout(_job), do: :timer.seconds(5) + def timeout(_job), do: :timer.seconds(15) end From cf70656d141b075bf99114a662aba1e940106d65 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sat, 20 Jul 2024 23:58:47 -0400 Subject: [PATCH 5/6] Fix test now that the reject error has more detail --- test/pleroma/workers/receiver_worker_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pleroma/workers/receiver_worker_test.exs b/test/pleroma/workers/receiver_worker_test.exs index b9b6d6af2..2b8bd3c40 100644 --- a/test/pleroma/workers/receiver_worker_test.exs +++ b/test/pleroma/workers/receiver_worker_test.exs @@ -16,7 +16,7 @@ defmodule Pleroma.Workers.ReceiverWorkerTest do with_mock Pleroma.Web.ActivityPub.Transmogrifier, handle_incoming: fn _ -> {:reject, "MRF"} end do - assert {:cancel, "MRF"} = + assert {:cancel, {:reject, "MRF"}} = ReceiverWorker.perform(%Oban.Job{ args: %{"op" => "incoming_ap_doc", "params" => params} }) From f9647a86ede93da9066671790f37234128669f0f Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sun, 21 Jul 2024 00:03:51 -0400 Subject: [PATCH 6/6] Fix the ObjectValidator error matching --- lib/pleroma/workers/receiver_worker.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/workers/receiver_worker.ex b/lib/pleroma/workers/receiver_worker.ex index cf1269a1f..d01813c25 100644 --- a/lib/pleroma/workers/receiver_worker.ex +++ b/lib/pleroma/workers/receiver_worker.ex @@ -48,7 +48,7 @@ defmodule Pleroma.Workers.ReceiverWorker do {:error, :origin_containment_failed} -> {:cancel, :origin_containment_failed} {:error, :already_present} -> {:cancel, :already_present} {:error, {:validate_object, _} = reason} -> {:cancel, reason} - {:error, {:validate, _} = reason} -> {:cancel, reason} + {:error, {:error, {:validate, {:error, _changeset} = reason}}} -> {:cancel, reason} {:error, {:reject, _} = reason} -> {:cancel, reason} {:signature, false} -> {:cancel, :invalid_signature} {:error, "Object has been deleted"} = reason -> {:cancel, reason}