From 935cc237dbb006290f9894035667dc9c18bf6833 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 15 Sep 2026 00:42:29 +0530 Subject: [PATCH] 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. --- tests/gateway/test_matrix.py | 69 +++++++++++++++++------------ tests/gateway/test_ws_auth_retry.py | 12 ++--- 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/tests/gateway/test_matrix.py b/tests/gateway/test_matrix.py index ec5b392f3a..4b9a183839 100644 --- a/tests/gateway/test_matrix.py +++ b/tests/gateway/test_matrix.py @@ -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: ', - http_status=502, + ( + _sync_error( + '502: ', + 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: proxy", 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): diff --git a/tests/gateway/test_ws_auth_retry.py b/tests/gateway/test_ws_auth_retry.py index a00f4fb659..b5ad1948ae 100644 --- a/tests/gateway/test_ws_auth_retry.py +++ b/tests/gateway/test_ws_auth_retry.py @@ -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