From 19691389b92e617f1edad7d4e3168fe917d0a675 Mon Sep 17 00:00:00 2001
From: rinpatch <rinpatch@sdf.org>
Date: Wed, 2 Sep 2020 14:21:28 +0300
Subject: [PATCH] Rich media: Add failure tracking

---
 CHANGELOG.md                                |  3 ++
 config/config.exs                           |  1 +
 config/description.exs                      |  7 ++++
 docs/configuration/cheatsheet.md            |  1 +
 lib/pleroma/web/rich_media/parser.ex        | 56 +++++++++++++++++------------
 test/web/rich_media/aws_signed_url_test.exs |  2 +-
 6 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07bc6d77c..4424c060f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## unreleased-patch - ???
 
+### Added
+- Rich media failure tracking (along with `:failure_backoff` option)
+
 ### Fixed
 - Mastodon API: Search parameter `following` now correctly returns the followings rather than the followers
 
diff --git a/config/config.exs b/config/config.exs
index ea8869d48..ed37b93c0 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -412,6 +412,7 @@ config :pleroma, :rich_media,
     Pleroma.Web.RichMedia.Parsers.TwitterCard,
     Pleroma.Web.RichMedia.Parsers.OEmbed
   ],
+  failure_backoff: 60_000,
   ttl_setters: [Pleroma.Web.RichMedia.Parser.TTL.AwsSignedUrl]
 
 config :pleroma, :media_proxy,
diff --git a/config/description.exs b/config/description.exs
index 29a657333..5e08ba109 100644
--- a/config/description.exs
+++ b/config/description.exs
@@ -2385,6 +2385,13 @@ config :pleroma, :config_description, [
         suggestions: [
           Pleroma.Web.RichMedia.Parser.TTL.AwsSignedUrl
         ]
+      },
+      %{
+        key: :failure_backoff,
+        type: :integer,
+        description:
+          "Amount of milliseconds after request failure, during which the request will not be retried.",
+        suggestions: [60_000]
       }
     ]
   },
diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md
index 2f440adf4..a9a650fab 100644
--- a/docs/configuration/cheatsheet.md
+++ b/docs/configuration/cheatsheet.md
@@ -361,6 +361,7 @@ config :pleroma, Pleroma.Web.MediaProxy.Invalidation.Http,
 * `ignore_hosts`: list of hosts which will be ignored by the metadata parser. For example `["accounts.google.com", "xss.website"]`, defaults to `[]`.
 * `ignore_tld`: list TLDs (top-level domains) which will ignore for parse metadata. default is ["local", "localdomain", "lan"].
 * `parsers`: list of Rich Media parsers.
+* `failure_backoff`: Amount of milliseconds after request failure, during which the request will not be retried.
 
 ## HTTP server
 
diff --git a/lib/pleroma/web/rich_media/parser.ex b/lib/pleroma/web/rich_media/parser.ex
index e9aa2dd03..e98c743ca 100644
--- a/lib/pleroma/web/rich_media/parser.ex
+++ b/lib/pleroma/web/rich_media/parser.ex
@@ -17,14 +17,25 @@ defmodule Pleroma.Web.RichMedia.Parser do
   else
     @spec parse(String.t()) :: {:ok, map()} | {:error, any()}
     def parse(url) do
-      Cachex.fetch!(:rich_media_cache, url, fn _ ->
-        with {:ok, data} <- parse_url(url) do
-          {:commit, {:ok, data}}
-        else
-          error -> {:ignore, error}
-        end
-      end)
-      |> set_ttl_based_on_image(url)
+      with {:ok, data} <- get_cached_or_parse(url),
+           {:ok, _} <- set_ttl_based_on_image(data, url) do
+        {:ok, data}
+      else
+        error ->
+          Logger.error(fn -> "Rich media error: #{inspect(error)}" end)
+      end
+    end
+
+    defp get_cached_or_parse(url) do
+      case Cachex.fetch!(:rich_media_cache, url, fn _ -> {:commit, parse_url(url)} end) do
+        {:ok, _data} = res ->
+          res
+
+        {:error, _} = e ->
+          ttl = Pleroma.Config.get([:rich_media, :failure_backoff], 60_000)
+          Cachex.expire(:rich_media_cache, url, ttl)
+          e
+      end
     end
   end
 
@@ -50,22 +61,21 @@ defmodule Pleroma.Web.RichMedia.Parser do
       config :pleroma, :rich_media,
         ttl_setters: [MyModule]
   """
-  @spec set_ttl_based_on_image({:ok, map()} | {:error, any()}, String.t()) ::
-          {:ok, map()} | {:error, any()}
-  def set_ttl_based_on_image({:ok, data}, url) do
-    with {:ok, nil} <- Cachex.ttl(:rich_media_cache, url),
-         {:ok, ttl} when is_number(ttl) <- get_ttl_from_image(data, url) do
-      Cachex.expire_at(:rich_media_cache, url, ttl * 1000)
-      {:ok, data}
-    else
-      _ ->
-        {:ok, data}
-    end
-  end
+  @spec set_ttl_based_on_image(map(), String.t()) ::
+          {:ok, Integer.t() | :noop} | {:error, :no_key}
+  def set_ttl_based_on_image(data, url) do
+    case get_ttl_from_image(data, url) do
+      {:ok, ttl} when is_number(ttl) ->
+        ttl = ttl * 1000
 
-  def set_ttl_based_on_image({:error, _} = error, _) do
-    Logger.error("parsing error: #{inspect(error)}")
-    error
+        case Cachex.expire_at(:rich_media_cache, url, ttl) do
+          {:ok, true} -> {:ok, ttl}
+          {:ok, false} -> {:error, :no_key}
+        end
+
+      _ ->
+        {:ok, :noop}
+    end
   end
 
   defp get_ttl_from_image(data, url) do
diff --git a/test/web/rich_media/aws_signed_url_test.exs b/test/web/rich_media/aws_signed_url_test.exs
index a21f3c935..1ceae1a31 100644
--- a/test/web/rich_media/aws_signed_url_test.exs
+++ b/test/web/rich_media/aws_signed_url_test.exs
@@ -55,7 +55,7 @@ defmodule Pleroma.Web.RichMedia.TTL.AwsSignedUrlTest do
 
     Cachex.put(:rich_media_cache, url, metadata)
 
-    Pleroma.Web.RichMedia.Parser.set_ttl_based_on_image({:ok, metadata}, url)
+    Pleroma.Web.RichMedia.Parser.set_ttl_based_on_image(metadata, url)
 
     {:ok, cache_ttl} = Cachex.ttl(:rich_media_cache, url)