mirror of
https://github.com/Screenly/Anthias.git
synced 2026-07-30 17:25:50 -04:00
* fix(ws): tolerate a client that disconnects mid-broadcast
AssetConsumer.asset_update did an unguarded await self.send(). A browser
can disconnect in the window between the group_send dispatch and this
send, so the ASGI server has already emitted 'websocket.close' and
channels raises RuntimeError("Unexpected ASGI message 'websocket.send',
after sending 'websocket.close'") — a disconnect-vs-broadcast race
surfacing as an unhandled error in Sentry (ANTHIAS-1K).
The nudge is best-effort (the client's 5s poll backs it up) and
group_discard runs in disconnect(), so the stale channel is already on
its way out. Swallow the RuntimeError and log at debug rather than let it
reach Sentry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ws): log exception + asset_id when swallowing send-after-close
Address Copilot review: logging the swallowed RuntimeError without
detail made the expected send-after-close race indistinguishable from an
unexpected RuntimeError out of send(). Log the asset_id and exc_info at
debug — same swallow behavior, but diagnostics are preserved.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ws): only swallow the send-after-close race, re-raise other errors
Address Copilot re-review: the broad `except RuntimeError` swallowed
every RuntimeError from send(), which contradicts the intent (tolerate
only the disconnect race) and could hide genuine failures. Match the
channels "Unexpected ASGI message 'websocket.send'" message and re-raise
anything else. Add a regression test asserting an unrelated RuntimeError
still propagates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(ws): require the close/completed clause before swallowing
Address Copilot re-review: the substring check matched any RuntimeError
mentioning "Unexpected ASGI message 'websocket.send'", which could hide
an unrelated ASGI state bug. Require the close/completed clause too
('websocket.close' or 'response already completed') so only the genuine
send-after-close race is swallowed. Add a test that a websocket.send
RuntimeError without the close clause still propagates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(ws): mock send() via patch.object to satisfy mypy method-assign
The direct `consumer.send = AsyncMock()` assignments tripped mypy's
method-assign check (run-mypy CI failure). Patch the instance attribute
with mock.patch.object instead — same behaviour, restores cleanly, and
no assignment to a method-typed attribute.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
73 lines
2.6 KiB
Python
73 lines
2.6 KiB
Python
import asyncio
|
|
from unittest import mock
|
|
|
|
import pytest
|
|
|
|
from anthias_server.app.consumers import AssetConsumer
|
|
|
|
|
|
def test_asset_update_sends_asset_id() -> None:
|
|
"""The happy path forwards the asset_id as the text frame so the
|
|
browser's htmx handler knows which row changed."""
|
|
consumer = AssetConsumer()
|
|
send = mock.AsyncMock()
|
|
|
|
with mock.patch.object(consumer, 'send', send):
|
|
asyncio.run(consumer.asset_update({'asset_id': 'abc123'}))
|
|
|
|
send.assert_awaited_once_with(text_data='abc123')
|
|
|
|
|
|
def test_asset_update_swallows_send_after_close() -> None:
|
|
"""A browser can disconnect between the group_send dispatch and this
|
|
send, so channels raises RuntimeError("Unexpected ASGI message
|
|
'websocket.send', after sending 'websocket.close'"). The nudge is
|
|
best-effort (the 5s poll backs it up), so the consumer must swallow
|
|
it rather than let it reach Sentry (ANTHIAS-1K)."""
|
|
consumer = AssetConsumer()
|
|
send = mock.AsyncMock(
|
|
side_effect=RuntimeError(
|
|
"Unexpected ASGI message 'websocket.send', after sending "
|
|
"'websocket.close' or response already completed."
|
|
)
|
|
)
|
|
|
|
# Must not raise.
|
|
with mock.patch.object(consumer, 'send', send):
|
|
asyncio.run(consumer.asset_update({'asset_id': 'abc123'}))
|
|
|
|
send.assert_awaited_once()
|
|
|
|
|
|
def test_asset_update_reraises_unrelated_runtime_error() -> None:
|
|
"""The swallow is scoped to the send-after-close message — any other
|
|
RuntimeError out of send() (a serialization error, a Channels bug) is
|
|
a genuine failure and must still propagate."""
|
|
consumer = AssetConsumer()
|
|
send = mock.AsyncMock(side_effect=RuntimeError('something actually broke'))
|
|
|
|
with (
|
|
mock.patch.object(consumer, 'send', send),
|
|
pytest.raises(RuntimeError, match='something actually broke'),
|
|
):
|
|
asyncio.run(consumer.asset_update({'asset_id': 'abc123'}))
|
|
|
|
|
|
def test_asset_update_reraises_non_close_websocket_send_error() -> None:
|
|
"""The swallow requires the close/completed clause: a RuntimeError
|
|
that merely mentions 'websocket.send' but is not the send-after-close
|
|
race (some other ASGI state bug) must still propagate."""
|
|
consumer = AssetConsumer()
|
|
send = mock.AsyncMock(
|
|
side_effect=RuntimeError(
|
|
"Unexpected ASGI message 'websocket.send', after sending "
|
|
"'websocket.accept' was expected"
|
|
)
|
|
)
|
|
|
|
with (
|
|
mock.patch.object(consumer, 'send', send),
|
|
pytest.raises(RuntimeError, match='websocket.accept'),
|
|
):
|
|
asyncio.run(consumer.asset_update({'asset_id': 'abc123'}))
|