Respect 503 responses and backoff

CHANGELOG.md

@@ -35,7 +35,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Issue leading to Mastodon bot accounts being rejected
 - Scope misdetection of remote posts resulting from not recognising
   JSON-LD-compacted forms of public scope; affected e.g. federation with bovine
-- Ratelimits encountered when fetching objects are now respected; 429 responses will cause a backoff when we get one.
+- Ratelimits encountered when fetching objects are now respected;
+  429 and 503 responses will cause a backoff when we get one.
 
 ## Removed
 - ActivityPub Client-To-Server write API endpoints have been disabled;

lib/pleroma/http/backoff.ex

@@ -5,38 +5,68 @@ defmodule Pleroma.HTTP.Backoff do
   @cachex Pleroma.Config.get([:cachex, :provider], Cachex)
   @backoff_cache :http_backoff_cache
 
-  defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
-    # figure out from the 429 response when we can make the next request
-    # mastodon uses the x-ratelimit-reset header, so we will use that!
-    # other servers may not, so we'll default to 5 minutes from now if we can't find it
-    default_5_minute_backoff =
-      DateTime.utc_now()
-      |> Timex.shift(seconds: 5 * 60)
-
-    case Enum.find_value(headers, fn {"x-ratelimit-reset", value} -> value end) do
-      nil ->
-        Logger.error(
-          "Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}"
-        )
-
-        default_5_minute_backoff
-
-      value ->
-        with {:ok, stamp, _} <- DateTime.from_iso8601(value) do
-          Logger.error("Rate limited until #{stamp}")
-          stamp
-        else
-          _ ->
-            Logger.error(
-              "Rate limited, but couldn't parse timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}"
-            )
-
-            default_5_minute_backoff
-        end
+  defp maybe_parse_date(value) when is_binary(value) do
+    case DateTime.from_iso8601(value) do
+      {:ok, stamp, _} -> stamp
+      _ -> value
     end
   end
 
-  defp next_backoff_timestamp(_), do: DateTime.utc_now() |> Timex.shift(seconds: 5 * 60)
+  defp maybe_parse_date(value), do: value
+
+  defp maybe_parse_offset(value) when is_binary(value) do
+    case Integer.parse(value) do
+      {offset, _} -> DateTime.add(DateTime.utc_now(), offset, :second)
+      _ -> value
+    end
+  end
+
+  defp maybe_parse_offset(value), do: value
+
+  defp date_or_nil(value) do
+    case value do
+      %DateTime{} -> value
+      _ -> nil
+    end
+  end
+
+  defp parse_reset_header(headers, headername) do
+    :proplists.get_value(headername, headers, nil)
+    |> maybe_parse_date()
+    |> maybe_parse_offset()
+    |> date_or_nil()
+  end
+
+  defp default_backoff() do
+    DateTime.utc_now()
+    |> Timex.shift(seconds: 5 * 60)
+  end
+
+  defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
+    # figure out from a 429 or 503 response when we can make the next request
+    # mastodon uses the x-ratelimit-reset header, so we will prefer that, others Retry-After
+    # or something else entirely. If we can't find a header, default to 5 minutes from now
+    header_reset =
+      parse_reset_header(headers, "x-ratelimit-reset") ||
+        parse_reset_header(headers, "retry-after")
+
+    case header_reset do
+      nil ->
+        defback = default_backoff()
+
+        Logger.error(
+          "Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{defback}"
+        )
+
+        defback
+
+      value ->
+        Logger.error("Rate limited until #{value}")
+        value
+    end
+  end
+
+  defp next_backoff_timestamp(_), do: default_backoff()
 
   def get(url, headers \\ [], options \\ []) do
     # this acts as a single throughput for all GET requests
@@ -50,7 +80,7 @@ def get(url, headers \\ [], options \\ []) do
         case HTTP.get(url, headers, options) do
           {:ok, env} ->
             case env.status do
-              429 ->
+              status when status in [429, 503] ->
                 Logger.error("Rate limited on #{host}! Backing off...")
                 timestamp = next_backoff_timestamp(env)
                 ttl = Timex.diff(timestamp, DateTime.utc_now(), :seconds)

test/pleroma/http/backoff_test.exs

@@ -20,7 +20,7 @@ test "should return {:error, env} when rate limited" do
       assert {:error, :ratelimit} = Backoff.get("https://akkoma.dev/api/v1/instance")
     end
 
-    test "should insert a value into the cache when rate limited" do
+    test "should insert a value into the cache when rate limited via 429" do
       Tesla.Mock.mock_global(fn
         %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
           {:ok, %Tesla.Env{status: 429, body: "Rate limited"}}
@@ -30,6 +30,16 @@ test "should insert a value into the cache when rate limited" do
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
     end
 
+    test "should insert a value into the cache when remote sends 503" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://unavailable.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 503, body: "Temporarily unavailable"}}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://unavailable.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "unavailable.dev")
+    end
+
     test "should parse the value of x-ratelimit-reset, if present" do
       ten_minutes_from_now =
         DateTime.utc_now() |> Timex.shift(minutes: 10) |> DateTime.to_iso8601()
@@ -47,5 +57,22 @@ test "should parse the value of x-ratelimit-reset, if present" do
       assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
       assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
     end
+
+    test "should parse the value of retry-after, if present" do
+      ten_minutes_offset = 600
+
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://unavailable.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 503,
+             body: "Temporarily unavailable",
+             headers: [{"retry-after", "#{ten_minutes_offset}"}]
+           }}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://unavailable.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "unavailable.dev")
+    end
   end
 end