fix(rich media): fix error handling when resource preview URL leads to empty parsed data

Closes #1279

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/graphql/resolvers/resource.ex

@@ -80,6 +80,8 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do
           }
         } = _resolution
       ) do
+    Logger.debug("Getting resource for group with username #{username}")
+
     with {:group, %Actor{id: group_id}} <- {:group, Actors.get_actor_by_name(username, :Group)},
          {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)},
          {:resource, %Resource{} = resource} <-
@@ -222,13 +224,8 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do
       {:ok, data} when is_map(data) ->
         {:ok, struct(Metadata, data)}
 
-      {:error, :invalid_parsed_data} ->
+      {:error, _error_type, _} ->
         {:error, dgettext("errors", "Unable to fetch resource details from this URL.")}
-
-      {:error, err} ->
-        Logger.warn("Error while fetching preview from #{inspect(resource_url)}")
-        Logger.debug(inspect(err))
-        {:error, :unknown_resource}
     end
   end
 

lib/service/rich_media/parser.ex

@@ -29,19 +29,19 @@ defmodule Mobilizon.Service.RichMedia.Parser do
 
   def parse(nil), do: {:error, "No URL provided"}
 
-  @spec parse(String.t()) :: {:ok, map()} | {:error, any()}
+  @spec parse(String.t()) :: {:ok, map()} | {:error, :http | :parsing | :unknown, any()}
   def parse(url) do
     case Cachex.fetch(:rich_media_cache, url, fn _ ->
            case parse_url(url) do
              {:ok, data} -> {:commit, data}
-             {:error, err} -> {:ignore, err}
+             {:error, error_type, error} -> {:ignore, {error_type, error}}
            end
          end) do
       {status, value} when status in [:ok, :commit] ->
         {:ok, value}
 
-      {_, err} ->
+      {_, {error_type, err}} ->
-        {:error, err}
+        {:error, error_type, err}
     end
   rescue
     e ->
@@ -56,7 +56,8 @@ defmodule Mobilizon.Service.RichMedia.Parser do
     get_filename_from_headers(response_headers) || get_filename_from_url(url)
   end
 
-  @spec parse_url(String.t(), Enum.t()) :: {:ok, map()} | {:error, any()}
+  @spec parse_url(String.t(), Enum.t()) ::
+          {:ok, map()} | {:error, :http | :parsing | :unknown, any()}
   defp parse_url(url, options \\ []) do
     user_agent = Keyword.get(options, :user_agent, default_user_agent(url))
     headers = [{"User-Agent", user_agent}]
@@ -64,13 +65,14 @@ defmodule Mobilizon.Service.RichMedia.Parser do
 
     try do
       with {:ok, _} <- prevent_local_address(url),
-           {:ok, %{body: body, status: code, headers: response_headers}}
+           {:fetch, {:ok, %{body: body, status: code, headers: response_headers}}}
            when code in 200..299 <-
-             RichMediaPreviewClient.get(
+             {:fetch,
-               url,
+              RichMediaPreviewClient.get(
-               headers: headers,
+                url,
-               opts: @options
+                headers: headers,
-             ),
+                opts: @options
+              )},
            {:is_html, _response_headers, true} <-
              {:is_html, response_headers, is_html(response_headers)} do
         body
@@ -87,17 +89,17 @@ defmodule Mobilizon.Service.RichMedia.Parser do
 
           {:ok, data}
 
-        {:ok, err} ->
+        {:fetch, {_, err}} ->
           Logger.debug("HTTP error: #{inspect(err)}")
-          {:error, "HTTP error: #{inspect(err)}"}
+          {:error, :http, err}
 
         {:error, err} ->
-          Logger.debug("HTTP error: #{inspect(err)}")
+          Logger.debug("Parsing error: #{inspect(err)}")
-          {:error, "HTTP error: #{inspect(err)}"}
+          {:error, :parsing, err}
       end
     rescue
       e ->
-        {:error, "Parsing error: #{inspect(e)} #{inspect(__STACKTRACE__)}"}
+        {:error, :unknown, "Parsing error: #{inspect(e)} #{inspect(__STACKTRACE__)}"}
     end
   end
 
@@ -228,7 +230,7 @@ defmodule Mobilizon.Service.RichMedia.Parser do
       check_parsed_data(data, html, false)
     else
       Logger.debug("Found metadata was invalid or incomplete: #{inspect(data)}")
-      {:error, :invalid_parsed_data}
+      {:error, :parsing, :invalid_parsed_data}
     end
   end
 
@@ -252,11 +254,11 @@ defmodule Mobilizon.Service.RichMedia.Parser do
              validate_ip(host) do
           {:ok, url}
         else
-          {:error, "Host violates local access rules"}
+          {:error, :local_address, "Host violates local access rules"}
         end
 
       _ ->
-        {:error, "Could not detect any host"}
+        {:error, :no_host, "Could not detect any host"}
     end
   end
 
@@ -302,6 +304,8 @@ defmodule Mobilizon.Service.RichMedia.Parser do
     {:ok, data}
   end
 
+  defp check_remote_picture_path({:error, _, _} = err), do: err
+
   defp check_remote_picture_path(data), do: {:ok, data}
 
   @spec format_url(String.t(), String.t()) :: String.t()

lib/service/rich_media/parsers/oembed_parser.ex

@@ -67,8 +67,8 @@ defmodule Mobilizon.Service.RichMedia.Parsers.OEmbed do
          {:ok, data} <- Jason.decode(json),
          data <-
            data
-           |> Map.new(fn {k, v} -> {String.to_existing_atom(k), String.trim(v)} end)
+           |> Map.new(fn {k, v} -> {k, if(is_binary(v), do: String.trim(v), else: v)} end)
-           |> Map.take(@oembed_allowed_attributes) do
+           |> Map.take(Enum.map(@oembed_allowed_attributes, &to_string/1)) do
       {:ok, data}
     end
   end