Reachability refactor

The result of Oban jobs determine the reachability status.

Publisher jobs will cancel themselves at execution time if the target server is now unreachable.

Receiving activities does not immediately mark a server as reachable, but creates a ReachabilityWorker job to validate.

A Cron will execute daily to test all unreachable servers.

changelog.d/reachabililty.change

@@ -0,0 +1 @@
+Improved the logic of how we determine if a server is unreachable.

config/config.exs

@@ -194,7 +194,6 @@ config :pleroma, :instance,
   account_approval_required: false,
   federating: true,
   federation_incoming_replies_max_depth: 100,
-  federation_reachability_timeout_days: 7,
   allow_relay: true,
   public: true,
   quarantined_instances: [],
@@ -603,7 +602,8 @@ config :pleroma, Oban,
   crontab: [
     {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker},
     {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker},
-    {"*/10 * * * *", Pleroma.Workers.Cron.AppCleanupWorker}
+    {"*/10 * * * *", Pleroma.Workers.Cron.AppCleanupWorker},
+    {"0 0 * * *", Pleroma.Workers.Cron.ScheduleReachabilityWorker}
   ]
 
 config :pleroma, Pleroma.Formatter,

lib/pleroma/instances.ex

@@ -15,25 +15,7 @@ defmodule Pleroma.Instances do
 
   defdelegate set_unreachable(url_or_host, unreachable_since \\ nil), to: Instance
 
-  defdelegate get_consistently_unreachable, to: Instance
+  defdelegate get_unreachable, to: Instance
-
-  def set_consistently_unreachable(url_or_host),
-    do: set_unreachable(url_or_host, reachability_datetime_threshold())
-
-  def reachability_datetime_threshold do
-    federation_reachability_timeout_days =
-      Pleroma.Config.get([:instance, :federation_reachability_timeout_days], 0)
-
-    if federation_reachability_timeout_days > 0 do
-      NaiveDateTime.add(
-        NaiveDateTime.utc_now(),
-        -federation_reachability_timeout_days * 24 * 3600,
-        :second
-      )
-    else
-      ~N[0000-01-01 00:00:00]
-    end
-  end
 
   def host(url_or_host) when is_binary(url_or_host) do
     if url_or_host =~ ~r/^http/i do

lib/pleroma/instances/instance.ex

@@ -51,7 +51,7 @@ defmodule Pleroma.Instances.Instance do
     |> cast(params, [:software_name, :software_version, :software_repository])
   end
 
-  def filter_reachable([]), do: %{}
+  def filter_reachable([]), do: []
 
   def filter_reachable(urls_or_hosts) when is_list(urls_or_hosts) do
     hosts =
@@ -68,19 +68,15 @@ defmodule Pleroma.Instances.Instance do
       )
       |> Map.new(& &1)
 
-    reachability_datetime_threshold = Instances.reachability_datetime_threshold()
-
     for entry <- Enum.filter(urls_or_hosts, &is_binary/1) do
       host = host(entry)
       unreachable_since = unreachable_since_by_host[host]
 
-      if !unreachable_since ||
+      if is_nil(unreachable_since) do
-           NaiveDateTime.compare(unreachable_since, reachability_datetime_threshold) == :gt do
+        entry
-        {entry, unreachable_since}
       end
     end
     |> Enum.filter(& &1)
-    |> Map.new(& &1)
   end
 
   def reachable?(url_or_host) when is_binary(url_or_host) do
@@ -88,7 +84,7 @@ defmodule Pleroma.Instances.Instance do
       from(i in Instance,
         where:
           i.host == ^host(url_or_host) and
-            i.unreachable_since <= ^Instances.reachability_datetime_threshold(),
+            i.unreachable_since <= ^NaiveDateTime.utc_now(),
         select: true
       )
     )
@@ -132,11 +128,9 @@ defmodule Pleroma.Instances.Instance do
 
   def set_unreachable(_, _), do: {:error, nil}
 
-  def get_consistently_unreachable do
+  def get_unreachable do
-    reachability_datetime_threshold = Instances.reachability_datetime_threshold()
-
     from(i in Instance,
-      where: ^reachability_datetime_threshold > i.unreachable_since,
+      where: not is_nil(i.unreachable_since),
       order_by: i.unreachable_since,
       select: {i.host, i.unreachable_since}
     )

lib/pleroma/object/fetcher.ex

@@ -4,7 +4,6 @@
 
 defmodule Pleroma.Object.Fetcher do
   alias Pleroma.HTTP
-  alias Pleroma.Instances
   alias Pleroma.Maps
   alias Pleroma.Object
   alias Pleroma.Object.Containment
