From 46148c082522bcb4b4c87e0f7a5b3800269cc20c Mon Sep 17 00:00:00 2001
From: Oneric <oneric@oneric.stub>
Date: Tue, 7 Jan 2025 04:10:58 +0100
Subject: [PATCH] Don't return garbage on failed collection fetches

And for now treat partial fetches as a success, since for all
current users partial collection data is better than no data at all.

If an error occurred while fetching a page, this previously
returned a bogus {:ok, {:error, _}} success, causing the error
to be attached to the object as an reply list subsequently
leading to the whole post getting rejected during validation.

Also the pinned collection caller did not actually handle
the preexisting error case resulting in process crashes.
---
 lib/pleroma/collections/fetcher.ex           | 25 +++++++++++++-------
 lib/pleroma/web/activity_pub/activity_pub.ex | 22 ++++++++++-------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/lib/pleroma/collections/fetcher.ex b/lib/pleroma/collections/fetcher.ex
index 9ab883cc2..a4c3463a2 100644
--- a/lib/pleroma/collections/fetcher.ex
+++ b/lib/pleroma/collections/fetcher.ex
@@ -14,7 +14,7 @@ defmodule Akkoma.Collections.Fetcher do
   @spec fetch_collection(String.t() | map()) :: {:ok, [Pleroma.Object.t()]} | {:error, any()}
   def fetch_collection(ap_id) when is_binary(ap_id) do
     with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(ap_id) do
-      {:ok, objects_from_collection(page)}
+      partial_as_success(objects_from_collection(page))
     else
       e ->
         Logger.error("Could not fetch collection #{ap_id} - #{inspect(e)}")
@@ -24,9 +24,12 @@ def fetch_collection(ap_id) when is_binary(ap_id) do
 
   def fetch_collection(%{"type" => type} = page)
       when type in ["Collection", "OrderedCollection", "CollectionPage", "OrderedCollectionPage"] do
-    {:ok, objects_from_collection(page)}
+    partial_as_success(objects_from_collection(page))
   end
 
+  defp partial_as_success({:partial, items}), do: {:ok, items}
+  defp partial_as_success(res), do: res
+
   defp items_in_page(%{"type" => type, "orderedItems" => items})
        when is_list(items) and type in ["OrderedCollection", "OrderedCollectionPage"],
        do: items
@@ -53,11 +56,11 @@ defp objects_from_collection(%{"type" => type, "first" => %{"id" => id}})
     fetch_page_items(id)
   end
 
-  defp objects_from_collection(_page), do: []
+  defp objects_from_collection(_page), do: {:ok, []}
 
   defp fetch_page_items(id, items \\ []) do
     if Enum.count(items) >= Config.get([:activitypub, :max_collection_objects]) do
-      items
+      {:ok, items}
     else
       with {:ok, page} <- Fetcher.fetch_and_contain_remote_object_from_id(id) do
         objects = items_in_page(page)
@@ -65,18 +68,22 @@ defp fetch_page_items(id, items \\ []) do
         if Enum.count(objects) > 0 do
           maybe_next_page(page, items ++ objects)
         else
-          items
+          {:ok, items}
         end
       else
         {:error, :not_found} ->
-          items
+          {:ok, items}
 
         {:error, :forbidden} ->
-          items
+          {:ok, items}
 
         {:error, error} ->
           Logger.error("Could not fetch page #{id} - #{inspect(error)}")
-          {:error, error}
+
+          case items do
+            [] -> {:error, error}
+            _ -> {:partial, items}
+          end
       end
     end
   end
@@ -85,5 +92,5 @@ defp maybe_next_page(%{"next" => id}, items) when is_binary(id) do
     fetch_page_items(id, items)
   end
 
-  defp maybe_next_page(_, items), do: items
+  defp maybe_next_page(_, items), do: {:ok, items}
 end
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index 224767b80..746d18639 100644
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -1797,7 +1797,7 @@ def pin_data_from_featured_collection(%{
     else
       e ->
         Logger.error("Could not decode featured collection at fetch #{first}, #{inspect(e)}")
-        {:ok, %{}}
+        %{}
     end
   end
 
@@ -1807,14 +1807,18 @@ def pin_data_from_featured_collection(
         } = collection
       )
       when type in ["OrderedCollection", "Collection"] do
-    {:ok, objects} = Collections.Fetcher.fetch_collection(collection)
-
-    # Items can either be a map _or_ a string
-    objects
-    |> Map.new(fn
-      ap_id when is_binary(ap_id) -> {ap_id, NaiveDateTime.utc_now()}
-      %{"id" => object_ap_id} -> {object_ap_id, NaiveDateTime.utc_now()}
-    end)
+    with {:ok, objects} <- Collections.Fetcher.fetch_collection(collection) do
+      # Items can either be a map _or_ a string
+      objects
+      |> Map.new(fn
+        ap_id when is_binary(ap_id) -> {ap_id, NaiveDateTime.utc_now()}
+        %{"id" => object_ap_id} -> {object_ap_id, NaiveDateTime.utc_now()}
+      end)
+    else
+      e ->
+        Logger.warning("Failed to fetch featured collection #{collection}, #{inspect(e)}")
+        %{}
+    end
   end
 
   def pin_data_from_featured_collection(obj) do