fix(matrix): isolate per-event failures in _dispatch_sync gather
`_dispatch_sync` gathers the mautrix per-event handler tasks with a bare `asyncio.gather(*tasks)`. Without `return_exceptions=True`, the first handler that raises aborts the gather, so the sibling events in the same sync response are dropped unprocessed — the exception propagates up to the sync loop, which logs a single "sync error" and moves on. The invite/redaction gathers a few lines above already use `return_exceptions=True`. Use `return_exceptions=True` and log each failing handler, so one bad event no longer takes out the rest of its batch and per-event failures stay visible. Regression test: a batch with one failing and one succeeding handler no longer raises, the good handler still runs, and the failure is logged (mutation- verified — reverting re-raises RuntimeError out of _dispatch_sync).
This commit is contained in:
parent
e4dbb67bf5
commit
62882b8e6f
2 changed files with 45 additions and 1 deletions
|
|
@ -2351,7 +2351,18 @@ class MatrixAdapter(BasePlatformAdapter):
|
|||
if inspect.isawaitable(tasks):
|
||||
tasks = await tasks
|
||||
if tasks:
|
||||
await asyncio.gather(*tasks)
|
||||
# return_exceptions=True so one failing event handler doesn't abort
|
||||
# the whole gather and silently drop the SIBLING events in the same
|
||||
# sync response (a bare gather re-raises the first exception, leaving
|
||||
# the rest of the batch unprocessed). Mirrors the invite/redaction
|
||||
# gathers above. Surface each failure instead of swallowing it.
|
||||
results = await asyncio.gather(*tasks, return_exceptions=True)
|
||||
for result in results:
|
||||
if isinstance(result, Exception):
|
||||
logger.warning(
|
||||
"Matrix: event handler failed during sync dispatch: %s",
|
||||
result,
|
||||
)
|
||||
|
||||
def _is_self_sender(self, sender: str) -> bool:
|
||||
"""Return True if the sender refers to the bot's own account.
|
||||
|
|
|
|||
|
|
@ -5276,3 +5276,36 @@ class TestDeviceIdRecoveryOnReconnect:
|
|||
assert None not in _verify_call.args[0]["@bot:example.org"]
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
|
||||
class TestMatrixDispatchSyncIsolation:
|
||||
"""A failing mautrix event handler must not abort the whole sync batch.
|
||||
|
||||
``_dispatch_sync`` gathers the per-event handler tasks. Without
|
||||
``return_exceptions=True`` the first exception aborts the gather and the
|
||||
sibling events in the same sync response are silently dropped.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_sync_isolates_failing_handler(self, caplog):
|
||||
import logging
|
||||
|
||||
adapter = _make_adapter()
|
||||
ran = {"ok": False}
|
||||
|
||||
async def _boom():
|
||||
raise RuntimeError("handler boom")
|
||||
|
||||
async def _ok():
|
||||
ran["ok"] = True
|
||||
|
||||
client = MagicMock()
|
||||
client.handle_sync = MagicMock(return_value=[_boom(), _ok()])
|
||||
adapter._client = client
|
||||
|
||||
with caplog.at_level(logging.WARNING):
|
||||
# Must not raise despite the failing handler.
|
||||
await adapter._dispatch_sync({"next_batch": "s1"})
|
||||
|
||||
assert ran["ok"] is True # the sibling handler still ran
|
||||
assert "event handler failed" in caplog.text # failure surfaced, not swallowed
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue