Private
Public Access
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>
This commit is contained in:
1 parent
86584660fb
commit
850d9ec0fa
3 files changed
+127
-4
No files matched your search
@@ -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()
|
||||
Reference in new issue
Block a user