From 14ff522b67d264adc30ec5aa8adbf8ec1c0b612b Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Wed, 20 Dec 2023 16:45:35 +0000 Subject: [PATCH] Add ratelimit backoff to HTTP get --- lib/pleroma/http/backoff.ex | 22 +++++++++++++++++----- test/pleroma/http/backoff_test.exs | 22 +++++++++++++++++++++- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/http/backoff.ex b/lib/pleroma/http/backoff.ex index 20edeae42..795b4f846 100644 --- a/lib/pleroma/http/backoff.ex +++ b/lib/pleroma/http/backoff.ex @@ -9,14 +9,24 @@ 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 -> - DateTime.utc_now() - |> Timex.shift(seconds: 5 * 60) + Logger.error("Rate limited, but couldn't find timestamp! Using default 5 minute backoff until #{default_5_minute_backoff}") + default_5_minute_backoff value -> - {:ok, stamp} = DateTime.from_iso8601(value) - stamp + 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 end end @@ -28,7 +38,8 @@ def get(url, headers \\ [], options \\ []) do # this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire # this is a very simple implementation, and can be improved upon! %{host: host} = URI.parse(url) - case @cachex.get(@backoff_cache, host) do + + case @cachex.get(@backoff_cache, host) do {:ok, nil} -> case HTTP.get(url, headers, options) do {:ok, env} -> @@ -49,6 +60,7 @@ def get(url, headers \\ [], options \\ []) do end _ -> + Logger.error("Request not made to #{host} because we are rate limited!") {:error, %Tesla.Env{status: 429, body: "Rate limited (internal backoff)"}} end end diff --git a/test/pleroma/http/backoff_test.exs b/test/pleroma/http/backoff_test.exs index e8a571e87..b50a4c458 100644 --- a/test/pleroma/http/backoff_test.exs +++ b/test/pleroma/http/backoff_test.exs @@ -9,6 +9,7 @@ test "should return {:ok, env} when not rate limited" do %Tesla.Env{url: "https://akkoma.dev/api/v1/instance"} -> {:ok, %Tesla.Env{status: 200, body: "ok"}} end) + assert {:ok, env} = Backoff.get("https://akkoma.dev/api/v1/instance") assert env.status == 200 end @@ -29,6 +30,25 @@ test "should insert a value into the cache when rate limited" do assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance") assert env.status == 429 assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev") - end + 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() + + Tesla.Mock.mock_global(fn + %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} -> + {:ok, + %Tesla.Env{ + status: 429, + body: "Rate limited", + headers: [{"x-ratelimit-reset", ten_minutes_from_now}] + }} + end) + + assert {:error, env} = Backoff.get("https://ratelimited.dev/api/v1/instance") + assert env.status == 429 + assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev") + end end end