Merge branch 'fix_flaky_nil_provided_to_functions_who_expect_string' into 'develop'

Draft: Fix flaky test: default S3 uploader values make code fail

See merge request pleroma/pleroma!3700

changelog.d/card-image-description.add

@@ -0,0 +1 @@
+Include image description in status media cards

changelog.d/ffmpeg-limiter.add

@@ -0,0 +1 @@
+Framegrabs with ffmpeg will execute with a 5 second timeout and cache the URLs of failures with a TTL of 15 minutes to prevent excessive retries.

changelog.d/receiverworker-error-handling.fix

@@ -0,0 +1 @@
+ReceiverWorker: Make sure non-{:ok, _} is returned as {:error, …}

lib/pleroma/application.ex

@@ -156,6 +156,7 @@ defmodule Pleroma.Application do
       build_cachex("web_resp", limit: 2500),
       build_cachex("emoji_packs", expiration: emoji_packs_expiration(), limit: 10),
       build_cachex("failed_proxy_url", limit: 2500),
+      build_cachex("failed_media_helper_url", default_ttl: :timer.minutes(15), limit: 2_500),
       build_cachex("banned_urls", default_ttl: :timer.hours(24 * 30), limit: 5_000),
       build_cachex("chat_message_id_idempotency_key",
         expiration: chat_message_id_idempotency_key_expiration(),

lib/pleroma/helpers/media_helper.ex

@@ -12,6 +12,8 @@ defmodule Pleroma.Helpers.MediaHelper do
 
   require Logger
 
+  @cachex Pleroma.Config.get([:cachex, :provider], Cachex)
+
   def missing_dependencies do
     Enum.reduce([ffmpeg: "ffmpeg"], [], fn {sym, executable}, acc ->
       if Pleroma.Utils.command_available?(executable) do
@@ -43,29 +45,40 @@ defmodule Pleroma.Helpers.MediaHelper do
   @spec video_framegrab(String.t()) :: {:ok, binary()} | {:error, any()}
   def video_framegrab(url) do
     with executable when is_binary(executable) <- System.find_executable("ffmpeg"),
+         false <- @cachex.exists?(:failed_media_helper_cache, url),
          {:ok, env} <- HTTP.get(url, [], pool: :media),
          {:ok, pid} <- StringIO.open(env.body) do
       body_stream = IO.binstream(pid, 1)
 
-      result =
-        Exile.stream!(
-          [
-            executable,
-            "-i",
-            "pipe:0",
-            "-vframes",
-            "1",
-            "-f",
-            "mjpeg",
-            "pipe:1"
-          ],
-          input: body_stream,
-          ignore_epipe: true,
-          stderr: :disable
-        )
-        |> Enum.into(<<>>)
+      task =
+        Task.async(fn ->
+          Exile.stream!(
+            [
+              executable,
+              "-i",
+              "pipe:0",
+              "-vframes",
+              "1",
+              "-f",
+              "mjpeg",
+              "pipe:1"
+            ],
+            input: body_stream,
+            ignore_epipe: true,
+            stderr: :disable
+          )
+          |> Enum.into(<<>>)
+        end)
 
-      {:ok, result}
+      case Task.yield(task, 5_000) do
+        nil ->
+          Task.shutdown(task)
+          @cachex.put(:failed_media_helper_cache, url, nil)
+          {:error, {:ffmpeg, :timeout}}
+
+        result ->
+          {:ok, result}
+      end
     else
       nil -> {:error, {:ffmpeg, :command_not_found}}
       {:error, _} = error -> error

lib/pleroma/web/api_spec/schemas/status.ex

@@ -58,6 +58,10 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Status do
             format: :uri,
             description: "Preview thumbnail"
           },
+          image_description: %Schema{
+            type: :string,
+            description: "Alternate text that describes what is in the thumbnail"
+          },
           title: %Schema{type: :string, description: "Title of linked resource"},
           description: %Schema{type: :string, description: "Description of preview"}
         }

lib/pleroma/web/mastodon_api/views/status_view.ex

@@ -589,6 +589,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusView do
       provider_url: page_url_data.scheme <> "://" <> page_url_data.host,
       url: page_url,
       image: image_url,
