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

Don't error on unknown receipt types #12670

Merged
merged 3 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/12670.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement [changes](https://github.com/matrix-org/matrix-spec-proposals/pull/2285/commits/4a77139249c2e830aec3c7d6bd5501a514d1cc27) to [MSC2285 (hidden read receipts)](https://github.com/matrix-org/matrix-spec-proposals/pull/2285). Contributed by @SimonBrandner.
26 changes: 15 additions & 11 deletions synapse/rest/client/read_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,21 @@ async def on_POST(

body = parse_json_object_from_request(request)

valid_receipt_types = {ReceiptTypes.READ, ReceiptTypes.FULLY_READ}
if self.config.experimental.msc2285_enabled:
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
valid_receipt_types.add(ReceiptTypes.READ_PRIVATE)

if set(body.keys()) > valid_receipt_types:
raise SynapseError(
400,
"Receipt type must be 'm.read', 'org.matrix.msc2285.read.private' or 'm.fully_read'"
if self.config.experimental.msc2285_enabled
else "Receipt type must be 'm.read' or 'm.fully_read'",
)
valid_receipt_types = {
ReceiptTypes.READ,
ReceiptTypes.FULLY_READ,
ReceiptTypes.READ_PRIVATE,
}
Comment on lines +52 to +56
Copy link
Member

@clokep clokep May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you still want to consider READ_PRIVATE valid only if the flag is on?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #12670 (comment). My understanding from what Erik said was that this only governs the warning we show, not the enforcement logic later(?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but would still be a confusing/incorrect warning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I guess the alternative is log spam...so maybe this is best.


unrecognized_types = set(body.keys()) - valid_receipt_types
if unrecognized_types:
# It's fine if there are unrecognized receipt types, but let's log
# it to help debug clients that have typoed the receipt type.
#
# We specifically *don't* error here, as a) it stops us processing
# the valid receipts, and b) we need to be extensible on receipt
# types.
logger.info("Ignoring unrecognized receipt types: %s", unrecognized_types)

read_event_id = body.get(ReceiptTypes.READ, None)
if read_event_id:
Expand Down