Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Stop adding a message_id to to-device messages
Browse files Browse the repository at this point in the history
Currently, we add a `message_id` field to any to-device message that is sent by
a local user to another local user, and then strip it off again before serving
it to the user. This isn't terribly useful, because:

1. it can't be correlated in the sending/receiving clients

2. a single sendToDevice call, or federation EDU, can create multiple to-device
   messages all with the same message id, which again makes them less than
   useful for tracing.

3. It was never wired up for messages received over federation.

So, let's stop doing that, in preparation for something better.
  • Loading branch information
richvdh committed Dec 2, 2022
1 parent 6acb6d7 commit 76d44db
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 15 deletions.
3 changes: 0 additions & 3 deletions synapse/handlers/appservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,9 +578,6 @@ async def _get_to_device_messages(
device_id,
), messages in recipient_device_to_messages.items():
for message_json in messages:
# Remove 'message_id' from the to-device message, as it's an internal ID
message_json.pop("message_id", None)

message_payload.append(
{
"to_user_id": user_id,
Expand Down
10 changes: 5 additions & 5 deletions synapse/handlers/devicemessage.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,6 @@ async def send_device_message(
"""
sender_user_id = requester.user.to_string()

message_id = random_string(16)
set_tag(SynapseTags.TO_DEVICE_MESSAGE_ID, message_id)

log_kv({"number_of_to_device_messages": len(messages)})
set_tag("sender", sender_user_id)
local_messages = {}
Expand Down Expand Up @@ -247,7 +244,6 @@ async def send_device_message(
"content": message_content,
"type": message_type,
"sender": sender_user_id,
"message_id": message_id,
}
for device_id, message_content in by_device.items()
}
Expand All @@ -267,7 +263,11 @@ async def send_device_message(

remote_edu_contents = {}
for destination, messages in remote_messages.items():
log_kv({"destination": destination})
# The EDU contains a "message_id" property which is used for
# idempotence. Make up a random one.
message_id = random_string(16)
log_kv({"destination": destination, "message_id": message_id})

remote_edu_contents[destination] = {
"messages": messages,
"sender": sender_user_id,
Expand Down
7 changes: 0 additions & 7 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -1602,13 +1602,6 @@ async def _generate_sync_entry_for_to_device(
user_id, device_id, since_stream_id, now_token.to_device_key
)

for message in messages:
# We pop here as we shouldn't be sending the message ID down
# `/sync`
message_id = message.pop("message_id", None)
if message_id:
set_tag(SynapseTags.TO_DEVICE_MESSAGE_ID, message_id)

logger.debug(
"Returning %d to-device messages between %d and %d (current token: %d)",
len(messages),
Expand Down

0 comments on commit 76d44db

Please sign in to comment.