+      image_description: rich_media["image:alt"] || "",
       title: rich_media["title"] || "",
       description: rich_media["description"] || "",
       pleroma: %{

lib/pleroma/workers/receiver_worker.ex

@@ -52,7 +52,8 @@ defmodule Pleroma.Workers.ReceiverWorker do
       {:error, {:reject, reason}} -> {:cancel, reason}
       {:signature, false} -> {:cancel, :invalid_signature}
       {:error, {:error, reason = "Object has been deleted"}} -> {:cancel, reason}
-      e -> e
+      {:error, _} = e -> e
+      e -> {:error, e}
     end
   end
 end

test/pleroma/web/activity_pub/activity_pub_test.exs

@@ -25,6 +25,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
   import Tesla.Mock
 
   setup do
+    clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+    clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
     mock(fn env -> apply(HttpRequestMock, :request, [env]) end)
 
     ConfigMock

test/pleroma/web/common_api_test.exs

@@ -36,6 +36,8 @@ defmodule Pleroma.Web.CommonAPITest do
   end
 
   setup_all do
+    clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+    clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
     Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
     :ok
   end

test/pleroma/web/mastodon_api/controllers/media_controller_test.exs

@@ -13,6 +13,9 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
 
+  setup do: clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+  setup do: clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
+
   describe "Upload media" do
     setup do: oauth_access(["write:media"])
 

test/pleroma/web/mastodon_api/controllers/status_controller_test.exs

@@ -27,6 +27,8 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
   setup do: clear_config([:instance, :allow_relay])
   setup do: clear_config([:mrf, :policies])
   setup do: clear_config([:mrf_keyword, :reject])
+  setup do: clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+  setup do: clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
 
   setup do
     Pleroma.UnstubbedConfigMock
@@ -1717,6 +1719,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
 
       card_data = %{
         "image" => "http://ia.media-imdb.com/images/rock.jpg",
+        "image_description" => "",
         "provider_name" => "example.com",
         "provider_url" => "https://example.com",
         "title" => "The Rock",
@@ -1770,6 +1773,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
                "title" => "Pleroma",
                "description" => "",
                "image" => nil,
+               "image_description" => "",
                "provider_name" => "example.com",
                "provider_url" => "https://example.com",
                "url" => "https://example.com/ogp-missing-data",

test/pleroma/web/mastodon_api/views/status_view_test.exs

@@ -773,6 +773,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusViewTest do
       page_url = "http://example.com"
 
       card = %{
+        "image:alt" => "Example image description",
         url: page_url,
         site_name: "Example site name",
         title: "Example website",
@@ -780,7 +781,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusViewTest do
         description: "Example description"
       }
 
-      %{provider_name: "example.com"} =
+      %{provider_name: "example.com", image_description: "Example image description"} =
         StatusView.render("card.json", %{page_url: page_url, rich_media: card})
     end
 

test/pleroma/web/pleroma_api/controllers/backup_controller_test.exs

@@ -10,7 +10,8 @@ defmodule Pleroma.Web.PleromaAPI.BackupControllerTest do
   alias Pleroma.Web.PleromaAPI.BackupView
 
   setup do
-    clear_config([Pleroma.Upload, :uploader])
+    clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+    clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
     clear_config([Backup, :limit_days])
 
     ConfigMock

test/pleroma/web/pleroma_api/controllers/chat_controller_test.exs

@@ -109,6 +109,9 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do
     end
 
     test "it works with an attachment", %{conn: conn, user: user} do
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
+
       file = %Plug.Upload{
         content_type: "image/jpeg",
         path: Path.absname("test/fixtures/image.jpg"),

test/pleroma/web/twitter_api/remote_follow_controller_test.exs

@@ -430,6 +430,8 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowControllerTest do
 
     test "with media proxy" do
       clear_config([:media_proxy, :enabled], true)
+      clear_config([Pleroma.Upload, :uploader], Pleroma.Uploaders.Local)
+      clear_config([Pleroma.Uploaders.Local, :uploads], "uploads")
 
       ConfigMock
       |> stub_with(Pleroma.Test.StaticConfig)