@@ -152,10 +151,6 @@ defmodule Pleroma.Object.Fetcher do
          {:ok, body} <- get_object(id),
          {:ok, data} <- safe_json_decode(body),
          :ok <- Containment.contain_origin_from_id(id, data) do
-      if not Instances.reachable?(id) do
-        Instances.set_reachable(id)
-      end
-
       {:ok, data}
     else
       {:scheme, _} ->

lib/pleroma/web/activity_pub/activity_pub_controller.ex

@@ -53,7 +53,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
   )
 
   plug(:log_inbox_metadata when action in [:inbox])
-  plug(:set_requester_reachable when action in [:inbox])
   plug(:relay_active? when action in [:relay])
 
   defp relay_active?(conn, _) do
@@ -520,15 +519,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
     |> json(dgettext("errors", "error"))
   end
 
-  defp set_requester_reachable(%Plug.Conn{} = conn, _) do
-    with actor <- conn.params["actor"],
-         true <- is_binary(actor) do
-      Pleroma.Instances.set_reachable(actor)
-    end
-
-    conn
-  end
-
   defp log_inbox_metadata(%{params: %{"actor" => actor, "type" => type}} = conn, _) do
     Logger.metadata(actor: actor, type: type)
     conn

lib/pleroma/web/activity_pub/publisher.ex

@@ -148,17 +148,9 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
                {"digest", p.digest}
              ]
            ) do
-      if not is_nil(p.unreachable_since) do
-        Instances.set_reachable(p.inbox)
-      end
-
       result
     else
       {_post_result, %{status: code} = response} = e ->
-        if is_nil(p.unreachable_since) do
-          Instances.set_unreachable(p.inbox)
-        end
-
         Logger.metadata(activity: p.activity_id, inbox: p.inbox, status: code)
         Logger.error("Publisher failed to inbox #{p.inbox} with status #{code}")
 
@@ -179,10 +171,6 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
         connection_pool_snooze()
 
       e ->
-        if is_nil(p.unreachable_since) do
-          Instances.set_unreachable(p.inbox)
-        end
-
         Logger.metadata(activity: p.activity_id, inbox: p.inbox)
         Logger.error("Publisher failed to inbox #{p.inbox} #{inspect(e)}")
         {:error, e}
