From feca764c2a879c3d331ae4d3b196c25f057a8266 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Thu, 15 Feb 2024 17:22:45 +0000 Subject: [PATCH 1/3] MSC4102: Clarifying precedence in threaded and unthreaded read receipts in EDUs --- proposals/4102-unthreaded-read-receipts.md | 154 +++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 proposals/4102-unthreaded-read-receipts.md diff --git a/proposals/4102-unthreaded-read-receipts.md b/proposals/4102-unthreaded-read-receipts.md new file mode 100644 index 00000000000..14a7c73d789 --- /dev/null +++ b/proposals/4102-unthreaded-read-receipts.md @@ -0,0 +1,154 @@ +# MSC4102: Clarifying precedence in threaded and unthreaded read receipts in EDUs + +*This MSC assumes some knowledge around threaded receipts. Please read +[MSC3771: Read receipts for threads](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3771-read-receipts-for-threads.md) +as for brevity some concepts won't be re-explained.* + +This proposal clarifies previously undefined behaviour around how read receipts are expressed over +the CSAPI and SSAPI. This clarification reliably provides clients with sufficient information to +determine read receipts, without placing additional burdens on client implementations. + +## Background + +[MSC3771: Read receipts for threads](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3771-read-receipts-for-threads.md) +defined an API shape for threaded read receipts over the `/sync` API. They look like this: +```js +{ + "content": { + "$thread_reply": { + "m.read": { + "@rikj:jki.re": { + "ts": 1436451550453, + "thread_id": "$thread_root" // or "main" or absent + } + } + } + }, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "type": "m.receipt" +} +``` + +This is problematic because it expresses receipt uniqueness based on the 3-uple (event ID, receipt type, user ID). +In reality, uniqueness is based on the 4-uple of (event ID, receipt type, user ID, **thread ID**). This makes it +impossible to express certain receipt aggregations, such as: +``` + Receipts Table +room | user | event | thread +-----+------+-------+-------- +!foo | @bob | $abc | NULL // unthreaded receipt +!foo | @bob | $abc | "some_id" // threaded receipt +``` +It is impossible to express these two receipts in a single `m.receipt` EDU, as the presence of `thread_id: "some_id"` +by definition removes the absence of a thread ID, turning the receipt from an unthreaded receipt into a threaded receipt. + +[MSC3771: Read receipts for threads](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3771-read-receipts-for-threads.md) +does not provide rules around how to combine these receipts. This has led to undefined server behaviour. In practice, +both Synapse and the Sliding Sync Proxy will combine them in a "last write wins" manner. If the unthreaded receipt is +sent after the threaded receipt, then `/sync` will always return the unthreaded receipt and vice versa. This is not a +hypothetical problem: Element-Web will send both threaded and unthreaded receipts for the same event if the event in +question is the most recent event in the room, which is fairly common. + +This becomes a major problem when the threaded receipt "wins" and is returned _instead of_ the unthreaded receipt. +Some clients, such as Element X, do not handle threads and will ignore receipts with a `thread_id`, assuming that +clients will also be sending unthreaded receipts. When the threaded receipt "wins" however, these clients will simply +not see the receipt, despite the sender sending it. + +## Proposal + +When a server is combining receipts into an EDU, if there are multiple receipts for the same (user, event, receipt type), always +choose the receipt which is unthreaded (has no `thread_id`) when aggregating into an EDU. + +This change will apply to all `m.receipt` EDUs, which includes both CSAPI and Federation endpoints. + +For example, given these two EDUs: +```js +// EDU 1 +{ + "content": { + "$1435641916114394fHBLK:matrix.org": { + "m.read": { + "@erikj:jki.re": { + "ts": 1550000000000 + } + } + } + }, + "type": "m.receipt" +} + +// EDU 2 +{ + "content": { + "$1435641916114394fHBLK:matrix.org": { + "m.read": { + "@erikj:jki.re": { + "ts": 1559999999999, + "thread_id": "foo" + } + }, + "m.read.private": { + "@self:example.org": { + "ts": 1660000000000, + "thread_id": "bar" + } + } + } + }, + "type": "m.receipt" +} +``` +The unthreaded receipt wins and the combined EDU should become: +```js +// Combined EDU +{ + "content": { + "$1435641916114394fHBLK:matrix.org": { + "m.read": { + "@erikj:jki.re": { + "ts": 1550000000000 + } + }, + "m.read.private": { + "@self:example.org": { + "ts": 1660000000000, + "thread_id": "bar" + } + } + } + }, + "type": "m.receipt" +} +``` +Note: + - the data from the unthreaded receipt (the `ts`) is used. + - the threaded receipt for type `m.read.private` is kept, as the 3-uple (user, event, receipt type) must match + for the precedence rules to kick in. + +## Potential issues + +Some data is lost when aggregating EDUs, as the fact that a threaded receipt was sent will be lost. This is not a +problem in practice because an unthreaded receipt always takes precendence over a threaded receipt due to it +being a superset of the same data. + +## Alternatives + +Servers could split `m.receipt` EDUs and send threaded receipts in one EDU and unthreaded EDUs in another. This +creates additional EDUs, increasing bandwidth usage, processing time and increases complexity as servers need to +handle cases where EDUs are sent incompletely to remote clients/servers. + +Servers could instead always choose the threaded receipt instead of the unthreaded receipt. This would make it +harder to implement thread-unaware Matrix clients, as some receipt information is lost and can only be found by +introspecting the threaded receipts. + +## Security considerations + +None. + +## Unstable prefix + +No unstable prefix required as no new endpoints / JSON keys are added in this proposal. + +## Dependencies + +None. From 9a014a8fc77ae20886b9b350d805fad9eddd8c73 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Thu, 15 Feb 2024 17:34:13 +0000 Subject: [PATCH 2/3] Clarify the main thread --- proposals/4102-unthreaded-read-receipts.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/proposals/4102-unthreaded-read-receipts.md b/proposals/4102-unthreaded-read-receipts.md index 14a7c73d789..a7954952a4e 100644 --- a/proposals/4102-unthreaded-read-receipts.md +++ b/proposals/4102-unthreaded-read-receipts.md @@ -50,7 +50,7 @@ hypothetical problem: Element-Web will send both threaded and unthreaded receipt question is the most recent event in the room, which is fairly common. This becomes a major problem when the threaded receipt "wins" and is returned _instead of_ the unthreaded receipt. -Some clients, such as Element X, do not handle threads and will ignore receipts with a `thread_id`, assuming that +Some clients, such as Element X, do not handle threads and will ignore receipts with a non-main `thread_id`, assuming that clients will also be sending unthreaded receipts. When the threaded receipt "wins" however, these clients will simply not see the receipt, despite the sender sending it. @@ -131,6 +131,12 @@ Some data is lost when aggregating EDUs, as the fact that a threaded receipt was problem in practice because an unthreaded receipt always takes precendence over a threaded receipt due to it being a superset of the same data. +Thread-unaware clients need to inspect `thread_id: "main"` receipts to accurately get read receipts from thread-aware +clients, as that is how thread-aware clients send receipts for unthreaded events. Despite this, the +`main` thread ID has no significance in this proposal, and should be treated as any other thread ID when determining +precedence. This is because an unthreaded receipt (no thread ID) is still a superset of the `main` thread, just like +any other thread. + ## Alternatives Servers could split `m.receipt` EDUs and send threaded receipts in one EDU and unthreaded EDUs in another. This From 79c04bb0ad22d22b7c8cfcde2f83c30b1bada0ca Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Thu, 15 Feb 2024 17:35:50 +0000 Subject: [PATCH 3/3] tpyo --- proposals/4102-unthreaded-read-receipts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/4102-unthreaded-read-receipts.md b/proposals/4102-unthreaded-read-receipts.md index a7954952a4e..22dfcade137 100644 --- a/proposals/4102-unthreaded-read-receipts.md +++ b/proposals/4102-unthreaded-read-receipts.md @@ -128,7 +128,7 @@ Note: ## Potential issues Some data is lost when aggregating EDUs, as the fact that a threaded receipt was sent will be lost. This is not a -problem in practice because an unthreaded receipt always takes precendence over a threaded receipt due to it +problem in practice because an unthreaded receipt always takes precedence over a threaded receipt due to it being a superset of the same data. Thread-unaware clients need to inspect `thread_id: "main"` receipts to accurately get read receipts from thread-aware