From 77949d4590b2a82ef6bb4c79f0777962991e28b1 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 29 Dec 2023 00:25:11 -0500 Subject: [PATCH 1/5] Make the Publisher log error less noisy --- changelog.d/publisher_log.change | 1 + lib/pleroma/web/activity_pub/publisher.ex | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog.d/publisher_log.change diff --git a/changelog.d/publisher_log.change b/changelog.d/publisher_log.change new file mode 100644 index 000000000..3f85f5a1e --- /dev/null +++ b/changelog.d/publisher_log.change @@ -0,0 +1 @@ +Publisher errors will now emit logs indicating the inbox that was not available for delivery. diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index e0dd2e7c9..103b00f11 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -119,7 +119,8 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa else {_post_result, response} = e -> unless params[:unreachable_since], do: Instances.set_unreachable(inbox) - Logger.error("Failed to publish activity #{id} #{inspect(e)}") + Logger.metadata(activity: id, inbox: inbox, status: code) + Logger.error("Publisher failed to inbox #{inbox} with status #{code}") {:error, response} end end From 7ebca7ecfa93f41d9eac2dcefa4b2e55f1b0c4ac Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 29 Dec 2023 00:25:33 -0500 Subject: [PATCH 2/5] Activity publishing failures will prevent the job from retrying if the publishing request returns a 403 or 410 --- changelog.d/publisher_discard.change | 1 + lib/pleroma/web/activity_pub/publisher.ex | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 changelog.d/publisher_discard.change diff --git a/changelog.d/publisher_discard.change b/changelog.d/publisher_discard.change new file mode 100644 index 000000000..85e530d8d --- /dev/null +++ b/changelog.d/publisher_discard.change @@ -0,0 +1 @@ +Activity publishing failures will prevent the job from retrying if the publishing request returns a 403 or 410 diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 103b00f11..1ce920e81 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -117,11 +117,16 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa result else - {_post_result, response} = e -> + {_post_result, %{status: code} = response} = e -> unless params[:unreachable_since], do: Instances.set_unreachable(inbox) Logger.metadata(activity: id, inbox: inbox, status: code) Logger.error("Publisher failed to inbox #{inbox} with status #{code}") - {:error, response} + + case response do + %{status: 403} -> {:discard, :forbidden} + %{status: 410} -> {:discard, :not_found} + _ -> {:error, response} + end end end From 141702538b7ebdc501425b09928332d345c2b577 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 29 Dec 2023 00:31:05 -0500 Subject: [PATCH 3/5] Discard on a 404 as well --- lib/pleroma/web/activity_pub/publisher.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 1ce920e81..282cc672a 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -124,6 +124,7 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa case response do %{status: 403} -> {:discard, :forbidden} + %{status: 404} -> {:discard, :not_found} %{status: 410} -> {:discard, :not_found} _ -> {:error, response} end From 4afe211e50466606c898e3fbd08b617413ef2e16 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 29 Dec 2023 01:04:05 -0500 Subject: [PATCH 4/5] Return the full tuple from Tesla --- lib/pleroma/web/activity_pub/publisher.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index 282cc672a..e44e6eac1 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -126,7 +126,7 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa %{status: 403} -> {:discard, :forbidden} %{status: 404} -> {:discard, :not_found} %{status: 410} -> {:discard, :not_found} - _ -> {:error, response} + _ -> {:error, e} end end end From 833117f5738c286864d525aea9a87d1a7c193ff3 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 29 Dec 2023 12:18:23 -0500 Subject: [PATCH 5/5] Fix tests Need to handle the edge case of no valid HTTP response which has no status code --- lib/pleroma/web/activity_pub/publisher.ex | 6 ++++++ test/pleroma/web/activity_pub/publisher_test.exs | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/publisher.ex b/lib/pleroma/web/activity_pub/publisher.ex index e44e6eac1..cc47b3d27 100644 --- a/lib/pleroma/web/activity_pub/publisher.ex +++ b/lib/pleroma/web/activity_pub/publisher.ex @@ -128,6 +128,12 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa %{status: 410} -> {:discard, :not_found} _ -> {:error, e} end + + e -> + unless params[:unreachable_since], do: Instances.set_unreachable(inbox) + Logger.metadata(activity: id, inbox: inbox) + Logger.error("Publisher failed to inbox #{inbox} #{inspect(e)}") + {:error, e} end end diff --git a/test/pleroma/web/activity_pub/publisher_test.exs b/test/pleroma/web/activity_pub/publisher_test.exs index bab25c8ab..7aa06a5c4 100644 --- a/test/pleroma/web/activity_pub/publisher_test.exs +++ b/test/pleroma/web/activity_pub/publisher_test.exs @@ -212,7 +212,8 @@ test "publish to url with with different ports" do actor = insert(:user) inbox = "http://404.site/users/nick1/inbox" - assert {:error, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + assert {:discard, _} = + Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) assert called(Instances.set_unreachable(inbox)) end