test(matrix): cover status-only 401 and rate-limit paths in the sync-loop test

Fold the transient/permanent sync-loop tests into one parametrized table
and add the two classifier branches that had no loop-level coverage: a
401 whose body was rewritten to HTML by a reverse proxy (errcode dropped,
so only http_status can stop the loop) and a 429 M_LIMIT_EXCEEDED that
must be retried because neither errcode nor status is an auth signal.
Deleting the http_status fallback in _is_permanent_matrix_auth_error now
fails the 401-html case.

Hoist _sync_error to module level so parametrize can call it directly
instead of the staticmethod.__func__ workaround. Trim the
test_ws_auth_retry docstring, which still described a Matrix test class
that moved to test_matrix.py.
This commit is contained in:
kshitijk4poor
2026-09-15 00:42:29 +05:30
committed by kshitij
parent 257288ede1
commit 935cc237db
2 changed files with 47 additions and 34 deletions

View File

@@ -1266,9 +1266,16 @@ class TestMatrixDeviceIdConfig:
assert mc.extra.get("device_id") == "HERMES_BOT"
class TestMatrixSyncLoop:
def _sync_error(message, **attrs):
"""Shape of mautrix's MatrixRequestError: message text + structured attrs."""
exc = Exception(message)
for k, v in attrs.items():
setattr(exc, k, v)
return exc
class TestMatrixSyncLoop:
@pytest.mark.asyncio
async def test_dispatch_sync_accepts_async_handle_sync(self):
"""Some fake clients expose handle_sync as an async dispatcher."""
@@ -1340,14 +1347,6 @@ class TestMatrixSyncLoop:
assert captured[0].text == "hello"
assert captured[0].source.chat_type == "dm"
@staticmethod
def _sync_error(message, **attrs):
"""Shape of mautrix's MatrixRequestError: message text + structured attrs."""
exc = Exception(message)
for k, v in attrs.items():
setattr(exc, k, v)
return exc
async def _run_sync_loop_with_first_error(self, exc):
"""Drive _sync_loop: sync() raises exc once, then returns a clean dict and closes."""
adapter = _make_adapter()
@@ -1373,34 +1372,46 @@ class TestMatrixSyncLoop:
@pytest.mark.asyncio
@pytest.mark.parametrize(
"exc",
("exc", "expected_sync_calls"),
[
# Umbrel app-proxy 502: an SVG path coordinate embeds "403".
_sync_error.__func__(
'502: <!DOCTYPE html><svg><path d="M17.4517 1403.2C12.7214 1403.2"/></svg>',
http_status=502,
(
_sync_error(
'502: <!DOCTYPE html><svg><path d="M17.4517 1403.2C12.7214 1403.2"/></svg>',
http_status=502,
),
2,
),
# Plain timeout echoing the pagination token, which embeds "401".
asyncio.TimeoutError(
"Connection timeout to host https://matrix.example.org/_matrix/"
"client/v3/sync?timeout=30000&since=s72802_401975_486_12943_11759"
(
asyncio.TimeoutError(
"Connection timeout to host https://matrix.example.org/_matrix/"
"client/v3/sync?timeout=30000&since=s72802_401975_486_12943_11759"
),
2,
),
# Rate limiting is a non-auth errcode on a non-auth status: retried.
(_sync_error("rate limited", errcode="M_LIMIT_EXCEEDED", http_status=429), 2),
# Structured 401 with an auth errcode: permanent, loop returns.
(_sync_error("Invalid access token", errcode="M_UNKNOWN_TOKEN", http_status=401), 1),
# Reverse proxy rewrote the body to HTML and dropped the errcode; the
# 401 status alone must still stop the loop.
(_sync_error("401: <html>proxy</html>", errcode=None, http_status=401), 1),
],
ids=[
"502-html-body-with-403-digits",
"timeout-since-token-with-401-digits",
"429-rate-limited",
"401-unknown-token",
"401-html-body-no-errcode",
],
ids=["502-html-body-with-403-digits", "timeout-since-token-with-401-digits"],
)
async def test_sync_loop_retries_transient_error_whose_text_embeds_auth_digits(self, exc):
"""Both production repros: the old substring check stopped the loop forever on these."""
async def test_sync_loop_retries_only_non_auth_errors(self, exc, expected_sync_calls):
"""Transient errors (even when their text embeds auth digits) are retried once
with the 5s backoff; structured auth failures return without retrying."""
sync_calls, sleeps = await self._run_sync_loop_with_first_error(exc)
assert sync_calls == 2
assert 5 in sleeps # the retry backoff, not the 0s dispatch-yield
@pytest.mark.asyncio
async def test_sync_loop_stops_on_structured_auth_error(self):
"""A 401 with errcode M_UNKNOWN_TOKEN is permanent: no retry, loop returns."""
exc = self._sync_error("Invalid access token", errcode="M_UNKNOWN_TOKEN", http_status=401)
sync_calls, sleeps = await self._run_sync_loop_with_first_error(exc)
assert sync_calls == 1
assert 5 not in sleeps
assert sync_calls == expected_sync_calls
assert (5 in sleeps) is (expected_sync_calls == 2) # the retry backoff, not the 0s dispatch-yield
@pytest.mark.asyncio
async def test_connect_receives_dm_from_initial_sync_dispatch(self):

View File

@@ -1,11 +1,13 @@
"""Tests for auth-aware retry in Mattermost WS and Matrix sync loops.
"""Tests for auth-aware retry in the Mattermost WS loop.
Both Mattermost's _ws_loop and Matrix's _sync_loop previously caught all
exceptions with a broad ``except Exception`` and retried forever. Permanent
auth failures (401, 403, M_UNKNOWN_TOKEN) would loop indefinitely instead
of stopping. These tests verify that auth errors now stop the reconnect.
Mattermost's _ws_loop previously caught all exceptions with a broad
``except Exception`` and retried forever, so permanent auth failures (401,
403) looped indefinitely instead of stopping. These tests verify that auth
errors now stop the reconnect. The Matrix sync-loop counterpart lives in
tests/gateway/test_matrix.py::TestMatrixSyncLoop.
"""
import asyncio
from unittest.mock import AsyncMock, MagicMock, patch