diff --git a/CHANGELOG.md b/CHANGELOG.md index 257dc4adb..6ab5f6183 100644 --- a/CHANGELOG.md +++ b/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; diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex index dac05c971..63562bacb 100644 --- a/lib/pleroma/http/backoff.ex +++ b/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) diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs index 62419eb5c..717e7a140 100644 --- a/test/pleroma/http/backoff_test.exs +++ b/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