Private
Public Access
Expire presence so users can't get stuck online (#77)
Presence was a per-user refcount in Redis, decremented only when a connection's cleanup ran. Anything that ended a connection without cleanup (a crash, an unclean restart, a laptop asleep with the page frozen) leaked its count forever, so those users never showed offline. Production had 19 counts for 2 live connections. Presence is now one expiring entry per connection (90s TTL) for global, per-room and focus presence, in new presence:v2:* keys. Clients renew theirs by sending any frame, which the existing 30s ping/pong already does. The server closes a human connection that sends nothing for 90s (code 4408), which catches a frozen page whose browser still answers transport pings. Bot connections can't be expected to answer pings, so the server renews theirs itself while the handler is alive. An expiry has no disconnect event to announce it, so a sweeper on each worker (one per 15s overall, via a Redis lock) prunes expired entries and broadcasts the offline transition. The old presence:* hashes are deleted at startup. Tests cover the expiry store, the sweeper, and the websocket cases. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
ksmithandClaude Sonnet 5.5 committed
1 parent
7ff7f2ec0e
commit
dda85a2b78
11 files changed
+843
-97
No files matched your search
@@ -0,0 +1,155 @@
|
||||
import os
|
||||
import time
|
||||
import uuid
|
||||
|
||||
import pytest_asyncio
|
||||
from redis.asyncio import Redis
|
||||
|
||||
from app.ws.expiring_presence import ExpiringPresenceSet
|
||||
|
||||
|
||||
class Clock:
|
||||
def __init__(self) -> None:
|
||||
self.now = time.time()
|
||||
|
||||
def __call__(self) -> float:
|
||||
return self.now
|
||||
|
||||
def advance(self, seconds: float) -> None:
|
||||
self.now += seconds
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def redis_client():
|
||||
client = Redis.from_url(os.environ["REDIS_URL"], decode_responses=True)
|
||||
yield client
|
||||
await client.aclose()
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def setup(redis_client):
|
||||
clock = Clock()
|
||||
store = ExpiringPresenceSet(redis_client, ttl_seconds=90, clock=clock)
|
||||
key = f"presence:v2:test:{uuid.uuid4()}"
|
||||
yield store, clock, key
|
||||
await redis_client.delete(key)
|
||||
|
||||
|
||||
async def test_first_connection_is_a_transition_and_a_second_is_not(setup):
|
||||
store, _, key = setup
|
||||
user = uuid.uuid4()
|
||||
assert await store.add(key, user, "tab1") is True
|
||||
assert await store.add(key, user, "tab2") is False
|
||||
assert await store.live_user_ids(key) == {user}
|
||||
|
||||
|
||||
async def test_renewing_a_live_entry_is_not_a_transition(setup):
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
clock.advance(30)
|
||||
assert await store.add(key, user, "tab1") is False
|
||||
|
||||
|
||||
async def test_entry_stops_counting_after_the_ttl(setup):
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
clock.advance(89)
|
||||
assert await store.is_live(key, user) is True
|
||||
clock.advance(2)
|
||||
assert await store.is_live(key, user) is False
|
||||
assert await store.live_user_ids(key) == set()
|
||||
|
||||
|
||||
async def test_renewal_keeps_a_connection_alive_past_the_original_expiry(setup):
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
for _ in range(5):
|
||||
clock.advance(60)
|
||||
await store.add(key, user, "tab1")
|
||||
assert await store.is_live(key, user) is True
|
||||
|
||||
|
||||
async def test_coming_back_after_expiry_is_a_transition_again(setup):
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
clock.advance(200)
|
||||
assert await store.add(key, user, "tab1") is True
|
||||
|
||||
|
||||
async def test_one_live_connection_keeps_the_user_online_when_another_expires(setup):
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "old-tab")
|
||||
clock.advance(60)
|
||||
await store.add(key, user, "new-tab")
|
||||
clock.advance(60) # old-tab is now 120s stale, new-tab 60s
|
||||
assert await store.is_live(key, user) is True
|
||||
|
||||
|
||||
async def test_remove_reports_only_the_last_connection_leaving(setup):
|
||||
store, _, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
await store.add(key, user, "tab2")
|
||||
assert await store.remove(key, user, "tab1") is False
|
||||
assert await store.remove(key, user, "tab2") is True
|
||||
|
||||
|
||||
async def test_remove_of_an_entry_that_is_already_gone_reports_nothing(setup):
|
||||
# Whoever removed it first (e.g. prune) already reported the transition.
|
||||
store, _, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
await store.remove(key, user, "tab1")
|
||||
assert await store.remove(key, user, "tab1") is False
|
||||
|
||||
|
||||
async def test_remove_of_an_expired_but_unpruned_entry_still_reports_offline(setup):
|
||||
# Nobody has been told this user went offline yet (no prune has run), so
|
||||
# their eventual clean close is the one place left to report it.
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
clock.advance(200)
|
||||
assert await store.remove(key, user, "tab1") is True
|
||||
|
||||
|
||||
async def test_users_are_independent(setup):
|
||||
store, _, key = setup
|
||||
alice, bob = uuid.uuid4(), uuid.uuid4()
|
||||
await store.add(key, alice, "a")
|
||||
await store.add(key, bob, "b")
|
||||
await store.remove(key, alice, "a")
|
||||
assert await store.live_user_ids(key) == {bob}
|
||||
|
||||
|
||||
async def test_prune_reports_users_who_lost_their_last_live_entry(setup):
|
||||
store, clock, key = setup
|
||||
gone, still_here = uuid.uuid4(), uuid.uuid4()
|
||||
await store.add(key, gone, "tab1")
|
||||
await store.add(key, still_here, "old")
|
||||
clock.advance(60)
|
||||
await store.add(key, still_here, "new")
|
||||
clock.advance(40) # gone/tab1 and still_here/old are expired; still_here/new is not
|
||||
assert await store.prune(key) == {gone}
|
||||
|
||||
|
||||
async def test_prune_is_idempotent_and_removes_what_it_reports(setup):
|
||||
store, clock, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
clock.advance(200)
|
||||
assert await store.prune(key) == {user}
|
||||
assert await store.prune(key) == set()
|
||||
|
||||
|
||||
async def test_prune_with_nothing_expired_does_nothing(setup):
|
||||
store, _, key = setup
|
||||
user = uuid.uuid4()
|
||||
await store.add(key, user, "tab1")
|
||||
assert await store.prune(key) == set()
|
||||
assert await store.is_live(key, user) is True
|
||||
@@ -130,3 +130,108 @@ def test_online_users_endpoint_respects_appear_offline(ws_client):
|
||||
|
||||
online = ws_client.get("/api/users/online").json()
|
||||
assert alice["id"] not in online
|
||||
|
||||
|
||||
# --- #77: liveness -----------------------------------------------------------
|
||||
# A connection that stops answering the app-level heartbeat is dropped by the
|
||||
# server, and a connection that does answer keeps its presence entry renewed.
|
||||
# Timings are shrunk so nothing here waits on the real 30s/90s values.
|
||||
|
||||
|
||||
def _fast_heartbeat(monkeypatch):
|
||||
monkeypatch.setattr("app.ws.chat.WS_PING_INTERVAL_SECONDS", 0.05)
|
||||
monkeypatch.setattr("app.ws.chat.WS_DEAD_AFTER_SECONDS", 0.2)
|
||||
monkeypatch.setattr("app.ws.chat.PRESENCE_REFRESH_MIN_INTERVAL_SECONDS", 0)
|
||||
|
||||
|
||||
def test_unresponsive_client_is_disconnected_by_the_server(ws_client, monkeypatch):
|
||||
# The frozen-page / sleeping-laptop case: the transport stays open (the
|
||||
# browser answers protocol pings itself) but the page never replies to
|
||||
# the app-level ping.
|
||||
from starlette.websockets import WebSocketDisconnect
|
||||
|
||||
_fast_heartbeat(monkeypatch)
|
||||
_register_ws(ws_client, _unique("alice"))
|
||||
|
||||
with ws_client.websocket_connect("/ws/chat") as ws:
|
||||
pings = 0
|
||||
try:
|
||||
while True:
|
||||
assert ws.receive_json() == {"type": "ping"}
|
||||
pings += 1
|
||||
assert pings < 100, "server never gave up on a silent client"
|
||||
except WebSocketDisconnect as exc:
|
||||
assert exc.code == 4408
|
||||
assert pings >= 2
|
||||
|
||||
|
||||
def test_client_that_answers_pings_is_not_disconnected(ws_client, monkeypatch):
|
||||
_fast_heartbeat(monkeypatch)
|
||||
_register_ws(ws_client, _unique("alice"))
|
||||
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||
|
||||
with ws_client.websocket_connect("/ws/chat") as ws:
|
||||
# Far longer than WS_DEAD_AFTER_SECONDS in total.
|
||||
for _ in range(12):
|
||||
assert ws.receive_json() == {"type": "ping"}
|
||||
ws.send_json({"type": "pong"})
|
||||
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
while (msg := ws.receive_json())["type"] == "ping":
|
||||
pass
|
||||
assert msg == {"type": "joined", "room_id": room["id"]}
|
||||
|
||||
|
||||
def test_bot_connection_is_not_dropped_for_ignoring_pings(ws_client_factory, monkeypatch):
|
||||
# API-token clients aren't expected to speak the app heartbeat.
|
||||
_fast_heartbeat(monkeypatch)
|
||||
ws_client = ws_client_factory()
|
||||
admin = _register_ws(ws_client, _unique("admin"))
|
||||
|
||||
async def _promote():
|
||||
from app.models import User
|
||||
|
||||
async with ws_client.session_factory() as session:
|
||||
user = await session.get(User, uuid.UUID(admin["id"]))
|
||||
user.is_site_admin = True
|
||||
await session.commit()
|
||||
|
||||
ws_client.portal.call(_promote)
|
||||
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
|
||||
bot = ws_client.post("/api/admin/bots", json={"username": _unique("bot")}).json()
|
||||
token = ws_client.post(
|
||||
f"/api/admin/bots/{bot['id']}/tokens", json={"scopes": ["read:messages", "write:messages"]}
|
||||
).json()["token"]
|
||||
ws_client.post(f"/api/rooms/{room['id']}/join", headers={"Authorization": f"Bearer {token}"})
|
||||
|
||||
with ws_client.websocket_connect("/ws/chat", headers={"Authorization": f"Bearer {token}"}) as ws:
|
||||
for _ in range(8): # ~0.4s of silence, twice WS_DEAD_AFTER_SECONDS
|
||||
assert ws.receive_json() == {"type": "ping"}
|
||||
ws.send_json({"type": "join", "room_id": room["id"]})
|
||||
while (msg := ws.receive_json())["type"] == "ping":
|
||||
pass
|
||||
assert msg["type"] == "joined"
|
||||
|
||||
|
||||
def test_presence_entry_is_renewed_while_the_client_answers(ws_client, monkeypatch):
|
||||
import time
|
||||
|
||||
_fast_heartbeat(monkeypatch)
|
||||
alice = _register_ws(ws_client, _unique("alice"))
|
||||
|
||||
async def _scores():
|
||||
redis = ws_client.app.state.redis
|
||||
return await redis.zrange("presence:v2:global", 0, -1, withscores=True)
|
||||
|
||||
with ws_client.websocket_connect("/ws/chat") as ws:
|
||||
# Wait for the first ping so the connect-time entry exists.
|
||||
assert ws.receive_json() == {"type": "ping"}
|
||||
first = [s for m, s in ws_client.portal.call(_scores) if m.startswith(alice["id"])]
|
||||
assert len(first) == 1
|
||||
|
||||
time.sleep(0.1)
|
||||
ws.send_json({"type": "pong"})
|
||||
# Any later frame arriving back proves the pong was processed.
|
||||
while ws.receive_json()["type"] != "ping":
|
||||
pass
|
||||
second = [s for m, s in ws_client.portal.call(_scores) if m.startswith(alice["id"])]
|
||||
assert second and second[0] > first[0]
|
||||
@@ -0,0 +1,188 @@
|
||||
import os
|
||||
import time
|
||||
import uuid
|
||||
|
||||
import pytest_asyncio
|
||||
from redis.asyncio import Redis
|
||||
|
||||
from app.schemas.room import RoomCreate
|
||||
from app.schemas.user import UserCreate
|
||||
from app.services.auth_service import register_user
|
||||
from app.services.room_service import create_room, find_or_create_dm
|
||||
from app.ws.expiring_presence import ExpiringPresenceSet
|
||||
from app.ws.global_presence import GlobalPresence
|
||||
from app.ws.presence_sweeper import SWEEP_LOCK_KEY, purge_legacy_presence, sweep_once
|
||||
|
||||
|
||||
class Clock:
|
||||
def __init__(self) -> None:
|
||||
self.now = time.time()
|
||||
|
||||
def __call__(self) -> float:
|
||||
return self.now
|
||||
|
||||
def advance(self, seconds: float) -> None:
|
||||
self.now += seconds
|
||||
|
||||
|
||||
class FakeBroadcaster:
|
||||
def __init__(self) -> None:
|
||||
self.room_events: list[tuple[uuid.UUID, dict]] = []
|
||||
self.user_events: list[tuple[uuid.UUID, dict]] = []
|
||||
|
||||
async def publish(self, room_id, payload):
|
||||
self.room_events.append((room_id, payload))
|
||||
|
||||
async def publish_to_user(self, user_id, payload):
|
||||
self.user_events.append((user_id, payload))
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def redis_client():
|
||||
client = Redis.from_url(os.environ["REDIS_URL"], decode_responses=True)
|
||||
await client.delete(GlobalPresence.KEY, SWEEP_LOCK_KEY)
|
||||
yield client
|
||||
await client.delete(GlobalPresence.KEY, SWEEP_LOCK_KEY)
|
||||
await client.aclose()
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
async def world(redis_client, db_session):
|
||||
clock = Clock()
|
||||
presence = GlobalPresence(redis_client, ExpiringPresenceSet(redis_client, ttl_seconds=90, clock=clock))
|
||||
broadcaster = FakeBroadcaster()
|
||||
|
||||
class _Factory:
|
||||
def __call__(self):
|
||||
return self
|
||||
|
||||
async def __aenter__(self):
|
||||
return db_session
|
||||
|
||||
async def __aexit__(self, *exc):
|
||||
return False
|
||||
|
||||
async def make_user(prefix: str):
|
||||
name = f"{prefix}-{uuid.uuid4().hex[:8]}"
|
||||
return await register_user(
|
||||
db_session, UserCreate(username=name, email=f"{name}@example.com", password="password123")
|
||||
)
|
||||
|
||||
async def sweep(**kwargs):
|
||||
return await sweep_once(redis_client, presence, broadcaster, _Factory(), **kwargs)
|
||||
|
||||
return clock, presence, broadcaster, make_user, sweep
|
||||
|
||||
|
||||
async def test_sweep_announces_a_user_whose_connection_expired(world, db_session):
|
||||
clock, presence, broadcaster, make_user, sweep = world
|
||||
alice, bob = await make_user("alice"), await make_user("bob")
|
||||
room = await create_room(db_session, alice.id, RoomCreate(name=f"r-{uuid.uuid4().hex[:8]}"))
|
||||
dm = await find_or_create_dm(db_session, alice.id, bob.id)
|
||||
|
||||
await presence.connect(alice.id, "tab1")
|
||||
clock.advance(200)
|
||||
|
||||
assert await sweep(lock_seconds=None) == {alice.id}
|
||||
|
||||
room_ids = {rid for rid, p in broadcaster.room_events if p["type"] == "member_updated"}
|
||||
assert {room.id, dm.id} <= room_ids
|
||||
assert (bob.id, {"type": "dm_presence_update", "user_id": str(alice.id), "status": "offline"}) in (
|
||||
broadcaster.user_events
|
||||
)
|
||||
assert await presence.is_online(alice.id) is False
|
||||
|
||||
|
||||
async def test_sweep_announces_each_expiry_only_once(world, db_session):
|
||||
clock, presence, broadcaster, make_user, sweep = world
|
||||
alice = await make_user("alice")
|
||||
await create_room(db_session, alice.id, RoomCreate(name=f"r-{uuid.uuid4().hex[:8]}"))
|
||||
await presence.connect(alice.id, "tab1")
|
||||
clock.advance(200)
|
||||
|
||||
await sweep(lock_seconds=None)
|
||||
sent = len(broadcaster.room_events)
|
||||
assert sent > 0
|
||||
assert await sweep(lock_seconds=None) == set()
|
||||
assert len(broadcaster.room_events) == sent
|
||||
|
||||
|
||||
async def test_sweep_stays_quiet_while_the_user_has_another_live_connection(world, db_session):
|
||||
clock, presence, broadcaster, make_user, sweep = world
|
||||
alice = await make_user("alice")
|
||||
await create_room(db_session, alice.id, RoomCreate(name=f"r-{uuid.uuid4().hex[:8]}"))
|
||||
await presence.connect(alice.id, "old-tab")
|
||||
clock.advance(60)
|
||||
await presence.connect(alice.id, "new-tab")
|
||||
clock.advance(60)
|
||||
|
||||
assert await sweep(lock_seconds=None) == set()
|
||||
assert broadcaster.room_events == []
|
||||
assert await presence.is_online(alice.id) is True
|
||||
|
||||
|
||||
async def test_sweep_leaves_healthy_connections_alone(world, db_session):
|
||||
clock, presence, broadcaster, make_user, sweep = world
|
||||
alice = await make_user("alice")
|
||||
await presence.connect(alice.id, "tab1")
|
||||
clock.advance(30)
|
||||
assert await sweep(lock_seconds=None) == set()
|
||||
assert broadcaster.room_events == []
|
||||
|
||||
|
||||
async def test_only_one_sweeper_runs_per_interval(world, db_session, redis_client):
|
||||
clock, presence, broadcaster, make_user, sweep = world
|
||||
alice = await make_user("alice")
|
||||
await presence.connect(alice.id, "tab1")
|
||||
clock.advance(200)
|
||||
|
||||
# Another worker holds the lock: this one must not sweep (or broadcast).
|
||||
await redis_client.set(SWEEP_LOCK_KEY, "1", ex=14)
|
||||
assert await sweep(lock_seconds=14) == set()
|
||||
assert await redis_client.zcard(GlobalPresence.KEY) == 1
|
||||
|
||||
await redis_client.delete(SWEEP_LOCK_KEY)
|
||||
assert await sweep(lock_seconds=14) == {alice.id}
|
||||
# And the lock is now held, so an immediate second attempt does nothing.
|
||||
assert await sweep(lock_seconds=14) == set()
|
||||
|
||||
|
||||
async def test_one_failed_broadcast_does_not_stop_the_rest(world, db_session):
|
||||
clock, presence, broadcaster, make_user, sweep = world
|
||||
alice, bob = await make_user("alice"), await make_user("bob")
|
||||
room = await create_room(db_session, alice.id, RoomCreate(name=f"r-{uuid.uuid4().hex[:8]}"))
|
||||
other = await create_room(db_session, bob.id, RoomCreate(name=f"r-{uuid.uuid4().hex[:8]}"))
|
||||
await presence.connect(alice.id, "a")
|
||||
await presence.connect(bob.id, "b")
|
||||
clock.advance(200)
|
||||
|
||||
real_publish = broadcaster.publish
|
||||
failed = []
|
||||
|
||||
async def flaky_publish(room_id, payload):
|
||||
if room_id == room.id:
|
||||
failed.append(room_id)
|
||||
raise RuntimeError("redis hiccup")
|
||||
await real_publish(room_id, payload)
|
||||
|
||||
broadcaster.publish = flaky_publish
|
||||
|
||||
assert await sweep(lock_seconds=None) == {alice.id, bob.id}
|
||||
assert failed == [room.id]
|
||||
assert other.id in {rid for rid, _ in broadcaster.room_events}
|
||||
|
||||
|
||||
async def test_purge_removes_legacy_keys_but_keeps_v2(redis_client):
|
||||
legacy = ["presence:global", "presence:unfocused", f"presence:{uuid.uuid4()}"]
|
||||
current = f"presence:v2:test:{uuid.uuid4()}"
|
||||
for key in legacy:
|
||||
await redis_client.hset(key, "someone", 3)
|
||||
await redis_client.zadd(current, {"x:y": time.time() + 90})
|
||||
|
||||
try:
|
||||
assert await purge_legacy_presence(redis_client) >= len(legacy)
|
||||
for key in legacy:
|
||||
assert await redis_client.exists(key) == 0
|
||||
assert await redis_client.exists(current) == 1
|
||||
finally:
|
||||
await redis_client.delete(current, *legacy)
|
||||
Reference in new issue
Block a user