4 Commits
Author SHA1 Message Date
ksmithandClaude Sonnet 5 047f47d524 Bump to v2026.9.26
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-26 15:20:20 -06:00
ksmithandClaude Sonnet 5 850d9ec0fa Supervise the broadcast listener so it recovers from a dropped Redis connection (#76)
The previous fix hardened listen() against a single bad message, but a
connection-level failure (Redis restarting, a network blip) raises from
the async-for iteration itself, outside that guard, and killed the
task permanently and silently -- HTTP/REST and login kept working fine
since neither touches it, but live message delivery was gone for every
room and every user on the affected process until a manual restart.

Confirmed live in production after an OS update restarted Redis:
PUBSUB NUMPAT showed zero pattern subscriptions across all 4 gunicorn
workers, meaning every one of them had lost its listener with nothing
to bring it back.

Broadcaster.run_forever() now supervises listen(), restarting it with
exponential backoff (capped at 30s, reset after a stable 60s+ run) on
any failure, while still letting a genuine shutdown (task
cancellation) propagate immediately instead of being treated as just
another retry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-26 15:19:59 -06:00
ksmithandClaude Sonnet 5 86584660fb Bump to v2026.9.23
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-23 12:59:15 -06:00
ksmithandClaude Sonnet 5 6f67f36578 Add WS heartbeat and harden broadcast against dead sockets (#76)
Messages could silently stop arriving until a room switch happened to
paper over it with a REST refetch. Root cause: a connection that dies
without a clean close (laptop sleep, an idle connection recycled by a
proxy/NAT) looks identical to a healthy-but-quiet one -- readyState
stays OPEN, onclose never fires, so the already-correct reconnect and
rejoin logic never runs.

The backend now pings every 30s and the frontend force-closes the
socket if nothing (any frame, not just a pong) arrives within 65s,
routing straight through the existing reconnect/rejoin path instead of
needing separate recovery logic.

Also found and fixed a more severe adjacent bug while tracing this:
ConnectionManager.broadcast()/send_to_user() had no per-socket error
handling, so one dead or racing socket could raise inside the app's
single, never-restarted Redis pubsub listener task and silently kill
live delivery for every room and every user on that process until
restart. Now caught and logged per-socket instead of propagating.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-23 12:37:58 -06:00
9 changed files with 254 additions and 10 deletions

No files matched your search

+6 -2
View File
@@ -58,8 +58,12 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
# ASGITransport, used by the plain REST test fixtures -- only
# TestClient's websocket_connect-based tests actually run lifespan).
# The background listener task genuinely needs a running event loop
# though, so that part stays here.
listener_task = asyncio.create_task(app.state.broadcaster.listen())
# though, so that part stays here. run_forever() (not listen() directly)
# -- #76: a bare listen() here died permanently and silently the moment
# its Redis connection ever dropped (e.g. Redis itself restarting),
# with nothing to notice or reconnect; run_forever() supervises it and
# restarts on failure instead.
listener_task = asyncio.create_task(app.state.broadcaster.run_forever())
yield
+73
View File
@@ -1,13 +1,26 @@
import asyncio
import contextlib
import json
import logging
import uuid
from redis.asyncio import Redis
from app.ws.connection_manager import ConnectionManager
logger = logging.getLogger(__name__)
ROOM_CHANNEL_PREFIX = "room:"
USER_CHANNEL_PREFIX = "user:"
# #76: a run shorter than this before dying doesn't count as "stable" for
# resetting the backoff in run_forever() below -- long enough that a real,
# working connection has clearly settled in, short enough not to leave a
# just-recovered listener exposed to a full backoff climb if Redis blips
# again shortly after reconnecting.
STABLE_RUN_SECONDS = 60
MAX_BACKOFF_SECONDS = 30
class Broadcaster:
"""Cross-instance message fan-out (ARCHITECTURE.md phase 5).
@@ -36,6 +49,43 @@ class Broadcaster:
async def publish_to_user(self, user_id: uuid.UUID, payload: dict) -> None:
await self._redis.publish(f"{USER_CHANNEL_PREFIX}{user_id}", json.dumps(payload))
async def run_forever(self) -> None:
"""Supervises listen(), restarting it if it ever dies.
#76's real root cause, found after the per-message try/except below
turned out not to be enough: this is the *one* pubsub listener for
the whole process (see main.py's lifespan), created once at startup
with nothing watching over it afterward. A connection-level failure
-- Redis restarting, a network blip between the app and Redis --
raises from the `async for` iteration in listen() itself, outside
that per-message guard, and previously killed this task permanently
and silently: HTTP/REST and login kept working fine (neither
touches this), but live message delivery was gone for every room
and every user on this instance until the whole process was
restarted. Confirmed live in production: Redis got restarted by an
OS update, and `PUBSUB NUMPAT` against it afterward showed zero
pattern subscriptions across all 4 gunicorn workers -- every one of
them had silently lost its listener with nothing to bring it back.
"""
backoff_seconds = 1.0
while True:
started_at = asyncio.get_event_loop().time()
try:
await self.listen()
logger.warning("Broadcaster.listen() exited without error -- restarting")
except asyncio.CancelledError:
raise
except Exception:
logger.error("Broadcaster.listen() failed -- restarting", exc_info=True)
ran_for = asyncio.get_event_loop().time() - started_at
# Sleep at the *current* backoff first, then grow it for next
# time -- computing the next value before sleeping would skip
# straight past the first (shortest) retry delay.
await asyncio.sleep(backoff_seconds)
backoff_seconds = (
1.0 if ran_for > STABLE_RUN_SECONDS else min(backoff_seconds * 2, MAX_BACKOFF_SECONDS)
)
async def listen(self) -> None:
pubsub = self._redis.pubsub()
await pubsub.psubscribe(f"{ROOM_CHANNEL_PREFIX}*", f"{USER_CHANNEL_PREFIX}*")
@@ -43,6 +93,16 @@ class Broadcaster:
async for message in pubsub.listen():
if message["type"] != "pmessage":
continue
# #76: this is the one pubsub listener for the whole process
# (see main.py's lifespan) -- ConnectionManager.broadcast/
# send_to_user already guard against one dead socket taking
# the rest of a single fan-out down, but this belt-and-
# suspenders catch is for anything else unexpected (a
# malformed payload, e.g.) doing the same. An uncaught
# exception here previously meant *this whole async for*
# loop died silently -- no more live delivery to any room on
# this instance, for anyone, until the process restarted.
try:
channel = message["channel"]
payload = json.loads(message["data"])
if channel.startswith(ROOM_CHANNEL_PREFIX):
@@ -51,6 +111,19 @@ class Broadcaster:
elif channel.startswith(USER_CHANNEL_PREFIX):
user_id = uuid.UUID(channel.removeprefix(USER_CHANNEL_PREFIX))
await self._manager.send_to_user(user_id, payload)
except Exception:
logger.error("Error handling pubsub message on channel %s", message.get("channel"), exc_info=True)
finally:
# #76: reached via a connection that's already dead just as
# often as a clean shutdown (that's the whole failure mode
# run_forever() now recovers from) -- punsubscribe/aclose over
# a broken connection would themselves raise, masking whatever
# actually killed the loop above and, since run_forever() only
# starts backing off *after* listen() returns/raises, delaying
# the reconnect for no benefit (a fresh pubsub object is about
# to be created on retry regardless of whether this cleanup
# succeeded).
with contextlib.suppress(Exception):
await pubsub.punsubscribe(f"{ROOM_CHANNEL_PREFIX}*", f"{USER_CHANNEL_PREFIX}*")
with contextlib.suppress(Exception):
await pubsub.aclose()
+33 -1
View File
@@ -1,3 +1,4 @@
import asyncio
import uuid
from fastapi import APIRouter, Depends, WebSocket, WebSocketDisconnect
@@ -31,6 +32,18 @@ router = APIRouter(tags=["ws"])
WS_UNAUTHENTICATED = 4401
# #76: comfortably under the ~60s idle-connection timeout common to reverse
# proxies/load balancers sitting in front of this endpoint, and short enough
# that the frontend's own zombie-detection watchdog (useChatSocket.ts) can
# notice a dead connection and reconnect well within a session -- a socket
# can go silently dead (no close frame at all, e.g. a laptop sleeping or a
# NAT dropping an idle mapping) without this, and no live message reaches it
# again until *something* else finally triggers a close, which used to be
# "switch rooms and back" purely by accident (that remounts the message
# list, which refetches history over plain REST -- unrelated to the socket
# itself actually recovering).
WS_PING_INTERVAL_SECONDS = 30
class ClientEnvelope(BaseModel):
type: str
@@ -124,7 +137,18 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
try:
while True:
raw = await websocket.receive_json()
try:
raw = await asyncio.wait_for(
websocket.receive_json(), timeout=WS_PING_INTERVAL_SECONDS
)
except asyncio.TimeoutError:
# Nothing received in a while -- ping rather than assume the
# connection's still good. A genuinely dead socket fails this
# send (raising, caught below same as any other disconnect);
# a merely quiet one just gets proded and the loop continues
# waiting.
await websocket.send_json({"type": "ping"})
continue
try:
try:
envelope = ClientEnvelope.model_validate(raw)
@@ -146,6 +170,14 @@ async def chat_endpoint(websocket: WebSocket, db: AsyncSession = Depends(get_db)
joined_rooms.add(envelope.room_id)
await websocket.send_json({"type": "joined", "room_id": str(envelope.room_id)})
elif envelope.type == "pong":
# #76: the client's reply to our "ping" above -- nothing
# to do with it. Its only purpose is resetting the
# frontend's own liveness watchdog; the timeout on the
# receive_json() above already gets reset by receiving
# any frame at all, this one included.
pass
elif envelope.type == "leave":
if envelope.room_id is None:
await websocket.send_json({"type": "error", "detail": "room_id required"})
+17
View File
@@ -1,8 +1,11 @@
import logging
import uuid
from collections import defaultdict
from fastapi import WebSocket
logger = logging.getLogger(__name__)
class ConnectionManager:
"""Local, single-process WebSocket socket registry.
@@ -43,8 +46,22 @@ class ConnectionManager:
async def broadcast(self, room_id: uuid.UUID, payload: dict) -> None:
for websocket in list(self._rooms.get(room_id, ())):
# #76: one dead/racing socket (e.g. its own disconnect cleanup
# hasn't finished removing it from _rooms yet) must not take the
# rest of this broadcast down with it -- this runs inside
# Broadcaster.listen()'s single, never-restarted per-process
# pubsub loop, so an unhandled exception here previously meant
# one bad socket silently killed *live delivery for every room
# on this instance*, not just the one socket, until the process
# was restarted.
try:
await websocket.send_json(payload)
except Exception:
logger.warning("Dropping dead socket during broadcast to room %s", room_id, exc_info=True)
async def send_to_user(self, user_id: uuid.UUID, payload: dict) -> None:
for websocket in list(self._users.get(user_id, ())):
try:
await websocket.send_json(payload)
except Exception:
logger.warning("Dropping dead socket sending to user %s", user_id, exc_info=True)
+1 -1
View File
@@ -1,6 +1,6 @@
[project]
name = "ds-chat"
version = "2026.9.4"
version = "2026.9.26"
description = "DS Chat backend service"
license = { text = "AGPL-3.0-or-later" }
requires-python = ">=3.11"
@@ -0,0 +1,61 @@
import asyncio
import pytest
from app.ws.broadcaster import Broadcaster
async def test_run_forever_restarts_after_listen_raises(monkeypatch):
# #76: listen() dying (a dropped Redis connection, most commonly -- e.g.
# Redis itself restarting) must not permanently kill live delivery for
# the whole process. run_forever() has to notice and restart it, not
# just let the background task disappear silently while everything else
# (login, REST endpoints) keeps working fine.
broadcaster = Broadcaster(redis=None, manager=None)
calls = 0
async def fake_listen():
nonlocal calls
calls += 1
if calls < 3:
raise ConnectionError("redis went away")
# Stop the loop once the retry behavior below is proven, rather
# than looping forever in a test.
raise asyncio.CancelledError()
monkeypatch.setattr(broadcaster, "listen", fake_listen)
sleep_calls = []
async def fake_sleep(seconds):
sleep_calls.append(seconds)
monkeypatch.setattr(asyncio, "sleep", fake_sleep)
with pytest.raises(asyncio.CancelledError):
await broadcaster.run_forever()
assert calls == 3
# Slept once after each of the first two failures (exponential:
# 1s, then 2s) -- not after the third, which is the CancelledError
# that stops the loop before reaching the sleep.
assert sleep_calls == [1.0, 2.0]
async def test_run_forever_does_not_swallow_cancellation(monkeypatch):
# A genuine shutdown (the app's own lifespan cancels this task) must
# propagate immediately, not get treated as just another failure to
# retry after a backoff.
broadcaster = Broadcaster(redis=None, manager=None)
async def fake_listen():
raise asyncio.CancelledError()
monkeypatch.setattr(broadcaster, "listen", fake_listen)
async def fail_if_called(seconds):
raise AssertionError("should not sleep/retry on cancellation")
monkeypatch.setattr(asyncio, "sleep", fail_if_called)
with pytest.raises(asyncio.CancelledError):
await broadcaster.run_forever()
+18
View File
@@ -109,6 +109,24 @@ def test_history_tiebreaks_identical_timestamps_by_id(ws_client):
assert [m["content"] for m in history] == [m["content"] for m in expected_order]
def test_ws_sends_heartbeat_ping(ws_client, monkeypatch):
# #76: with nothing else sent on the connection, a "ping" must arrive
# proactively once the idle timeout elapses -- shrunk here so the test
# doesn't actually wait the real 30s. Replying "pong" (the frontend's
# own behavior) must be accepted silently and leave the connection
# fully usable afterward, not treated as an unknown message type.
monkeypatch.setattr("app.ws.chat.WS_PING_INTERVAL_SECONDS", 0.05)
username = _unique("alice")
_register(ws_client, username=username)
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
with ws_client.websocket_connect("/ws/chat") as ws:
assert ws.receive_json() == {"type": "ping"}
ws.send_json({"type": "pong"})
ws.send_json({"type": "join", "room_id": room["id"]})
assert ws.receive_json() == {"type": "joined", "room_id": room["id"]}
def test_ws_message_without_join_errors(ws_client):
_register(ws_client, username=_unique("alice"))
room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json()
+1 -1
View File
@@ -1,7 +1,7 @@
{
"name": "frontend",
"private": true,
"version": "2026.9.4",
"version": "2026.9.26",
"license": "AGPL-3.0-or-later",
"type": "module",
"scripts": {
+44 -5
View File
@@ -10,6 +10,13 @@ interface UseChatSocketOptions {
const RECONNECT_BASE_DELAY_MS = 1000
const RECONNECT_MAX_DELAY_MS = 30000
// #76: the backend pings every 30s (WS_PING_INTERVAL_SECONDS in chat.py) --
// this needs to comfortably tolerate one missed ping plus ordinary network
// jitter before declaring the connection dead, not fire on the very first
// late beat.
const ZOMBIE_TIMEOUT_MS = 65000
const ZOMBIE_CHECK_INTERVAL_MS = 10000
const desktopMode = isDesktopNotificationsSupported()
// Room join/leave (live message delivery) depends only on visibility --
@@ -51,6 +58,16 @@ export function useChatSocket({ onUnauthenticated }: UseChatSocketOptions) {
// live WebSocket delivery the user can't actually see will do the job.
const desiredRoomsRef = useRef(new Set<string>())
const isVisibleRef = useRef(isVisible())
// #76: last time *anything* arrived on the socket (a real message, a
// "ping", doesn't matter which) -- the only signal that a connection
// sitting at readyState OPEN is actually still alive rather than a
// zombie. Without this, a connection that dies without ever sending a
// close frame (laptop sleep, a NAT silently dropping an idle mapping)
// looks identical to a healthy-but-quiet one: onclose never fires, so the
// already-correct reconnect+rejoin logic below never runs, and new
// messages just stop arriving until something unrelated (e.g. switching
// rooms, which refetches history over plain REST) papers over it.
const lastActivityRef = useRef(Date.now())
const sendRoomFrame = useCallback((type: 'join' | 'leave', roomId: string) => {
const ws = socketRef.current
@@ -99,6 +116,7 @@ export function useChatSocket({ onUnauthenticated }: UseChatSocketOptions) {
if (hasConnectedBefore) checkForUpdate()
hasConnectedBefore = true
reconnectDelay = RECONNECT_BASE_DELAY_MS
lastActivityRef.current = Date.now()
setConnected(true)
// Re-join whatever rooms were joined before a reconnect -- the
// server has no memory of a dropped connection's prior state. Only
@@ -122,7 +140,17 @@ export function useChatSocket({ onUnauthenticated }: UseChatSocketOptions) {
ws.onmessage = (event) => {
if (socketRef.current !== ws) return
const envelope = JSON.parse(event.data) as ServerEnvelope
lastActivityRef.current = Date.now()
const parsed = JSON.parse(event.data)
// #76: the server's heartbeat -- purely transport-level, not a
// domain event, so it's answered and swallowed right here instead
// of being forwarded to subscribers (ServerEnvelope's own type
// doesn't include it for exactly that reason).
if (parsed.type === 'ping') {
ws.send(JSON.stringify({ type: 'pong' }))
return
}
const envelope = parsed as ServerEnvelope
for (const handler of subscribersRef.current) handler(envelope)
}
@@ -143,10 +171,9 @@ export function useChatSocket({ onUnauthenticated }: UseChatSocketOptions) {
if (stopped) return
// Unexpected close -- a deploy restarting the app server, a brief
// network blip, or (absent any app-level ping/pong) an idle
// connection getting recycled by a reverse proxy in front of it.
// Retry with exponential backoff instead of leaving the chat
// silently dead until the user manually reloads.
// network blip, or an idle connection getting recycled by a reverse
// proxy in front of it. Retry with exponential backoff instead of
// leaving the chat silently dead until the user manually reloads.
reconnectTimer = setTimeout(connect, reconnectDelay)
reconnectDelay = Math.min(reconnectDelay * 2, RECONNECT_MAX_DELAY_MS)
}
@@ -154,9 +181,21 @@ export function useChatSocket({ onUnauthenticated }: UseChatSocketOptions) {
connect()
// #76: catches the case onclose can't -- a connection that dies without
// ever sending a close frame at all (readyState stays OPEN forever) is
// otherwise invisible to this hook. Forcing a close here just routes it
// through the exact same onclose/reconnect/rejoin path as a normal
// disconnect, rather than needing separate recovery logic of its own.
const zombieCheckTimer = setInterval(() => {
if (Date.now() - lastActivityRef.current > ZOMBIE_TIMEOUT_MS) {
socketRef.current?.close()
}
}, ZOMBIE_CHECK_INTERVAL_MS)
return () => {
stopped = true
if (reconnectTimer) clearTimeout(reconnectTimer)
clearInterval(zombieCheckTimer)
socketRef.current?.close()
socketRef.current = null
}