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

Room marked as unread when I was the last person to speak #14837

Open
andybalaam opened this issue Jan 13, 2023 · 9 comments
Open

Room marked as unread when I was the last person to speak #14837

andybalaam opened this issue Jan 13, 2023 · 9 comments
Labels
A-Push Issues related to push/notifications A-Spec-Compliance places where synapse does not conform to the spec O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@andybalaam
Copy link
Contributor

Description

I have a room on matrix.org where the sync response shows me as the last person to send an event, but also has notification_count: 1. @gsouquet tells me that should never happen: if I sent the last event, the room should not be unread.

    "unread_notifications": {
          "notification_count": 1,
          "highlight_count": 0
    },

The room ID is !KCcFbqJHMQscdVsFTa:element.io

The URL I used to sync was: https://matrix-client.matrix.org/_matrix/client/r0/sync?filter=%7B%22room%22%3A%7B%22timeline%22%3A%7B%22unread_thread_notifications%22%3Atrue%2C%22limit%22%3A20%7D%2C%22state%22%3A%7B%22lazy_load_members%22%3Atrue%7D%7D%7D&timeout=0&_cacheBuster=1673621386081

My username is @andybalaam:matrix.org

The full sync response for this room is:

      "!KCcFbqJHMQscdVsFTa:element.io": {
        "timeline": {
          "events": [
            {
              "content": {
                "creator": "@lgian:element.io",
                "room_version": "9"
              },
              "origin_server_ts": 1669207170409,
              "sender": "@lgian:element.io",
              "state_key": "",
              "type": "m.room.create",
              "unsigned": {
                "age": 1651989
              },
              "event_id": "$T4yQFiEjK1NwvniRWmyjzdJQeJwSvz2TVh55BbPuhw8"
            },
            {
              "content": {
                "avatar_url": "mxc://element.io/079b96b843c4fe3f75712533f31233038d0d53d8",
                "displayname": "lgian",
                "membership": "join"
              },
              "origin_server_ts": 1669207170696,
              "sender": "@lgian:element.io",
              "state_key": "@lgian:element.io",
              "type": "m.room.member",
              "unsigned": {
                "age": 1651702
              },
              "event_id": "$STEseiYuCwtuJgj2nJH4_l2mq-b7aZJGMXefty9Yki4"
            },
            {
              "content": {
                "ban": 50,
                "events": {
                  "m.room.avatar": 50,
                  "m.room.canonical_alias": 50,
                  "m.room.encryption": 100,
                  "m.room.history_visibility": 100,
                  "m.room.name": 50,
                  "m.room.power_levels": 100,
                  "m.room.server_acl": 100,
                  "m.room.tombstone": 100
                },
                "events_default": 0,
                "historical": 100,
                "invite": 0,
                "kick": 50,
                "redact": 50,
                "state_default": 50,
                "users": {
                  "@andybalaam:matrix.org": 100,
                  "@lgian:element.io": 100
                },
                "users_default": 0
              },
              "origin_server_ts": 1669207170782,
              "sender": "@lgian:element.io",
              "state_key": "",
              "type": "m.room.power_levels",
              "unsigned": {
                "age": 1651616
              },
              "event_id": "$DoEVRqa-hHhg1GJdnlT7X_BPXgWpvv3ACp9akZ6pjc8"
            },
            {
              "content": {
                "join_rule": "invite"
              },
              "origin_server_ts": 1669207170814,
              "sender": "@lgian:element.io",
              "state_key": "",
              "type": "m.room.join_rules",
              "unsigned": {
                "age": 1651584
              },
              "event_id": "$5XX9KB-PNpUmp_zphOKtNVRdSiPPDwKO-zo1TWrsQDQ"
            },
            {
              "content": {
                "history_visibility": "shared"
              },
              "origin_server_ts": 1669207170820,
              "sender": "@lgian:element.io",
              "state_key": "",
              "type": "m.room.history_visibility",
              "unsigned": {
                "age": 1651578
              },
              "event_id": "$AoRJeIFLou1NpTPo8ydqXFV-5LyeXCDnT8NhJfot7nY"
            },
            {
              "content": {
                "guest_access": "can_join"
              },
              "origin_server_ts": 1669207170829,
              "sender": "@lgian:element.io",
              "state_key": "",
              "type": "m.room.guest_access",
              "unsigned": {
                "age": 1651569
              },
              "event_id": "$10LzQqIAeVJd6m5jbQ5iFZuIdUmNcCnj0k3-Yqkl6GE"
            },
            {
              "content": {
                "algorithm": "m.megolm.v1.aes-sha2"
              },
              "origin_server_ts": 1669207170837,
              "sender": "@lgian:element.io",
              "state_key": "",
              "type": "m.room.encryption",
              "unsigned": {
                "age": 1651561
              },
              "event_id": "$ZGp0VZGbACUCESNl8F7k2GM_w3umBFrhzPD17eR834Q"
            },
            {
              "content": {
                "avatar_url": "mxc://matrix.org/yqsMYtlqLZIlMOnqBRAXnkUm",
                "displayname": "andybalaam",
                "is_direct": true,
                "membership": "invite"
              },
              "origin_server_ts": 1669207171121,
              "sender": "@lgian:element.io",
              "state_key": "@andybalaam:matrix.org",
              "type": "m.room.member",
              "unsigned": {
                "age": 22
              },
              "event_id": "$rWoSNtthPJ1WHmoO_qqvX-Q-Jmy9dTqKfKZFTw4idPg"
            },
            {
              "content": {
                "algorithm": "m.megolm.v1.aes-sha2",
                "ciphertext": "AwgAEuABEy11curBDRBMoSJ7GLUujtDjycv2JXuIIqTrWNwVZr3tW4/YKBxUcD1SnBmffgoHDFlo9dTq03YAjg/sv2r/rcVb1xBwTJejQdOyVmL/QdLPLSWj3vgFWpA+jw1bV57sYyfawONZ5+hfemlZLClOjxZgD/IZ2/PmXlbStamj2uXbo2MrUiA4y/VhLCzvXZIeh5gs7WdxZXoanh0l9TAFISZtSr4GO60xL6GrbZ/oXf63WtDjZQ293eGr7xDMaMoF2qRycimZGQ3FX1+2N/peOM+p0PM0cgekP0rSqcLuRg3pq+dtF8guo9T/IhTIny25SmOSvmawc6pqXRO5WQI4hyfpNEmWLKTO9z/zU6nUGsDZMJMLFNnlGEwjpHZGO4ogQ1tyK8KWcQc",
                "device_id": "RJPFIRDZYD",
                "sender_key": "HlpwBwe9UsFhQ+WO7nJHI1CwjeXxPOOVCXE7zgzbCGg",
                "session_id": "gAQxX0A0RM2b+up0ibRb68u0gEm8pDQgrtw9w9F9hCo"
              },
              "origin_server_ts": 1669207174410,
              "sender": "@lgian:element.io",
              "type": "m.room.encrypted",
              "unsigned": {
                "age": 1648859
              },
              "event_id": "$MK3coNBUiKdbiwJbX3BZnYQsRFJE2DE5K7LSy5RBfYg"
            },
            {
              "content": {
                "algorithm": "m.megolm.v1.aes-sha2",
                "ciphertext": "AwgBEqABuFUjuZZaYZ0sd4tKqwZ3KBI1ULzLvINFHtPF053Bh8QqGDTx/Tgn6t5MFY/R1Oa7j1jNz6o5nH2kvj/30J5/w2e+R2ieZzjGhxzW1WsSu7DY+QTNipH4euorYp2R0BdawL+UHElQYZoYMg4ytVY+1fJRSF0aTPlXUjy4Wd1ZS9uzV9GtqMx4ImFtXmOeQSTUNEvllOEZxI4BAS5vuJt6HYrcLtuld7jiZlnvHHc8zuKLKqorMP06vuu4Bwx3OSy2zg5gf4ruXcAbP/jIv0mADpr2EK/Ez/eOmPlnKiDmCUUK6dfmNSnkAQ",
                "device_id": "RJPFIRDZYD",
                "sender_key": "HlpwBwe9UsFhQ+WO7nJHI1CwjeXxPOOVCXE7zgzbCGg",
                "session_id": "gAQxX0A0RM2b+up0ibRb68u0gEm8pDQgrtw9w9F9hCo"
              },
              "origin_server_ts": 1669207397906,
              "sender": "@lgian:element.io",
              "type": "m.room.encrypted",
              "unsigned": {
                "age": 1425363
              },
              "event_id": "$XO_0WwJAi40qcMUdOFqB4xvdY1mQP7jjdiKPKIkdS9k"
            },
            {
              "content": {
                "avatar_url": "mxc://matrix.org/yqsMYtlqLZIlMOnqBRAXnkUm",
                "displayname": "andybalaam",
                "membership": "join"
              },
              "origin_server_ts": 1669208822225,
              "sender": "@andybalaam:matrix.org",
              "state_key": "@andybalaam:matrix.org",
              "type": "m.room.member",
              "unsigned": {
                "age": 3,
                "replaces_state": "$rWoSNtthPJ1WHmoO_qqvX-Q-Jmy9dTqKfKZFTw4idPg",
                "prev_content": {
                  "avatar_url": "mxc://matrix.org/yqsMYtlqLZIlMOnqBRAXnkUm",
                  "displayname": "andybalaam",
                  "is_direct": true,
                  "membership": "invite"
                },
                "prev_sender": "@lgian:element.io"
              },
              "event_id": "$Wk2qUVwqsb8ive4l75sISoivifuXdm1l3iGEODpmm1s"
            },
            {
              "content": {
                "algorithm": "m.megolm.v1.aes-sha2",
                "ciphertext": "AwgAEqABvDb35eiTzLREumzMNc9nezQNMwKRlh9r9nHaPg8F4EBrVh6YIp+lgEpzhuUHYqCDIVF0j9jRZ693IsUxemQnN4cJbwGo/1GAqFovLZNXV6Q7MYxukgtPiVnyFNsYnd8yGHG90TiGRO5Wl8BJQ+gzqRqKjt6Ums3c8iNeSDmmwYrYxA+LcRGVTaFiey+ixg49XQ+R8Y8msVFnNzF1e48F/ZKBNs+ToaA3ks0younKFNIK0UEV0/z/evDcBsSqUf9W/23u/9RodXEswHHhUS6eE44HC9iOi7m6LZBt0YFMWfteBsE9gxMSDw",
                "device_id": "MKBDKTQJQJ",
                "sender_key": "Ss6eaJGA82Uc9TQn1BbcLqpm6jtjGGwqMBadf/8Y+WY",
                "session_id": "8RV7u4xrlnPO5yvqZpx/aqiHbq3+KTTYg193IpzzFPY"
              },
              "origin_server_ts": 1669208839345,
              "sender": "@andybalaam:matrix.org",
              "type": "m.room.encrypted",
              "unsigned": {
                "age": 4412625253
              },
              "event_id": "$dER5V1RCMxzAhHXQJoMjqyuoxpPtK2X6hCb9T8Jg2wU"
            }
          ],
          "prev_batch": "s3615474541_757284974_4801760_1841502201_1869275239_3913936_731927069_6351869257_0",
          "limited": false
        },
        "state": {
          "events": []
        },
        "account_data": {
          "events": [
            {
              "type": "m.fully_read",
              "content": {
                "event_id": "$dER5V1RCMxzAhHXQJoMjqyuoxpPtK2X6hCb9T8Jg2wU"
              }
            }
          ]
        },
        "ephemeral": {
          "events": [
            {
              "type": "m.receipt",
              "content": {
                "$XO_0WwJAi40qcMUdOFqB4xvdY1mQP7jjdiKPKIkdS9k": {
                  "m.read": {
                    "@andybalaam:matrix.org": {
                      "ts": 1669208824249
                    }
                  },
                  "m.read.private": {
                    "@andybalaam:matrix.org": {
                      "ts": 1669208824233
                    }
                  }
                },
                "$dER5V1RCMxzAhHXQJoMjqyuoxpPtK2X6hCb9T8Jg2wU": {
                  "m.read": {
                    "@lgian:element.io": {
                      "ts": 1669208845306
                    }
                  }
                }
              }
            }
          ]
        },
        "unread_notifications": {
          "notification_count": 1,
          "highlight_count": 0
        },
        "summary": {
          "m.joined_member_count": 2,
          "m.invited_member_count": 0,
          "m.heroes": [
            "@lgian:element.io"
          ]
        }
      },

