fix(tracking): tolerate replica reconnect grace
This commit is contained in:
parent
cbb5efb62a
commit
7b09becc09
|
|
@ -34,6 +34,7 @@ config :who_need_help,
|
||||||
e2e_routes: false,
|
e2e_routes: false,
|
||||||
secure_cookies: false,
|
secure_cookies: false,
|
||||||
rate_limit_policies: %{},
|
rate_limit_policies: %{},
|
||||||
|
tracking_presence_cleanup_grace_ms: 5_000,
|
||||||
map_tile_url: "https://tile.openstreetmap.org/{z}/{x}/{y}.png",
|
map_tile_url: "https://tile.openstreetmap.org/{z}/{x}/{y}.png",
|
||||||
android_app_links: nil,
|
android_app_links: nil,
|
||||||
web_push_public_key: nil,
|
web_push_public_key: nil,
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
import Config
|
import Config
|
||||||
|
|
||||||
config :who_need_help, :handover_secret, "isolated-test-handover-secret"
|
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
|
# Only in tests, remove the complexity from the password hashing algorithm
|
||||||
config :bcrypt_elixir, :log_rounds, 1
|
config :bcrypt_elixir, :log_rounds, 1
|
||||||
|
|
|
||||||
|
|
@ -16,13 +16,40 @@ defmodule WhoNeedHelp.TrackingPresenceCleanup do
|
||||||
end
|
end
|
||||||
|
|
||||||
@impl GenServer
|
@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
|
@impl GenServer
|
||||||
def handle_cast(
|
def handle_cast(
|
||||||
{:maybe_stop, key, assignment_id, user_id, tracking_session_id},
|
{:maybe_stop, key, assignment_id, user_id, tracking_session_id},
|
||||||
state
|
state
|
||||||
) do
|
) 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
|
if Presence.get_by_key(Presence.tracking_topic(), key) == [] do
|
||||||
Tracking.stop_browser_session(assignment_id, user_id, tracking_session_id)
|
Tracking.stop_browser_session(assignment_id, user_id, tracking_session_id)
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -1085,6 +1085,64 @@ defmodule WhoNeedHelp.MutualAidFlowTest do
|
||||||
refute Tracking.active_session?(context.helper_scope, assignment)
|
refute Tracking.active_session?(context.helper_scope, assignment)
|
||||||
end
|
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, attempts \\ 100)
|
||||||
|
|
||||||
defp wait_for_presence_count(_key, _expected, 0), do: false
|
defp wait_for_presence_count(_key, _expected, 0), do: false
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user