Skip to content

refactor(matrix): remove delayed reaction redaction, run in background tasks (#22090) #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Phoenix1819 wants to merge 1 commit into from
Closed

refactor(matrix): remove delayed reaction redaction, run in background tasks (#22090) #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions gateway/platforms/matrix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,11 +384,6 @@ def __init__(self, config: PlatformConfig):
"MATRIX_REACTIONS", "true"
).lower() not in ("false", "0", "no")
self._pending_reactions: dict[tuple[str, str], str] = {}
# Delay before redacting reactions so Matrix homeservers have time to
# deliver the final message event without tripping "missing event"
# errors in some clients. 5s is empirically safe; not user-tunable —
# if that changes, add a config.yaml entry rather than an env var.
self._reaction_redaction_delay_seconds = 5.0
self._reaction_redaction_tasks: Set[asyncio.Task] = set()

# Proxy support — resolve once at init, reuse for all HTTP traffic.
Expand Down Expand Up @@ -1981,12 +1976,10 @@ def _schedule_reaction_redaction(
reaction_event_id: str,
reason: str = "",
) -> None:
"""Redact a reaction after a short delay so message delivery settles."""
"""Redact a reaction in the background so the processing loop isn't blocked."""

async def _redact_later() -> None:
async def _redact_bg() -> None:
try:
if self._reaction_redaction_delay_seconds:
await asyncio.sleep(self._reaction_redaction_delay_seconds)
if not await self._redact_reaction(room_id, reaction_event_id, reason):
logger.debug(
"Matrix: failed to redact reaction %s", reaction_event_id
Expand All @@ -1995,12 +1988,12 @@ async def _redact_later() -> None:
raise
except Exception as exc:
logger.debug(
"Matrix: delayed reaction redaction failed for %s: %s",
"Matrix: background reaction redaction failed for %s: %s",
reaction_event_id,
exc,
)

task = asyncio.create_task(_redact_later())
task = asyncio.create_task(_redact_bg())
self._reaction_redaction_tasks.add(task)
task.add_done_callback(self._reaction_redaction_tasks.discard)

Expand Down
20 changes: 7 additions & 13 deletions tests/gateway/test_matrix.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1738,7 +1738,6 @@ async def test_on_processing_complete_sends_check(self):
from gateway.platforms.base import MessageEvent, MessageType, ProcessingOutcome

self.adapter._reactions_enabled = True
self.adapter._reaction_redaction_delay_seconds = 0.01
self.adapter._pending_reactions = {("!room:ex", "$msg1"): "$eyes_reaction_123"}
self.adapter._redact_reaction = AsyncMock(return_value=True)
self.adapter._send_reaction = AsyncMock(return_value="$check_reaction_456")
Expand All @@ -1753,21 +1752,19 @@ async def test_on_processing_complete_sends_check(self):
message_id="$msg1",
)
await self.adapter.on_processing_complete(event, ProcessingOutcome.SUCCESS)
self.adapter._redact_reaction.assert_not_awaited()
self.adapter._send_reaction.assert_called_once_with("!room:ex", "$msg1", "\u2705")
await asyncio.sleep(0.03)
await asyncio.sleep(0)
self.adapter._redact_reaction.assert_awaited_once_with(
"!room:ex",
"$eyes_reaction_123",
"processing complete",
)
self.adapter._send_reaction.assert_called_once_with("!room:ex", "$msg1", "\u2705")

@pytest.mark.asyncio
async def test_on_processing_complete_sends_cross_on_failure(self):
from gateway.platforms.base import MessageEvent, MessageType, ProcessingOutcome

self.adapter._reactions_enabled = True
self.adapter._reaction_redaction_delay_seconds = 0.01
self.adapter._pending_reactions = {("!room:ex", "$msg1"): "$eyes_reaction_123"}
self.adapter._redact_reaction = AsyncMock(return_value=True)
self.adapter._send_reaction = AsyncMock(return_value="$cross_reaction_456")
Expand All @@ -1782,14 +1779,13 @@ async def test_on_processing_complete_sends_cross_on_failure(self):
message_id="$msg1",
)
await self.adapter.on_processing_complete(event, ProcessingOutcome.FAILURE)
self.adapter._redact_reaction.assert_not_awaited()
self.adapter._send_reaction.assert_called_once_with("!room:ex", "$msg1", "\u274c")
await asyncio.sleep(0.03)
await asyncio.sleep(0)
self.adapter._redact_reaction.assert_awaited_once_with(
"!room:ex",
"$eyes_reaction_123",
"processing complete",
)
self.adapter._send_reaction.assert_called_once_with("!room:ex", "$msg1", "\u274c")

@pytest.mark.asyncio
async def test_on_processing_complete_cancelled_sends_no_terminal_reaction(self):
Expand Down Expand Up @@ -1834,10 +1830,9 @@ async def test_on_processing_complete_no_pending_reaction(self):
self.adapter._send_reaction.assert_called_once_with("!room:ex", "$msg1", "\u2705")

@pytest.mark.asyncio
async def test_approval_reaction_cleanup_is_delayed(self):
"""Bot approval reaction redactions should not run inline."""
async def test_approval_reaction_cleanup_is_background(self):
"""Bot approval reaction redactions run in background tasks."""

self.adapter._reaction_redaction_delay_seconds = 0.01
self.adapter._redact_reaction = AsyncMock(return_value=True)
prompt = MagicMock()
prompt.bot_reaction_events = {
Expand All @@ -1847,8 +1842,7 @@ async def test_approval_reaction_cleanup_is_delayed(self):

await self.adapter._redact_bot_approval_reactions("!room:ex", prompt)

self.adapter._redact_reaction.assert_not_awaited()
await asyncio.sleep(0.03)
await asyncio.sleep(0)
self.adapter._redact_reaction.assert_any_await(
"!room:ex",
"$allow_reaction",
Expand Down
Loading