Steps to reproduce

.

Homeserver

matrix.org

Synapse Version

{"server_version":"1.75.0rc1 (b=matrix-org-hotfixes,6a185c8b17)","python_version":"3.8.12"}

Installation Method

I don't know

Database

.

Workers

Multiple workers

Platform

.

Configuration

.

Relevant log output

.

Anything else that would be useful to know?

.

@clokep
Copy link
Member

clokep commented Jan 13, 2023

@gsouquet tells me that should never happen: if I sent the last event, the room should not be unread.

I'll need to double check, but I don't think Synapse has any code to mark things as read when a new event comes in. (There's no implicit receipt created in this case.)

I thought I saw code in Element Web somewhere that ignored unread counts if you had sent the last event in the room?

@DMRobertson DMRobertson added T-Other Questions, user support, anything else. X-Needs-Info This issue is blocked awaiting information from the reporter labels Jan 17, 2023
@clokep
Copy link
Member

clokep commented Jan 19, 2023

I did some testing and this seems to be working as expected. I did something like:

  1. Have two users (alice & bob) join a room.
  2. Send a message and set both alice & bob's read receipt to it.
  3. Sync & ensure 0 unread notifications.
  4. Send a message from each user.
  5. Sync again and each user has 1 unread message.

From what I can tell in the spec this is expected behavior and clients should be sending a read receipt.

@gsouquet tells me that should never happen: if I sent the last event, the room should not be unread.

@gsouquet can you expand on why you think this shouldn't be possible?

@germain-gg
Copy link
Contributor

There is some logic in the client that says that if you're the last person who sent a message to a room, it is implied that you've read the entire conversation up until that point.
And you don't send a read receipt for your own messages to Synapse. At least that was the impression I was under.

Maybe something has regressed in Element after all, 🤔

@clokep
Copy link
Member

clokep commented Jan 19, 2023

There is some logic in the client that says that if you're the last person who sent a message to a room, it is implied that you've read the entire conversation up until that point.

Can you link? Is this the logic that broke?

@germain-gg
Copy link
Contributor

https://github.com/matrix-org/matrix-js-sdk/blob/8e29f8ead0bd0341563469ab43a6ac87ae1c39e8/src/models/room.ts#L2253

Me and Andy looked into that, and it seems that this line is called in Element Web when loading from the cache.
But maybe my understanding of synapse's role was incorrect. I thought that sending an event in the room would clear the notifications, which is an incorrect assumption from what you're saying. So i will look further into what changed client-side.

@clokep
Copy link
Member

clokep commented Jan 19, 2023

I thought that sending an event in the room would clear the notifications, which is an incorrect assumption from what you're saying.

I don't see anything in the code which connects event sending and clearing notifications, but I could be missing it!

@clokep
Copy link
Member

clokep commented Jan 19, 2023

matrix-org/matrix-js-sdk@8e29f8e/src/models/room.ts#L2253

Me and Andy looked into that, and it seems that this line is called in Element Web when loading from the cache. But maybe my understanding of synapse's role was incorrect.

I'm a little suspicious that is related since it doesn't seem to do any checks against the current user? I think what it is doing is saying:

  1. Alice has a receipt on event A.
  2. Bob and Charlie talk a bunch and create events B, C, and D.
  3. (Note that Alice hasn't sent an updated read receipt for whatever reason, e.g hidden read receipts, broken federation, who knows.)
  4. Alice sends event E.

We know that Alice must have read at least up-to event E since she sent it?

I might be misunderstanding what that code is attempting to do, but that's what the comments around it imply.

@clokep
Copy link
Member

clokep commented Jan 24, 2023

I thought that sending an event in the room would clear the notifications, which is an incorrect assumption from what you're saying.

I don't see anything in the code which connects event sending and clearing notifications, but I could be missing it!

It seems that the spec, however, does say we should be doing this. See matrix-org/matrix-spec#1410, which quotes the spec:

When the user updates their read receipt (either by using the API or by sending an event), notifications prior to and including that event MUST be marked as read.

I still don't think that Synapse has ever done this though?

@H-Shay H-Shay added A-Spec-Compliance places where synapse does not conform to the spec T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. and removed T-Other Questions, user support, anything else. X-Needs-Info This issue is blocked awaiting information from the reporter labels Jan 31, 2023
@MadLittleMods MadLittleMods added the A-Push Issues related to push/notifications label Apr 25, 2023
@t3chguy
Copy link
Member

t3chguy commented Aug 4, 2023

I don't see anything in the code which connects event sending and clearing notifications, but I could be missing it!

The code is on the receiving side ftr, if we see an event from a user we generate a synthetic receipt for them

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Push Issues related to push/notifications A-Spec-Compliance places where synapse does not conform to the spec O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

7 participants