diff --git a/config/config.exs b/config/config.exs index 9fd10ca..457a182 100644 --- a/config/config.exs +++ b/config/config.exs @@ -34,6 +34,7 @@ config :who_need_help, e2e_routes: false, secure_cookies: false, rate_limit_policies: %{}, + tracking_presence_cleanup_grace_ms: 5_000, map_tile_url: "https://tile.openstreetmap.org/{z}/{x}/{y}.png", android_app_links: nil, web_push_public_key: nil, diff --git a/config/test.exs b/config/test.exs index 6b0fa68..6a04186 100644 --- a/config/test.exs +++ b/config/test.exs @@ -1,6 +1,7 @@ import Config config :who_need_help, :handover_secret, "isolated-test-handover-secret" +config :who_need_help, :tracking_presence_cleanup_grace_ms, 100 # Only in tests, remove the complexity from the password hashing algorithm config :bcrypt_elixir, :log_rounds, 1 diff --git a/lib/who_need_help/tracking_presence_cleanup.ex b/lib/who_need_help/tracking_presence_cleanup.ex index 22f066c..38cc54d 100644 --- a/lib/who_need_help/tracking_presence_cleanup.ex +++ b/lib/who_need_help/tracking_presence_cleanup.ex @@ -16,13 +16,40 @@ defmodule WhoNeedHelp.TrackingPresenceCleanup do end @impl GenServer - def init(_options), do: {:ok, %{}} + def init(_options) do + # Phoenix 1.8.9's locked browser client can wait up to 5 seconds between + # reconnect attempts. Waiting for that same interval prevents a serving + # node replacement from being mistaken for the user closing the last tab. + grace_ms = + Application.fetch_env!(:who_need_help, :tracking_presence_cleanup_grace_ms) + + if not is_integer(grace_ms) or grace_ms < 0 do + raise ArgumentError, + ":tracking_presence_cleanup_grace_ms must be a non-negative integer" + end + + {:ok, %{grace_ms: grace_ms}} + end @impl GenServer def handle_cast( {:maybe_stop, key, assignment_id, user_id, tracking_session_id}, state ) do + Process.send_after( + self(), + {:maybe_stop_after_grace, key, assignment_id, user_id, tracking_session_id}, + state.grace_ms + ) + + {:noreply, state} + end + + @impl GenServer + def handle_info( + {:maybe_stop_after_grace, key, assignment_id, user_id, tracking_session_id}, + state + ) do if Presence.get_by_key(Presence.tracking_topic(), key) == [] do Tracking.stop_browser_session(assignment_id, user_id, tracking_session_id) end diff --git a/test/who_need_help/mutual_aid_flow_test.exs b/test/who_need_help/mutual_aid_flow_test.exs index 955857e..c23001f 100644 --- a/test/who_need_help/mutual_aid_flow_test.exs +++ b/test/who_need_help/mutual_aid_flow_test.exs @@ -1085,6 +1085,64 @@ defmodule WhoNeedHelp.MutualAidFlowTest do refute Tracking.active_session?(context.helper_scope, assignment) end + test "browser tracking survives a replacement tab during the cleanup grace period", context do + {:ok, request} = Help.create_request(context.requester_scope, context.request_attrs) + {:ok, assignment} = Help.accept_request(context.helper_scope, request.id) + {:ok, session} = Tracking.start_session(context.helper_scope, assignment) + :ok = Tracking.subscribe(assignment.id) + + parent = self() + + first = + spawn(fn -> + {:ok, key} = + Presence.track_browser(self(), assignment.id, context.helper.id, session.id) + + send(parent, {:first_browser_ready, self(), key}) + + receive do + :close -> :ok + end + end) + + key = Presence.tracking_key(assignment.id, context.helper.id) + assert_receive {:first_browser_ready, ^first, ^key} + assert wait_for_presence_count(key, 1) + + first_ref = Process.monitor(first) + send(first, :close) + assert_receive {:DOWN, ^first_ref, :process, ^first, :normal} + assert wait_for_presence_count(key, 0) + + replacement = + spawn(fn -> + {:ok, ^key} = + Presence.track_browser(self(), assignment.id, context.helper.id, session.id) + + send(parent, {:replacement_browser_ready, self()}) + + receive do + :close -> :ok + end + end) + + assert_receive {:replacement_browser_ready, ^replacement} + assert wait_for_presence_count(key, 1) + + Process.sleep( + Application.fetch_env!(:who_need_help, :tracking_presence_cleanup_grace_ms) + 100 + ) + + assert Tracking.active_session?(context.helper_scope, assignment) + + replacement_ref = Process.monitor(replacement) + send(replacement, :close) + assert_receive {:DOWN, ^replacement_ref, :process, ^replacement, :normal} + assert_receive {:tracking_stopped, helper_id}, 1_000 + assert helper_id == context.helper.id + refute Tracking.active_session?(context.helper_scope, assignment) + end + defp wait_for_presence_count(key, expected, attempts \\ 100) defp wait_for_presence_count(_key, _expected, 0), do: false