@@ -308,7 +296,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
 
     Repo.checkout(fn ->
       Enum.each(inboxes, fn inboxes ->
-        Enum.each(inboxes, fn {inbox, unreachable_since} ->
+        Enum.each(inboxes, fn inbox ->
           %User{ap_id: ap_id} = Enum.find(recipients, fn actor -> actor.inbox == inbox end)
 
           # Get all the recipients on the same host and add them to cc. Otherwise, a remote
@@ -318,8 +306,7 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
           __MODULE__.enqueue_one(%{
             inbox: inbox,
             cc: cc,
-            activity_id: activity.id,
+            activity_id: activity.id
-            unreachable_since: unreachable_since
           })
         end)
       end)
@@ -352,12 +339,11 @@ defmodule Pleroma.Web.ActivityPub.Publisher do
     |> Enum.each(fn {inboxes, priority} ->
       inboxes
       |> Instances.filter_reachable()
-      |> Enum.each(fn {inbox, unreachable_since} ->
+      |> Enum.each(fn inbox ->
         __MODULE__.enqueue_one(
           %{
             inbox: inbox,
-            activity_id: activity.id,
+            activity_id: activity.id
-            unreachable_since: unreachable_since
           },
           priority: priority
         )

lib/pleroma/web/pleroma_api/controllers/instances_controller.ex

@@ -13,7 +13,7 @@ defmodule Pleroma.Web.PleromaAPI.InstancesController do
 
   def show(conn, _params) do
     unreachable =
-      Instances.get_consistently_unreachable()
+      Instances.get_unreachable()
       |> Map.new(fn {host, date} -> {host, to_string(date)} end)
 
     json(conn, %{"unreachable" => unreachable})

lib/pleroma/workers/cron/schedule_reachability_worker.ex

@@ -0,0 +1,33 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Workers.Cron.ScheduleReachabilityWorker do
+  use Oban.Worker,
+    queue: :background,
+    max_attempts: 2
+
+  alias Pleroma.Instances
+  alias Pleroma.Repo
+
+  @impl true
+  def perform(_job) do
+    unreachable_servers = Instances.get_unreachable()
+
+    jobs =
+      unreachable_servers
+      |> Enum.map(fn {domain, _} ->
+        Pleroma.Workers.ReachabilityWorker.new(%{"domain" => domain})
+      end)
+
+    case Repo.transaction(fn ->
+           Enum.each(jobs, &Oban.insert/1)
+         end) do
+      {:ok, _} ->
+        :ok
+
+      {:error, reason} ->
+        {:error, reason}
+    end
+  end
+end

lib/pleroma/workers/publisher_worker.ex

@@ -4,6 +4,7 @@
 
 defmodule Pleroma.Workers.PublisherWorker do
   alias Pleroma.Activity
+  alias Pleroma.Instances
   alias Pleroma.Web.Federator
 
   use Oban.Worker, queue: :federator_outgoing, max_attempts: 5
@@ -14,9 +15,30 @@ defmodule Pleroma.Workers.PublisherWorker do
     Federator.perform(:publish, activity)
   end
 
-  def perform(%Job{args: %{"op" => "publish_one", "params" => params}}) do
+  def perform(%Job{args: %{"op" => "publish_one", "params" => params}} = job) do
     params = Map.new(params, fn {k, v} -> {String.to_atom(k), v} end)
-    Federator.perform(:publish_one, params)
+
+    # Cancel / skip the job if this server believed to be unreachable now
+    if not Instances.reachable?(params.inbox) do
+      {:cancel, :unreachable}
+    else
+      case Federator.perform(:publish_one, params) do
+        {:ok, _} ->
+          :ok
+
+        {:error, _} = error ->
+          # Only mark as unreachable on final failure
+          if job.attempt == job.max_attempts do
+            Instances.set_unreachable(params.inbox)
+          end
+
+          error
+
+        error ->
+          # Unexpected error, may have been client side
+          error
+      end
+    end
   end
 
   @impl true

lib/pleroma/workers/reachability_worker.ex

@@ -0,0 +1,31 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Workers.ReachabilityWorker do
+  use Oban.Worker,
+    queue: :background,
+    max_attempts: 3,
+    unique: [period: :infinity, states: [:available, :scheduled]]
+
+  alias Pleroma.HTTP
+  alias Pleroma.Instances
+
+  @impl true
+  def perform(%Oban.Job{args: %{"domain" => domain}}) do
+    case HTTP.get("https://#{domain}/.well-known/nodeinfo") do
+      {:ok, %{status: status}} when status in 200..299 ->
+        Instances.set_reachable("https://#{domain}")
+        :ok
+
+      {:ok, %{status: _status}} ->
+        {:error, :unreachable}
+
+      {:error, _} = error ->
+        error
+    end
+  end
+
+  @impl true
+  def timeout(_job), do: :timer.seconds(5)
+end

lib/pleroma/workers/receiver_worker.ex

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Workers.ReceiverWorker do
+  alias Pleroma.Instances
   alias Pleroma.Signature
   alias Pleroma.User
   alias Pleroma.Web.Federator
@@ -37,6 +38,11 @@ defmodule Pleroma.Workers.ReceiverWorker do
          {:ok, _public_key} <- Signature.refetch_public_key(conn_data),
          {:signature, true} <- {:signature, Signature.validate_signature(conn_data)},
          {:ok, res} <- Federator.perform(:incoming_ap_doc, params) do
+      unless Instances.reachable?(params["actor"]) do
+        domain = URI.parse(params["actor"]).host
+        Oban.insert(Pleroma.Workers.ReachabilityWorker.new(%{"domain" => domain}))
+      end
+
       {:ok, res}
     else
       e -> process_errors(e)
@@ -45,6 +51,11 @@ defmodule Pleroma.Workers.ReceiverWorker do
 
   def perform(%Job{args: %{"op" => "incoming_ap_doc", "params" => params}}) do
     with {:ok, res} <- Federator.perform(:incoming_ap_doc, params) do
+      unless Instances.reachable?(params["actor"]) do
+        domain = URI.parse(params["actor"]).host
+        Oban.insert(Pleroma.Workers.ReachabilityWorker.new(%{"domain" => domain}))
+      end
+
       {:ok, res}
     else
       e -> process_errors(e)

lib/pleroma/workers/remote_fetcher_worker.ex

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Workers.RemoteFetcherWorker do
+  alias Pleroma.Instances
   alias Pleroma.Object.Fetcher
 
   use Oban.Worker, queue: :background, unique: [period: :infinity]
@@ -11,6 +12,15 @@ defmodule Pleroma.Workers.RemoteFetcherWorker do
   def perform(%Job{args: %{"op" => "fetch_remote", "id" => id} = args}) do
     case Fetcher.fetch_object_from_id(id, depth: args["depth"]) do
       {:ok, _object} ->
+        # Mark the server as reachable since we successfully fetched an object
+        case URI.parse(id) do
+          %URI{host: host} when not is_nil(host) ->
+            Instances.set_reachable("https://#{host}")
+
+          _ ->
+            :ok
+        end
+
         :ok
 
       {:allowed_depth, false} ->

test/pleroma/instances/instance_test.exs

@@ -3,7 +3,6 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Instances.InstanceTest do
-  alias Pleroma.Instances
   alias Pleroma.Instances.Instance
   alias Pleroma.Repo
   alias Pleroma.Tests.ObanHelpers
@@ -14,8 +13,6 @@ defmodule Pleroma.Instances.InstanceTest do
   import ExUnit.CaptureLog
   import Pleroma.Factory
 
-  setup_all do: clear_config([:instance, :federation_reachability_timeout_days], 1)
-
   describe "set_reachable/1" do
     test "clears `unreachable_since` of existing matching Instance record having non-nil `unreachable_since`" do
       unreachable_since = NaiveDateTime.to_iso8601(NaiveDateTime.utc_now())
@@ -145,7 +142,11 @@ defmodule Pleroma.Instances.InstanceTest do
     end
 
     test "Doesn't scrapes unreachable instances" do
-      instance = insert(:instance, unreachable_since: Instances.reachability_datetime_threshold())
+      instance =
+        insert(:instance,
+          unreachable_since: NaiveDateTime.utc_now() |> NaiveDateTime.add(-:timer.hours(24))
+        )
+
       url = "https://" <> instance.host
 
       assert capture_log(fn -> assert nil == Instance.get_or_update_favicon(URI.parse(url)) end) =~

test/pleroma/instances_test.exs

@@ -7,73 +7,40 @@ defmodule Pleroma.InstancesTest do
 
   use Pleroma.DataCase
 
-  setup_all do: clear_config([:instance, :federation_reachability_timeout_days], 1)
-
   describe "reachable?/1" do
     test "returns `true` for host / url with unknown reachability status" do
       assert Instances.reachable?("unknown.site")
       assert Instances.reachable?("http://unknown.site")
     end
-
-    test "returns `false` for host / url marked unreachable for at least `reachability_datetime_threshold()`" do
-      host = "consistently-unreachable.name"
-      Instances.set_consistently_unreachable(host)
-
-      refute Instances.reachable?(host)
-      refute Instances.reachable?("http://#{host}/path")
-    end
-
-    test "returns `true` for host / url marked unreachable for less than `reachability_datetime_threshold()`" do
-      url = "http://eventually-unreachable.name/path"
-
-      Instances.set_unreachable(url)
-
-      assert Instances.reachable?(url)
-      assert Instances.reachable?(URI.parse(url).host)
-    end
-
-    test "raises FunctionClauseError exception on non-binary input" do
-      assert_raise FunctionClauseError, fn -> Instances.reachable?(nil) end
-      assert_raise FunctionClauseError, fn -> Instances.reachable?(1) end
-    end
   end
 
   describe "filter_reachable/1" do
     setup do
-      host = "consistently-unreachable.name"
+      unreachable_host = "consistently-unreachable.name"
-      url1 = "http://eventually-unreachable.com/path"
+      reachable_host = "http://domain.com/path"
-      url2 = "http://domain.com/path"
 
-      Instances.set_consistently_unreachable(host)
+      Instances.set_unreachable(unreachable_host)
-      Instances.set_unreachable(url1)
 
-      result = Instances.filter_reachable([host, url1, url2, nil])
+      result = Instances.filter_reachable([unreachable_host, reachable_host, nil])
-      %{result: result, url1: url1, url2: url2}
+      %{result: result, reachable_host: reachable_host, unreachable_host: unreachable_host}
     end
 
-    test "returns a map with keys containing 'not marked consistently unreachable' elements of supplied list",
+    test "returns a list of only reachable elements",
-         %{result: result, url1: url1, url2: url2} do
+         %{result: result, reachable_host: reachable_host} do
-      assert is_map(result)
+      assert is_list(result)
-      assert Enum.sort([url1, url2]) == result |> Map.keys() |> Enum.sort()
+      assert [reachable_host] == result
     end
 
-    test "returns a map with `unreachable_since` values for keys",
+    test "returns an empty list when provided no data" do
-         %{result: result, url1: url1, url2: url2} do
+      assert [] == Instances.filter_reachable([])
-      assert is_map(result)
+      assert [] == Instances.filter_reachable([nil])
-      assert %NaiveDateTime{} = result[url1]
-      assert is_nil(result[url2])
-    end
-
-    test "returns an empty map for empty list or list containing no hosts / url" do
-      assert %{} == Instances.filter_reachable([])
-      assert %{} == Instances.filter_reachable([nil])
     end
   end
 
   describe "set_reachable/1" do
     test "sets unreachable url or host reachable" do
       host = "domain.com"
-      Instances.set_consistently_unreachable(host)
+      Instances.set_unreachable(host)
       refute Instances.reachable?(host)
 
       Instances.set_reachable(host)
@@ -102,23 +69,4 @@ defmodule Pleroma.InstancesTest do
       assert {:error, _} = Instances.set_unreachable(1)
     end
   end
-
-  describe "set_consistently_unreachable/1" do
-    test "sets reachable url or host unreachable" do
-      url = "http://domain.com?q="
-      assert Instances.reachable?(url)
-
-      Instances.set_consistently_unreachable(url)
-      refute Instances.reachable?(url)
-    end
-
-    test "keeps unreachable url or host unreachable" do
-      host = "site.name"
-      Instances.set_consistently_unreachable(host)
-      refute Instances.reachable?(host)
-
-      Instances.set_consistently_unreachable(host)
-      refute Instances.reachable?(host)
-    end
-  end
 end

test/pleroma/object/fetcher_test.exs

@@ -6,7 +6,6 @@ defmodule Pleroma.Object.FetcherTest do
   use Pleroma.DataCase
 
   alias Pleroma.Activity
-  alias Pleroma.Instances
   alias Pleroma.Object
   alias Pleroma.Object.Fetcher
   alias Pleroma.Web.ActivityPub.ObjectValidator
@@ -250,17 +249,6 @@ defmodule Pleroma.Object.FetcherTest do
       result = Fetcher.fetch_object_from_id("https://example.com/objects/no-content-type")
       assert {:fetch, {:error, nil}} = result
     end
-
-    test "it resets instance reachability on successful fetch" do
-      id = "http://mastodon.example.org/@admin/99541947525187367"
-      Instances.set_consistently_unreachable(id)
-      refute Instances.reachable?(id)
-
-      {:ok, _object} =
-        Fetcher.fetch_object_from_id("http://mastodon.example.org/@admin/99541947525187367")
-
-      assert Instances.reachable?(id)
-    end
   end
 
   describe "implementation quirks" do

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -8,7 +8,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
 
   alias Pleroma.Activity
   alias Pleroma.Delivery
-  alias Pleroma.Instances
   alias Pleroma.Object
   alias Pleroma.Tests.ObanHelpers
   alias Pleroma.User
@@ -601,23 +600,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       assert Activity.get_by_ap_id(data["id"])
     end
 
-    test "it clears `unreachable` federation status of the sender", %{conn: conn} do
-      data = File.read!("test/fixtures/mastodon-post-activity.json") |> Jason.decode!()
-
-      sender_url = data["actor"]
-      Instances.set_consistently_unreachable(sender_url)
-      refute Instances.reachable?(sender_url)
-
-      conn =
-        conn
-        |> assign(:valid_signature, true)
-        |> put_req_header("content-type", "application/activity+json")
-        |> post("/inbox", data)
-
-      assert "ok" == json_response(conn, 200)
-      assert Instances.reachable?(sender_url)
-    end
-
     test "accept follow activity", %{conn: conn} do
       clear_config([:instance, :federating], true)
       relay = Relay.get_actor()
@@ -1108,24 +1090,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       assert response(conn, 200) =~ note_object.data["content"]
     end
 
-    test "it clears `unreachable` federation status of the sender", %{conn: conn, data: data} do
-      user = insert(:user)
-      data = Map.put(data, "bcc", [user.ap_id])
-
-      sender_host = URI.parse(data["actor"]).host
-      Instances.set_consistently_unreachable(sender_host)
-      refute Instances.reachable?(sender_host)
-
-      conn =
-        conn
-        |> assign(:valid_signature, true)
-        |> put_req_header("content-type", "application/activity+json")
-        |> post("/users/#{user.nickname}/inbox", data)
-
-      assert "ok" == json_response(conn, 200)
-      assert Instances.reachable?(sender_host)
-    end
-
     test "it removes all follower collections but actor's", %{conn: conn} do
       [actor, recipient] = insert_pair(:user)
 

test/pleroma/web/activity_pub/publisher_test.exs

@@ -6,13 +6,11 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
   use Oban.Testing, repo: Pleroma.Repo
   use Pleroma.Web.ConnCase
 
-  import ExUnit.CaptureLog
   import Pleroma.Factory
   import Tesla.Mock
   import Mock
 
   alias Pleroma.Activity
-  alias Pleroma.Instances
   alias Pleroma.Object
   alias Pleroma.Tests.ObanHelpers
   alias Pleroma.Web.ActivityPub.Publisher
@@ -167,115 +165,6 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do
                })
                |> Publisher.publish_one()
     end
-
-    test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is set",
-                   Instances,
-                   [:passthrough],
-                   [] do
-      _actor = insert(:user)
-      inbox = "http://200.site/users/nick1/inbox"
-      activity = insert(:note_activity)
-
-      assert {:ok, _} =
-               Publisher.prepare_one(%{
-                 inbox: inbox,
-                 activity_id: activity.id,
-                 unreachable_since: NaiveDateTime.utc_now() |> NaiveDateTime.to_string()
-               })
-               |> Publisher.publish_one()
-
-      assert called(Instances.set_reachable(inbox))
-    end
-
-    test_with_mock "does NOT call `Instances.set_reachable` on successful federation if `unreachable_since` is nil",
-                   Instances,
-                   [:passthrough],
-                   [] do
-      _actor = insert(:user)
-      inbox = "http://200.site/users/nick1/inbox"
-      activity = insert(:note_activity)
-
-      assert {:ok, _} =
-               Publisher.prepare_one(%{
-                 inbox: inbox,
-                 activity_id: activity.id,
-                 unreachable_since: nil
-               })
-               |> Publisher.publish_one()
-
-      refute called(Instances.set_reachable(inbox))
-    end
-
-    test_with_mock "calls `Instances.set_unreachable` on target inbox on non-2xx HTTP response code",
-                   Instances,
-                   [:passthrough],
-                   [] do
-      _actor = insert(:user)
-      inbox = "http://404.site/users/nick1/inbox"
-      activity = insert(:note_activity)
-
-      assert {:cancel, _} =
-               Publisher.prepare_one(%{inbox: inbox, activity_id: activity.id})
-               |> Publisher.publish_one()
-
-      assert called(Instances.set_unreachable(inbox))
-    end
-
-    test_with_mock "it calls `Instances.set_unreachable` on target inbox on request error of any kind",
-                   Instances,
-                   [:passthrough],
-                   [] do
-      _actor = insert(:user)
-      inbox = "http://connrefused.site/users/nick1/inbox"
-      activity = insert(:note_activity)
-
-      assert capture_log(fn ->
-               assert {:error, _} =
-                        Publisher.prepare_one(%{
-                          inbox: inbox,
-                          activity_id: activity.id
-                        })
-                        |> Publisher.publish_one()
-             end) =~ "connrefused"
-
-      assert called(Instances.set_unreachable(inbox))
-    end
-
-    test_with_mock "does NOT call `Instances.set_unreachable` if target is reachable",
-                   Instances,
-                   [:passthrough],
-                   [] do
-      _actor = insert(:user)
-      inbox = "http://200.site/users/nick1/inbox"
-      activity = insert(:note_activity)
-
-      assert {:ok, _} =
-               Publisher.prepare_one(%{inbox: inbox, activity_id: activity.id})
-               |> Publisher.publish_one()
-
-      refute called(Instances.set_unreachable(inbox))
-    end
-
-    test_with_mock "does NOT call `Instances.set_unreachable` if target instance has non-nil `unreachable_since`",
-                   Instances,
-                   [:passthrough],
-                   [] do
-      _actor = insert(:user)
-      inbox = "http://connrefused.site/users/nick1/inbox"
-      activity = insert(:note_activity)
-
-      assert capture_log(fn ->
-               assert {:error, _} =
-                        Publisher.prepare_one(%{
-                          inbox: inbox,
-                          activity_id: activity.id,
-                          unreachable_since: NaiveDateTime.utc_now() |> NaiveDateTime.to_string()
-                        })
-                        |> Publisher.publish_one()
-             end) =~ "connrefused"
-
-      refute called(Instances.set_unreachable(inbox))
-    end
   end
 
   describe "publish/2" do

test/pleroma/web/federator_test.exs

@@ -126,22 +126,17 @@ defmodule Pleroma.Web.FederatorTest do
         inbox: inbox2
       })
 
-      dt = NaiveDateTime.utc_now()
+      Instances.set_unreachable(URI.parse(inbox2).host)
-      Instances.set_unreachable(inbox1, dt)
-
-      Instances.set_consistently_unreachable(URI.parse(inbox2).host)
 
       {:ok, _activity} =
         CommonAPI.post(user, %{status: "HI @nick1@domain.com, @nick2@domain2.com!"})
 
-      expected_dt = NaiveDateTime.to_iso8601(dt)
-
       ObanHelpers.perform(all_enqueued(worker: PublisherWorker))
 
       assert ObanHelpers.member?(
                %{
                  "op" => "publish_one",
-                 "params" => %{"inbox" => inbox1, "unreachable_since" => expected_dt}
+                 "params" => %{"inbox" => inbox1}
                },
                all_enqueued(worker: PublisherWorker)
              )

test/pleroma/web/pleroma_api/controllers/instances_controller_test.exs

@@ -7,16 +7,11 @@ defmodule Pleroma.Web.PleromaApi.InstancesControllerTest do
 
   alias Pleroma.Instances
 
-  setup_all do: clear_config([:instance, :federation_reachability_timeout_days], 1)
-
   setup do
     constant = "http://consistently-unreachable.name/"
-    eventual = "http://eventually-unreachable.com/path"
 
     {:ok, %Pleroma.Instances.Instance{unreachable_since: constant_unreachable}} =
-      Instances.set_consistently_unreachable(constant)
+      Instances.set_unreachable(constant)
-
-    _eventual_unreachable = Instances.set_unreachable(eventual)
 
     %{constant_unreachable: constant_unreachable, constant: constant}
   end

test/pleroma/workers/cron/schedule_reachability_worker_test.exs

@@ -0,0 +1,52 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Workers.Cron.ScheduleReachabilityWorkerTest do
+  use Pleroma.DataCase, async: true
+  use Oban.Testing, repo: Pleroma.Repo
+
+  alias Pleroma.Instances
+  alias Pleroma.Workers.Cron.ScheduleReachabilityWorker
+
+  describe "perform/1" do
+    test "schedules reachability checks for unreachable servers" do
+      # Mark some servers as unreachable
+      Instances.set_unreachable("https://example.com")
+      Instances.set_unreachable("https://test.com")
+      Instances.set_unreachable("https://another.com")
+
+      # Verify they are marked as unreachable
+      refute Instances.reachable?("https://example.com")
+      refute Instances.reachable?("https://test.com")
+      refute Instances.reachable?("https://another.com")
+
+      # Run the worker
+      assert :ok = ScheduleReachabilityWorker.perform(%Oban.Job{})
+
+      # Verify ReachabilityWorker jobs were scheduled for each server
+      # Note: domains in get_unreachable/0 are without the https:// prefix
+      assert_enqueued(
+        worker: Pleroma.Workers.ReachabilityWorker,
+        args: %{"domain" => "example.com"}
+      )
+
+      assert_enqueued(
+        worker: Pleroma.Workers.ReachabilityWorker,
+        args: %{"domain" => "test.com"}
+      )
+
+      assert_enqueued(
+        worker: Pleroma.Workers.ReachabilityWorker,
+        args: %{"domain" => "another.com"}
+      )
+    end
+
+    test "handles empty list of unreachable servers" do
+      # Ensure no servers are marked as unreachable
+      assert [] = Instances.get_unreachable()
+      assert :ok = ScheduleReachabilityWorker.perform(%Oban.Job{})
+      refute_enqueued(worker: Pleroma.Workers.ReachabilityWorker)
+    end
+  end
+end

test/pleroma/workers/publisher_worker_test.exs

@@ -7,7 +7,9 @@ defmodule Pleroma.Workers.PublisherWorkerTest do
   use Oban.Testing, repo: Pleroma.Repo
 
   import Pleroma.Factory
+  import Mock
 
+  alias Pleroma.Instances
   alias Pleroma.Object
   alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.Builder
@@ -37,4 +39,85 @@ defmodule Pleroma.Workers.PublisherWorkerTest do
       assert {:ok, %Oban.Job{priority: 0}} = Federator.publish(post)
     end
   end
+
+  describe "Server reachability:" do
+    setup do
+      user = insert(:user)
+      remote_user = insert(:user, local: false, inbox: "https://example.com/inbox")
+      {:ok, _, _} = Pleroma.User.follow(remote_user, user)
+      {:ok, activity} = CommonAPI.post(user, %{status: "Test post"})
+
+      %{
+        user: user,
+        remote_user: remote_user,
+        activity: activity
+      }
+    end
+
+    test "marks server as unreachable only on final failure", %{activity: activity} do
+      with_mock Pleroma.Web.Federator,
+        perform: fn :publish_one, _params -> {:error, :connection_error} end do
+        # First attempt
+        job = %Oban.Job{
+          args: %{
+            "op" => "publish_one",
+            "params" => %{
+              "inbox" => "https://example.com/inbox",
+              "activity_id" => activity.id
+            }
+          },
+          attempt: 1,
+          max_attempts: 5
+        }
+
+        assert {:error, :connection_error} = Pleroma.Workers.PublisherWorker.perform(job)
+        assert Instances.reachable?("https://example.com/inbox")
+
+        # Final attempt
+        job = %{job | attempt: 5}
+        assert {:error, :connection_error} = Pleroma.Workers.PublisherWorker.perform(job)
+        refute Instances.reachable?("https://example.com/inbox")
+      end
+    end
+
+    test "does not mark server as unreachable on successful publish", %{activity: activity} do
+      with_mock Pleroma.Web.Federator,
+        perform: fn :publish_one, _params -> {:ok, %{status: 200}} end do
+        job = %Oban.Job{
+          args: %{
+            "op" => "publish_one",
+            "params" => %{
+              "inbox" => "https://example.com/inbox",
+              "activity_id" => activity.id
+            }
+          },
+          attempt: 1,
+          max_attempts: 5
+        }
+
+        assert :ok = Pleroma.Workers.PublisherWorker.perform(job)
+        assert Instances.reachable?("https://example.com/inbox")
+      end
+    end
+
+    test "cancels job if server is unreachable", %{activity: activity} do
+      # First mark the server as unreachable
+      Instances.set_unreachable("https://example.com/inbox")
+      refute Instances.reachable?("https://example.com/inbox")
+
+      job = %Oban.Job{
+        args: %{
+          "op" => "publish_one",
+          "params" => %{
+            "inbox" => "https://example.com/inbox",
+            "activity_id" => activity.id
+          }
+        },
+        attempt: 1,
+        max_attempts: 5
+      }
+
+      assert {:cancel, :unreachable} = Pleroma.Workers.PublisherWorker.perform(job)
+    end
+  end
 end

test/pleroma/workers/receiver_worker_test.exs

@@ -3,13 +3,14 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Workers.ReceiverWorkerTest do
-  use Pleroma.DataCase
+  use Pleroma.DataCase, async: true
   use Oban.Testing, repo: Pleroma.Repo
 
   import Mock
   import Pleroma.Factory
 
   alias Pleroma.User
+  alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.Federator
   alias Pleroma.Workers.ReceiverWorker
 
@@ -243,4 +244,62 @@ defmodule Pleroma.Workers.ReceiverWorkerTest do
 
     assert {:cancel, _} = ReceiverWorker.perform(oban_job)
   end
+
+  describe "Server reachability:" do
+    setup do
+      user = insert(:user)
+      remote_user = insert(:user, local: false, ap_id: "https://example.com/users/remote")
+      {:ok, _, _} = Pleroma.User.follow(user, remote_user)
+      {:ok, activity} = CommonAPI.post(remote_user, %{status: "Test post"})
+
+      %{
+        user: user,
+        remote_user: remote_user,
+        activity: activity
+      }
+    end
+
+    test "schedules ReachabilityWorker if host is unreachable", %{activity: activity} do
+      with_mocks [
+        {Pleroma.Web.ActivityPub.Transmogrifier, [],
+         [handle_incoming: fn _ -> {:ok, activity} end]},
+        {Pleroma.Instances, [], [reachable?: fn _ -> false end]},
+        {Pleroma.Web.Federator, [], [perform: fn :incoming_ap_doc, _params -> {:ok, nil} end]}
+      ] do
+        job = %Oban.Job{
+          args: %{
+            "op" => "incoming_ap_doc",
+            "params" => activity.data
+          }
+        }
+
+        Pleroma.Workers.ReceiverWorker.perform(job)
+
+        assert_enqueued(
+          worker: Pleroma.Workers.ReachabilityWorker,
+          args: %{"domain" => "example.com"}
+        )
+      end
+    end
+
+    test "does not schedule ReachabilityWorker if host is reachable", %{activity: activity} do
+      with_mocks [
+        {Pleroma.Web.ActivityPub.Transmogrifier, [],
+         [handle_incoming: fn _ -> {:ok, activity} end]},
+        {Pleroma.Instances, [], [reachable?: fn _ -> true end]},
+        {Pleroma.Web.Federator, [], [perform: fn :incoming_ap_doc, _params -> {:ok, nil} end]}
+      ] do
+        job = %Oban.Job{
+          args: %{
+            "op" => "incoming_ap_doc",
+            "params" => activity.data
+          }
+        }
+
+        Pleroma.Workers.ReceiverWorker.perform(job)
+
+        refute_enqueued(worker: Pleroma.Workers.ReachabilityWorker)
+      end
+    end
+  end
 end

test/pleroma/workers/remote_fetcher_worker_test.exs

@@ -3,7 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Workers.RemoteFetcherWorkerTest do
-  use Pleroma.DataCase
+  use Pleroma.DataCase, async: true
   use Oban.Testing, repo: Pleroma.Repo
 
   alias Pleroma.Workers.RemoteFetcherWorker