From ea0ec5fbcf2e17d97f323bcb6872df48d5e42714 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 14:20:33 -0500 Subject: [PATCH 01/23] Remove Fetcher.fetch_object_from_id!/2 It was only being called once and can be replaced with a case statement. --- lib/pleroma/object.ex | 5 ++++- lib/pleroma/object/fetcher.ex | 26 +++++--------------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index fa5baf1a4..f2ba24abf 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -177,7 +177,10 @@ defmodule Pleroma.Object do ap_id Keyword.get(options, :fetch) -> - Fetcher.fetch_object_from_id!(ap_id, options) + case Fetcher.fetch_object_from_id(ap_id, options) do + {:ok, object} -> object + _ -> nil + end true -> get_cached_by_ap_id(ap_id) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index cc3772563..508ee7e62 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -79,9 +79,11 @@ defmodule Pleroma.Object.Fetcher do {:error, "Object containment failed."} {:transmogrifier, {:error, {:reject, e}}} -> + Logger.info("Rejected #{id} while fetching: #{inspect(e)}") {:reject, e} {:transmogrifier, {:reject, e}} -> + Logger.info("Rejected #{id} while fetching: #{inspect(e)}") {:reject, e} {:transmogrifier, _} = e -> @@ -97,10 +99,12 @@ defmodule Pleroma.Object.Fetcher do {:ok, object} {:fetch, {:error, error}} -> + Logger.error("Error while fetching #{id}: #{inspect(error)}") {:error, error} e -> - e + Logger.error("Error while fetching #{id}: #{inspect(e)}") + {:error, e} end end @@ -117,26 +121,6 @@ defmodule Pleroma.Object.Fetcher do |> Maps.put_if_present("bcc", data["bcc"]) end - def fetch_object_from_id!(id, options \\ []) do - with {:ok, object} <- fetch_object_from_id(id, options) do - object - else - {:error, %Tesla.Mock.Error{}} -> - nil - - {:error, "Object has been deleted"} -> - nil - - {:reject, reason} -> - Logger.info("Rejected #{id} while fetching: #{inspect(reason)}") - nil - - e -> - Logger.error("Error while fetching #{id}: #{inspect(e)}") - nil - end - end - defp make_signature(id, date) do uri = URI.parse(id) From 603e9f6a92e8ea2d2d34d2af26ec2b12fe4b8543 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 14:22:04 -0500 Subject: [PATCH 02/23] Fix Transmogrifier tests These tests relied on the removed Fetcher.fetch_object_from_id!/2 function injecting the error tuple into a log message with the exact words "Object containment failed." We will keep this behavior by generating a similar log message, but perhaps this should do a better job of matching on the error tuple returned by Transmogrifier.handle_incoming/1 --- lib/pleroma/object/fetcher.ex | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 508ee7e62..13fb50639 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -75,8 +75,9 @@ defmodule Pleroma.Object.Fetcher do {:allowed_depth, false} -> {:error, "Max thread distance exceeded."} - {:containment, _} -> - {:error, "Object containment failed."} + {:containment, e} -> + Logger.info("Error while fetching #{id}: Object containment failed. #{inspect(e)}") + {:error, e} {:transmogrifier, {:error, {:reject, e}}} -> Logger.info("Rejected #{id} while fetching: #{inspect(e)}") From d472bafec19cee269e7c943bafae7c805785acd7 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 15:54:14 -0500 Subject: [PATCH 03/23] Mark instances as unreachable when returning a 403 from an object fetch This is a definite sign the instance is blocked and they are enforcing authorized_fetch --- lib/pleroma/object/fetcher.ex | 9 +++++++++ lib/pleroma/workers/remote_fetcher_worker.ex | 11 +++++++++++ test/pleroma/object/fetcher_test.exs | 15 +++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 13fb50639..a07b35d6f 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -99,6 +99,15 @@ defmodule Pleroma.Object.Fetcher do {:fetch_object, %Object{} = object} -> {:ok, object} + {:fetch, {:error, {:ok, %Tesla.Env{status: 403}}}} -> + Instances.set_consistently_unreachable(id) + + Logger.error( + "Error while fetching #{id}: HTTP 403 likely due to instance block rejecting the signed fetch." + ) + + {:error, "Object fetch has been denied"} + {:fetch, {:error, error}} -> Logger.error("Error while fetching #{id}: #{inspect(error)}") {:error, error} diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index d2a77aa17..70d074c1c 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -10,6 +10,17 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do @impl Oban.Worker def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do {:ok, _object} = Fetcher.fetch_object_from_id(id, depth: args["depth"]) + + case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do + {:ok, _object} -> + :ok + + {:error, reason = "Object fetch has been denied"} -> + {:cancel, reason} + + _ -> + :error + end end @impl Oban.Worker diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 53c9277d6..80272946c 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -25,6 +25,9 @@ defmodule Pleroma.Object.FetcherTest do %{method: :get, url: "https://mastodon.example.org/users/userisgone404"} -> %Tesla.Env{status: 404} + %{method: :get, url: "https://octodon.social/users/cwebber/statuses/111647596861000656"} -> + %Tesla.Env{status: 403} + %{ method: :get, url: @@ -233,6 +236,18 @@ defmodule Pleroma.Object.FetcherTest do ) end + test "handle HTTP 403 response" do + object_id = "https://octodon.social/users/cwebber/statuses/111647596861000656" + Instances.set_reachable(object_id) + + assert Instances.reachable?(object_id) + + assert {:error, "Object fetch has been denied"} == + Fetcher.fetch_object_from_id(object_id) + + refute Instances.reachable?(object_id) + end + test "it can fetch pleroma polls with attachments" do {:ok, object} = Fetcher.fetch_object_from_id("https://patch.cx/objects/tesla_mock/poll_attachment") From 67dd81e8254576950fc20ed84d3da3a3d24c1706 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 16:05:28 -0500 Subject: [PATCH 04/23] Consolidate the HTTP status code checking into the private get_object/1 --- lib/pleroma/object/fetcher.ex | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index a07b35d6f..75f39fb6a 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -99,15 +99,6 @@ defmodule Pleroma.Object.Fetcher do {:fetch_object, %Object{} = object} -> {:ok, object} - {:fetch, {:error, {:ok, %Tesla.Env{status: 403}}}} -> - Instances.set_consistently_unreachable(id) - - Logger.error( - "Error while fetching #{id}: HTTP 403 likely due to instance block rejecting the signed fetch." - ) - - {:error, "Object fetch has been denied"} - {:fetch, {:error, error}} -> Logger.error("Error while fetching #{id}: #{inspect(error)}") {:error, error} @@ -221,6 +212,10 @@ defmodule Pleroma.Object.Fetcher do {:error, {:content_type, nil}} end + {:ok, %{status: 403}} -> + Instances.set_consistently_unreachable(id) + {:error, "Object fetch has been denied"} + {:ok, %{status: code}} when code in [404, 410] -> {:error, "Object has been deleted"} From c6b38441f15a4f3bb925bc4ffc68ac8930bc29b2 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 16:05:44 -0500 Subject: [PATCH 05/23] Cancel remote fetch jobs for deleted objects --- lib/pleroma/workers/remote_fetcher_worker.ex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 70d074c1c..1a7bd2586 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -18,6 +18,9 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do {:error, reason = "Object fetch has been denied"} -> {:cancel, reason} + {:error, reason = "Object has been deleted"} -> + {:cancel, reason} + _ -> :error end From c4f0a3b570294e5746a6234156d9f01d3ad591fd Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 16:08:36 -0500 Subject: [PATCH 06/23] Changelogs --- changelog.d/handle_object_fetch_failures.change | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/handle_object_fetch_failures.change diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change new file mode 100644 index 000000000..413084322 --- /dev/null +++ b/changelog.d/handle_object_fetch_failures.change @@ -0,0 +1,2 @@ +Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403 due to instance block behavior with authorized_fetch enabled. +Mark instances as unreachable when object fetch is denied due to instance block and authorized_fetch. From 5f6966cd9ff9c728d97a8e4075cba738ceca0aeb Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 16:23:41 -0500 Subject: [PATCH 07/23] Remove mistaken duplicate fetch --- lib/pleroma/workers/remote_fetcher_worker.ex | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 1a7bd2586..ff0286e54 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -9,8 +9,6 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do @impl Oban.Worker def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do - {:ok, _object} = Fetcher.fetch_object_from_id(id, depth: args["depth"]) - case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do {:ok, _object} -> :ok From 9c0040124a9ea68cedca4959d32105ee6a6c3ee1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 16:28:05 -0500 Subject: [PATCH 08/23] Skip remote fetch jobs for unreachable instances --- .../handle_object_fetch_failures.change | 1 + lib/pleroma/workers/remote_fetcher_worker.ex | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change index 413084322..410f95efa 100644 --- a/changelog.d/handle_object_fetch_failures.change +++ b/changelog.d/handle_object_fetch_failures.change @@ -1,2 +1,3 @@ Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403 due to instance block behavior with authorized_fetch enabled. Mark instances as unreachable when object fetch is denied due to instance block and authorized_fetch. +Skip fetching objects from unreachable instances. diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index ff0286e54..d4fae33df 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -3,24 +3,29 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.RemoteFetcherWorker do + alias Pleroma.Instances alias Pleroma.Object.Fetcher use Pleroma.Workers.WorkerHelper, queue: "remote_fetcher" @impl Oban.Worker def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do - case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do - {:ok, _object} -> - :ok + if Instances.reachable?(id) do + case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do + {:ok, _object} -> + :ok - {:error, reason = "Object fetch has been denied"} -> - {:cancel, reason} + {:error, reason = "Object fetch has been denied"} -> + {:cancel, reason} - {:error, reason = "Object has been deleted"} -> - {:cancel, reason} + {:error, reason = "Object has been deleted"} -> + {:cancel, reason} - _ -> - :error + _ -> + :error + end + else + {:cancel, "Unreachable instance"} end end From 73c4c6d7de6d33c68cf663e65df8525ce8eef4f5 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Tue, 26 Dec 2023 17:12:58 -0500 Subject: [PATCH 09/23] Revert "Mark instances as unreachable when returning a 403 from an object fetch" This reverts commit d472bafec19cee269e7c943bafae7c805785acd7. --- changelog.d/handle_object_fetch_failures.change | 1 - lib/pleroma/object/fetcher.ex | 1 - test/pleroma/object/fetcher_test.exs | 15 --------------- 3 files changed, 17 deletions(-) diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change index 410f95efa..0b1dda38d 100644 --- a/changelog.d/handle_object_fetch_failures.change +++ b/changelog.d/handle_object_fetch_failures.change @@ -1,3 +1,2 @@ Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403 due to instance block behavior with authorized_fetch enabled. -Mark instances as unreachable when object fetch is denied due to instance block and authorized_fetch. Skip fetching objects from unreachable instances. diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 75f39fb6a..f1a0a483b 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -213,7 +213,6 @@ defmodule Pleroma.Object.Fetcher do end {:ok, %{status: 403}} -> - Instances.set_consistently_unreachable(id) {:error, "Object fetch has been denied"} {:ok, %{status: code}} when code in [404, 410] -> diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 80272946c..53c9277d6 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -25,9 +25,6 @@ defmodule Pleroma.Object.FetcherTest do %{method: :get, url: "https://mastodon.example.org/users/userisgone404"} -> %Tesla.Env{status: 404} - %{method: :get, url: "https://octodon.social/users/cwebber/statuses/111647596861000656"} -> - %Tesla.Env{status: 403} - %{ method: :get, url: @@ -236,18 +233,6 @@ defmodule Pleroma.Object.FetcherTest do ) end - test "handle HTTP 403 response" do - object_id = "https://octodon.social/users/cwebber/statuses/111647596861000656" - Instances.set_reachable(object_id) - - assert Instances.reachable?(object_id) - - assert {:error, "Object fetch has been denied"} == - Fetcher.fetch_object_from_id(object_id) - - refute Instances.reachable?(object_id) - end - test "it can fetch pleroma polls with attachments" do {:ok, object} = Fetcher.fetch_object_from_id("https://patch.cx/objects/tesla_mock/poll_attachment") From 5f5109413840d6ebcbee632fb883ae27dc3b45f7 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 11:09:42 -0500 Subject: [PATCH 10/23] Update changelog --- changelog.d/handle_object_fetch_failures.change | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change index 0b1dda38d..02c66a934 100644 --- a/changelog.d/handle_object_fetch_failures.change +++ b/changelog.d/handle_object_fetch_failures.change @@ -1,2 +1,2 @@ -Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403 due to instance block behavior with authorized_fetch enabled. +Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403. Skip fetching objects from unreachable instances. From d4c77103d1fe5df9b2ea8bd3429a8fc0240645c1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 15:27:37 -0500 Subject: [PATCH 11/23] Fix detection of user follower collection being private We were overzealous with matching on a raw error from the object fetch that should have never been relied on like this. If we can't fetch successfully we should assume that the collection is private. Building a more expressive and universal error struct to match on may be something to consider. --- lib/pleroma/web/activity_pub/activity_pub.ex | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 32d1a1037..39159b1c4 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -1697,9 +1697,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do Fetcher.fetch_and_contain_remote_object_from_id(first) do {:ok, false} else - {:error, {:ok, %{status: code}}} when code in [401, 403] -> {:ok, true} - {:error, _} = e -> e - e -> {:error, e} + {:error, _} -> {:ok, true} end end From 53db65678d4efaeb185bd9544401ef967ed20c3b Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 15:44:31 -0500 Subject: [PATCH 12/23] Separate files for each distinct sentence in the changelog --- changelog.d/handle_object_fetch_failures.change | 1 - changelog.d/handle_object_fetch_failures2.change | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 changelog.d/handle_object_fetch_failures2.change diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change index 02c66a934..03fbd4b9e 100644 --- a/changelog.d/handle_object_fetch_failures.change +++ b/changelog.d/handle_object_fetch_failures.change @@ -1,2 +1 @@ Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403. -Skip fetching objects from unreachable instances. diff --git a/changelog.d/handle_object_fetch_failures2.change b/changelog.d/handle_object_fetch_failures2.change new file mode 100644 index 000000000..f12350026 --- /dev/null +++ b/changelog.d/handle_object_fetch_failures2.change @@ -0,0 +1 @@ +Skip fetching objects from unreachable instances. From be0cca9afd8edf1e550f71f17914607c995b64f8 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 16:06:10 -0500 Subject: [PATCH 13/23] RemoteFetcherWorker Oban job tests --- .../workers/remote_fetcher_worker_test.exs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 test/pleroma/workers/remote_fetcher_worker_test.exs diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs new file mode 100644 index 000000000..d8cb52f52 --- /dev/null +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -0,0 +1,67 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2023 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Workers.RemoteFetcherWorkerTest do + use Pleroma.DataCase + use Oban.Testing, repo: Pleroma.Repo + + alias Pleroma.Instances + alias Pleroma.Workers.RemoteFetcherWorker + + @deleted_object_one "https://deleted-404.example.com/" + @deleted_object_two "https://deleted-410.example.com/" + @unauthorized_object "https://unauthorized.example.com/" + @unreachable_object "https://unreachable.example.com/" + + describe "it does not" do + setup do + Tesla.Mock.mock(fn + %{method: :get, url: @deleted_object_one} -> + %Tesla.Env{ + status: 404 + } + + %{method: :get, url: @deleted_object_two} -> + %Tesla.Env{ + status: 410 + } + + %{method: :get, url: @unauthorized_object} -> + %Tesla.Env{ + status: 403 + } + end) + end + + test "requeue a deleted object" do + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @deleted_object_one} + }) + + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @deleted_object_two} + }) + end + + test "requeue an unauthorized object" do + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @unauthorized_object} + }) + end + + test "fetch an unreachable instance" do + Instances.set_consistently_unreachable(@unreachable_object) + + refute Instances.reachable?(@unreachable_object) + + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @unreachable_object} + }) + end + end +end From 6c9929b809290d9caca6a7a40eb0f59af6dd558d Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 20:18:14 -0500 Subject: [PATCH 14/23] Set Logger level to error --- lib/pleroma/object/fetcher.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index f1a0a483b..9ba0ac0ca 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -76,15 +76,15 @@ defmodule Pleroma.Object.Fetcher do {:error, "Max thread distance exceeded."} {:containment, e} -> - Logger.info("Error while fetching #{id}: Object containment failed. #{inspect(e)}") + Logger.error("Error while fetching #{id}: Object containment failed. #{inspect(e)}") {:error, e} {:transmogrifier, {:error, {:reject, e}}} -> - Logger.info("Rejected #{id} while fetching: #{inspect(e)}") + Logger.error("Rejected #{id} while fetching: #{inspect(e)}") {:reject, e} {:transmogrifier, {:reject, e}} -> - Logger.info("Rejected #{id} while fetching: #{inspect(e)}") + Logger.error("Rejected #{id} while fetching: #{inspect(e)}") {:reject, e} {:transmogrifier, _} = e -> From becb070603bc65d5302698a2b6cf5f89b3d5c1f0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 20:47:18 -0500 Subject: [PATCH 15/23] Conslidate log messages for object fetcher failures and leverage Logger.metadata --- lib/pleroma/object/fetcher.ex | 39 +++++++++++-------- .../web/activity_pub/transmogrifier_test.exs | 8 ++-- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 9ba0ac0ca..72420519a 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -72,23 +72,25 @@ defmodule Pleroma.Object.Fetcher do {:object, data, Object.normalize(activity, fetch: false)} do {:ok, object} else - {:allowed_depth, false} -> + {:allowed_depth, false} = e -> + log_fetch_error(id, e) {:error, "Max thread distance exceeded."} - {:containment, e} -> - Logger.error("Error while fetching #{id}: Object containment failed. #{inspect(e)}") - {:error, e} + {:containment, reason} = e -> + log_fetch_error(id, e) + {:error, reason} - {:transmogrifier, {:error, {:reject, e}}} -> - Logger.error("Rejected #{id} while fetching: #{inspect(e)}") - {:reject, e} + {:transmogrifier, {:error, {:reject, reason}}} = e -> + log_fetch_error(id, e) + {:reject, reason} - {:transmogrifier, {:reject, e}} -> - Logger.error("Rejected #{id} while fetching: #{inspect(e)}") - {:reject, e} + {:transmogrifier, {:reject, reason}} = e -> + log_fetch_error(id, e) + {:reject, reason} - {:transmogrifier, _} = e -> - {:error, e} + {:transmogrifier, reason} = e -> + log_fetch_error(id, e) + {:error, reason} {:object, data, nil} -> reinject_object(%Object{}, data) @@ -99,16 +101,21 @@ defmodule Pleroma.Object.Fetcher do {:fetch_object, %Object{} = object} -> {:ok, object} - {:fetch, {:error, error}} -> - Logger.error("Error while fetching #{id}: #{inspect(error)}") - {:error, error} + {:fetch, {:error, reason}} = e -> + log_fetch_error(id, e) + {:error, reason} e -> - Logger.error("Error while fetching #{id}: #{inspect(e)}") + log_fetch_error(id, e) {:error, e} end end + defp log_fetch_error(id, error) do + Logger.metadata([object: id]) + Logger.error("Object rejected while fetching #{id} #{inspect(error)}") + end + defp prepare_activity_params(data) do %{ "type" => "Create", diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index 9c5983347..a49e459a6 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -132,7 +132,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert {:ok, activity} = Transmogrifier.handle_incoming(message) object = Object.normalize(activity) assert [%{"type" => "Mention"}, %{"type" => "Link"}] = object.data["tag"] - end) =~ "Error while fetching" + end) =~ "Object rejected while fetching" end test "it accepts quote posts" do @@ -410,7 +410,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert capture_log(fn -> {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + end) =~ "Object rejected while fetching" end test "it rejects activities which reference objects that have an incorrect attribution (variant 1)" do @@ -425,7 +425,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert capture_log(fn -> {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + end) =~ "Object rejected while fetching" end test "it rejects activities which reference objects that have an incorrect attribution (variant 2)" do @@ -440,7 +440,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert capture_log(fn -> {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + end) =~ "Object rejected while fetching" end end From 882267e3eceaa61cff16a9e0dee231d54a80d927 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 21:39:17 -0500 Subject: [PATCH 16/23] Remove duplicate log messages from Transmogrifier Object fetch errors are logged in the fetcher module --- lib/pleroma/web/activity_pub/transmogrifier.ex | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 35f3aea03..80f64cf1e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -23,7 +23,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do import Ecto.Query - require Logger require Pleroma.Constants @doc """ @@ -155,8 +154,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do |> Map.put("context", replied_object.data["context"] || object["conversation"]) |> Map.drop(["conversation", "inReplyToAtomUri"]) else - e -> - Logger.warning("Couldn't fetch #{inspect(in_reply_to_id)}, error: #{inspect(e)}") + _ -> object end else @@ -181,8 +179,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do {:quoting?, _} -> object - e -> - Logger.warning("Couldn't fetch #{inspect(quote_url)}, error: #{inspect(e)}") + _ -> object end end @@ -852,8 +849,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do relative_object do Map.put(data, "object", external_url) else - {:fetch, e} -> - Logger.error("Couldn't fetch #{object} #{inspect(e)}") + {:fetch, _} -> data _ -> From 287f2c9719310f96ce37b445f26e069f804c34b2 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 21:55:07 -0500 Subject: [PATCH 17/23] Formatting --- lib/pleroma/object/fetcher.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 72420519a..5bc1cfd5e 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -112,7 +112,7 @@ defmodule Pleroma.Object.Fetcher do end defp log_fetch_error(id, error) do - Logger.metadata([object: id]) + Logger.metadata(object: id) Logger.error("Object rejected while fetching #{id} #{inspect(error)}") end From a2708f7fe31c6009b0c9954e5de3b74c45f9818f Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 21:57:47 -0500 Subject: [PATCH 18/23] Leverage existing atoms as return errors for the object fetcher --- lib/pleroma/object/fetcher.ex | 4 ++-- lib/pleroma/workers/remote_fetcher_worker.ex | 8 ++++---- test/pleroma/object/fetcher_test.exs | 4 ++-- .../web/twitter_api/remote_follow_controller_test.exs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 5bc1cfd5e..0c90e0eaa 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -220,10 +220,10 @@ defmodule Pleroma.Object.Fetcher do end {:ok, %{status: 403}} -> - {:error, "Object fetch has been denied"} + {:error, :forbidden} {:ok, %{status: code}} when code in [404, 410] -> - {:error, "Object has been deleted"} + {:error, :not_found} {:error, e} -> {:error, e} diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index d4fae33df..dca4530d7 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -15,11 +15,11 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do {:ok, _object} -> :ok - {:error, reason = "Object fetch has been denied"} -> - {:cancel, reason} + {:error, :forbidden} -> + {:cancel, :forbidden} - {:error, reason = "Object has been deleted"} -> - {:cancel, reason} + {:error, :not_found} -> + {:cancel, :not_found} _ -> :error diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 53c9277d6..24f114c00 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -220,14 +220,14 @@ defmodule Pleroma.Object.FetcherTest do end test "handle HTTP 410 Gone response" do - assert {:error, "Object has been deleted"} == + assert {:error, :not_found} == Fetcher.fetch_and_contain_remote_object_from_id( "https://mastodon.example.org/users/userisgone" ) end test "handle HTTP 404 response" do - assert {:error, "Object has been deleted"} == + assert {:error, :not_found} == Fetcher.fetch_and_contain_remote_object_from_id( "https://mastodon.example.org/users/userisgone404" ) diff --git a/test/pleroma/web/twitter_api/remote_follow_controller_test.exs b/test/pleroma/web/twitter_api/remote_follow_controller_test.exs index 41f8ebcd7..c6ecb53f4 100644 --- a/test/pleroma/web/twitter_api/remote_follow_controller_test.exs +++ b/test/pleroma/web/twitter_api/remote_follow_controller_test.exs @@ -137,7 +137,7 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowControllerTest do |> html_response(200) assert response =~ "Error fetching user" - end) =~ "Object has been deleted" + end) =~ ":not_found" end end From ad0a5deb67f454b0529a4faf72399cd9ecc9c0e6 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 22:28:41 -0500 Subject: [PATCH 19/23] Prevent requeuing Remote Fetcher jobs that exceed thread depth --- changelog.d/handle_object_fetch_failures.change | 2 +- lib/pleroma/object/fetcher.ex | 2 +- lib/pleroma/workers/remote_fetcher_worker.ex | 3 +++ test/pleroma/object/fetcher_test.exs | 3 +-- .../workers/remote_fetcher_worker_test.exs | 15 +++++++++++++++ 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change index 03fbd4b9e..e115c8012 100644 --- a/changelog.d/handle_object_fetch_failures.change +++ b/changelog.d/handle_object_fetch_failures.change @@ -1 +1 @@ -Remote object fetch failures will prevent the object fetch job from retrying if the object has been deleted or the fetch was denied with a 403. +Remote object fetch failures will prevent the object fetch job from retrying if the object request returns 403, 404, 410, or exceeds the maximum thread depth. diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 0c90e0eaa..972dfaf28 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -74,7 +74,7 @@ defmodule Pleroma.Object.Fetcher do else {:allowed_depth, false} = e -> log_fetch_error(id, e) - {:error, "Max thread distance exceeded."} + {:error, :allowed_depth} {:containment, reason} = e -> log_fetch_error(id, e) diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index dca4530d7..fef8a26b2 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -21,6 +21,9 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do {:error, :not_found} -> {:cancel, :not_found} + {:error, :allowed_depth} -> + {:cancel, :allowed_depth} + _ -> :error end diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs index 24f114c00..6f21452a7 100644 --- a/test/pleroma/object/fetcher_test.exs +++ b/test/pleroma/object/fetcher_test.exs @@ -101,8 +101,7 @@ defmodule Pleroma.Object.FetcherTest do test "it returns thread depth exceeded error if thread depth is exceeded" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) - assert {:error, "Max thread distance exceeded."} = - Fetcher.fetch_object_from_id(@ap_id, depth: 1) + assert {:error, :allowed_depth} = Fetcher.fetch_object_from_id(@ap_id, depth: 1) end test "it fetches object if max thread depth is restricted to 0 and depth is not specified" do diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index d8cb52f52..df7d77ea0 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -13,6 +13,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do @deleted_object_two "https://deleted-410.example.com/" @unauthorized_object "https://unauthorized.example.com/" @unreachable_object "https://unreachable.example.com/" + @depth_object "https://depth.example.com/" describe "it does not" do setup do @@ -31,6 +32,11 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do %Tesla.Env{ status: 403 } + + %{method: :get, url: @depth_object} -> + %Tesla.Env{ + status: 200 + } end) end @@ -63,5 +69,14 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do args: %{"op" => "fetch_remote", "id" => @unreachable_object} }) end + + test "requeue an object that exceeded depth" do + clear_config([:instance, :federation_incoming_replies_max_depth], 0) + + assert {:cancel, _} = + RemoteFetcherWorker.perform(%Oban.Job{ + args: %{"op" => "fetch_remote", "id" => @depth_object, "depth" => 1} + }) + end end end From a6fd251e440ec3c6734f6b084c8a69f442bcebb0 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Wed, 27 Dec 2023 22:37:06 -0500 Subject: [PATCH 20/23] Improve test descriptions --- test/pleroma/workers/remote_fetcher_worker_test.exs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index df7d77ea0..84899cc5f 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -15,7 +15,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do @unreachable_object "https://unreachable.example.com/" @depth_object "https://depth.example.com/" - describe "it does not" do + describe "RemoteFetcherWorker" do setup do Tesla.Mock.mock(fn %{method: :get, url: @deleted_object_one} -> @@ -40,7 +40,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do end) end - test "requeue a deleted object" do + test "does not requeue a deleted object" do assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_one} @@ -52,14 +52,14 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do }) end - test "requeue an unauthorized object" do + test "does not requeue an unauthorized object" do assert {:cancel, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unauthorized_object} }) end - test "fetch an unreachable instance" do + test "does not fetch an unreachable instance" do Instances.set_consistently_unreachable(@unreachable_object) refute Instances.reachable?(@unreachable_object) @@ -70,7 +70,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do }) end - test "requeue an object that exceeded depth" do + test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) assert {:cancel, _} = From aa070c7dafbceb33b9656f54aa552672497942f6 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 28 Dec 2023 23:09:33 -0500 Subject: [PATCH 21/23] Handle 401s as I have observed it in the wild --- changelog.d/handle_object_fetch_failures.change | 2 +- lib/pleroma/object/fetcher.ex | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog.d/handle_object_fetch_failures.change b/changelog.d/handle_object_fetch_failures.change index e115c8012..ae44e6f4b 100644 --- a/changelog.d/handle_object_fetch_failures.change +++ b/changelog.d/handle_object_fetch_failures.change @@ -1 +1 @@ -Remote object fetch failures will prevent the object fetch job from retrying if the object request returns 403, 404, 410, or exceeds the maximum thread depth. +Remote object fetch failures will prevent the object fetch job from retrying if the object request returns 401, 403, 404, 410, or exceeds the maximum thread depth. diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex index 972dfaf28..af5642af4 100644 --- a/lib/pleroma/object/fetcher.ex +++ b/lib/pleroma/object/fetcher.ex @@ -219,7 +219,7 @@ defmodule Pleroma.Object.Fetcher do {:error, {:content_type, nil}} end - {:ok, %{status: 403}} -> + {:ok, %{status: code}} when code in [401, 403] -> {:error, :forbidden} {:ok, %{status: code}} when code in [404, 410] -> From f17f92105bff555d2d372ff2ec053fe40fa1b41b Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 28 Dec 2023 23:52:05 -0500 Subject: [PATCH 22/23] Oban jobs should be discarded on permanent errors --- lib/pleroma/workers/remote_fetcher_worker.ex | 8 ++++---- test/pleroma/workers/remote_fetcher_worker_test.exs | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index fef8a26b2..7919969aa 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -16,19 +16,19 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do :ok {:error, :forbidden} -> - {:cancel, :forbidden} + {:discard, :forbidden} {:error, :not_found} -> - {:cancel, :not_found} + {:discard, :not_found} {:error, :allowed_depth} -> - {:cancel, :allowed_depth} + {:discard, :allowed_depth} _ -> :error end else - {:cancel, "Unreachable instance"} + {:discard, "Unreachable instance"} end end diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index 84899cc5f..493b10fdc 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -41,19 +41,19 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do end test "does not requeue a deleted object" do - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_one} }) - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @deleted_object_two} }) end test "does not requeue an unauthorized object" do - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unauthorized_object} }) @@ -64,7 +64,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do refute Instances.reachable?(@unreachable_object) - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @unreachable_object} }) @@ -73,7 +73,7 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0) - assert {:cancel, _} = + assert {:discard, _} = RemoteFetcherWorker.perform(%Oban.Job{ args: %{"op" => "fetch_remote", "id" => @depth_object, "depth" => 1} }) From 12c052551bcd6b7871ccde5b9228315b89f45e01 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Sun, 14 Jan 2024 13:23:17 -0500 Subject: [PATCH 23/23] Allow the Remote Fetcher to attempt fetching an unreachable instance --- .../handle_object_fetch_failures2.change | 1 - lib/pleroma/workers/remote_fetcher_worker.ex | 27 ++++++++----------- .../workers/remote_fetcher_worker_test.exs | 13 --------- 3 files changed, 11 insertions(+), 30 deletions(-) delete mode 100644 changelog.d/handle_object_fetch_failures2.change diff --git a/changelog.d/handle_object_fetch_failures2.change b/changelog.d/handle_object_fetch_failures2.change deleted file mode 100644 index f12350026..000000000 --- a/changelog.d/handle_object_fetch_failures2.change +++ /dev/null @@ -1 +0,0 @@ -Skip fetching objects from unreachable instances. diff --git a/lib/pleroma/workers/remote_fetcher_worker.ex b/lib/pleroma/workers/remote_fetcher_worker.ex index 7919969aa..d526a99cb 100644 --- a/lib/pleroma/workers/remote_fetcher_worker.ex +++ b/lib/pleroma/workers/remote_fetcher_worker.ex @@ -3,32 +3,27 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Workers.RemoteFetcherWorker do - alias Pleroma.Instances alias Pleroma.Object.Fetcher use Pleroma.Workers.WorkerHelper, queue: "remote_fetcher" @impl Oban.Worker def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do - if Instances.reachable?(id) do - case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do - {:ok, _object} -> - :ok + case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do + {:ok, _object} -> + :ok - {:error, :forbidden} -> - {:discard, :forbidden} + {:error, :forbidden} -> + {:discard, :forbidden} - {:error, :not_found} -> - {:discard, :not_found} + {:error, :not_found} -> + {:discard, :not_found} - {:error, :allowed_depth} -> - {:discard, :allowed_depth} + {:error, :allowed_depth} -> + {:discard, :allowed_depth} - _ -> - :error - end - else - {:discard, "Unreachable instance"} + _ -> + :error end end diff --git a/test/pleroma/workers/remote_fetcher_worker_test.exs b/test/pleroma/workers/remote_fetcher_worker_test.exs index 493b10fdc..c30e773d4 100644 --- a/test/pleroma/workers/remote_fetcher_worker_test.exs +++ b/test/pleroma/workers/remote_fetcher_worker_test.exs @@ -6,13 +6,11 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do use Pleroma.DataCase use Oban.Testing, repo: Pleroma.Repo - alias Pleroma.Instances alias Pleroma.Workers.RemoteFetcherWorker @deleted_object_one "https://deleted-404.example.com/" @deleted_object_two "https://deleted-410.example.com/" @unauthorized_object "https://unauthorized.example.com/" - @unreachable_object "https://unreachable.example.com/" @depth_object "https://depth.example.com/" describe "RemoteFetcherWorker" do @@ -59,17 +57,6 @@ defmodule Pleroma.Workers.RemoteFetcherWorkerTest do }) end - test "does not fetch an unreachable instance" do - Instances.set_consistently_unreachable(@unreachable_object) - - refute Instances.reachable?(@unreachable_object) - - assert {:discard, _} = - RemoteFetcherWorker.perform(%Oban.Job{ - args: %{"op" => "fetch_remote", "id" => @unreachable_object} - }) - end - test "does not requeue an object that exceeded depth" do clear_config([:instance, :federation_incoming_replies_max_depth], 0)