Threaded message is missing my reaction

[MadLittleMods]

Steps to reproduce

1. Open a thread
2. Put a reaction on one of the messages
   • It's probably related to the fact that the threaded message was edited but my client wasn't showing the edit at the time and I reacted to the original message instead of the edit. In any case, it seems like the reaction should follow.
3. Come back to the room later and notice your reaction isn't displayed

Related to #21627

Thread information

mxMatrixClientPeg.matrixClient.getRoom("!xxx:matrix.org").findEventById("$threadRoot").getThread().events.length
28

When I look at the events in mxMatrixClientPeg.matrixClient.getRoom("!xxx:matrix.org").findEventById("$threadRoot").getThread().events, it does include my reaction. Full output in the rageshake issue.

Relevant events

Full uncensored screenshots, context, event_ids, etc in the rageshake log issue: https://github.com/matrix-org/element-web-rageshakes/issues/12869

The original message being reacted to:

{
    "content": {
        "algorithm": "m.megolm.v1.aes-sha2",
        "ciphertext": "...",
        "m.relates_to": {
            "event_id": "$E74-sH4t54_0GjTZDBiPdm7Wq1pb1_eHV_VdHUe6wac",
            "is_falling_back": true,
            "m.in_reply_to": {
                "event_id": "$UnyKkkxehyjqYR66zfb74F2OtnV7ts-L9fd83msrNnQ"
            },
            "rel_type": "m.thread"
        },
    },
    "origin_server_ts": 1652201438062,
    "room_id": "!xxx:matrix.org",
    "sender": "@alice",
    "type": "m.room.encrypted",
    "unsigned": {
        "age": 101777709,
        "m.relations": {
            "m.annotation": {
                "chunk": [
                    {
                        "type": "m.reaction",
                        "key": "😢",
                        "count": 1
                    }
                ]
            }
        }
    },
    "event_id": "$yYGxATPiCuhVCK1cKiCXB9MSXUyTZcXTVBjAZNT5fGM",
    "age": 101777709
},

The edit:

{
  "content": {
    "algorithm": "m.megolm.v1.aes-sha2",
    "ciphertext": "....",
    "m.relates_to": {
      "event_id": "$yYGxATPiCuhVCK1cKiCXB9MSXUyTZcXTVBjAZNT5fGM",
      "rel_type": "m.replace"
    },
  },
  "origin_server_ts": 1652201489856,
  "room_id": "!xxx:matrix.org",
  "sender": "@alice",
  "type": "m.room.encrypted",
  "unsigned": {
    "age": 101296010
  },
  "event_id": "$gEFlk0Jea101_sk8sI6Vq6NxSv21bXcT3ILaNn7al1Q",
  "age": 101296010
}

My reaction. Interestingly, my reaction comes after the edit in time, but applies to the original message. This probably means my client was showing me out of date information in the first place.

{
  "content": {
    "m.relates_to": {
      "event_id": "$yYGxATPiCuhVCK1cKiCXB9MSXUyTZcXTVBjAZNT5fGM",
      "key": "😢",
      "rel_type": "m.annotation"
    }
  },
  "origin_server_ts": 1652250027013,
  "room_id": "!xxx:matrix.org",
  "sender": "@madlittlemods:matrix.org",
  "type": "m.reaction",
  "unsigned": {
    "age": 52235505
  },
  "event_id": "$cQpsz6y51fHg9THifzRLG7ABAFODMVTnEuM1I3-11C8",
  "user_id": "@madlittlemods:matrix.org",
  "age": 52235505
}

Outcome

What did you expect?

Reaction is shown on the threaded message

What happened instead?

Threaded message is missing my reaction

Operating system

Windows 10

Browser information

Chrome Version 101.0.4951.64

URL for webapp

https://develop.element.io/

Application version

Element version: 479d4bf-react-5348572439be-js-4d4d6e1411b6 Olm version: 3.2.8

Homeserver

matrix.org

Will you send logs?

Yes

[t3chguy]

This is #9743

[MadLittleMods]

@t3chguy Thanks for the link!

Reading over there, even though the client has both the reaction and target event, it sounds like we don't try to apply reactions we stumble upon in client to their target events because those events could already have bundled unsigned data included and we have no way to de-duplicate the existing bundled data to the random reaction event ID.

It looks like this does reproduce a tricky situation:

1. Start a thread
2. Add some thread replies
3. React to one of the replies
4. Edit the reply you reacted to
5. Clear cache and refresh
6. Notice the thread now has a Can't load this message (m.room.message) Submit logs (tile error boundary) for the message in question

react_devtools_backend.js:3973 TypeError: Cannot read properties of null (reading 'getMyMembership')
   at ReactionsRow_ReactionsRow.render (ReactionsRow.tsx:170:32)
   at qi (react-dom.production.min.js:187:125)
   at pi (react-dom.production.min.js:186:166)
   at ck (react-dom.production.min.js:269:357)
   at bk (react-dom.production.min.js:250:341)
   at ak (react-dom.production.min.js:250:277)
   at Tj (react-dom.production.min.js:250:138)
   at Lj (react-dom.production.min.js:243:146)
   at react-dom.production.min.js:123:110
   at push.exports.unstable_runWithPriority (scheduler.production.min.js:18:336)
   at gg (react-dom.production.min.js:122:318)
   at jg (react-dom.production.min.js:123:61)
   at ig (react-dom.production.min.js:122:428)
   at Jg (react-dom.production.min.js:237:147)
   at Object.enqueueSetState (react-dom.production.min.js:133:211)
   at TimelinePanel_TimelinePanel.push.C.setState (react.production.min.js:12:356)
   at onLoaded (TimelinePanel.tsx:1279:18)

@t3chguy Seems like this deserves a separate issue to at least not crash on a message? Update: #22195 ⏩

[MadLittleMods]

What makes this work sometimes? I just checked in on the problem message today and the reaction is showing. Synapse is not bundling the information under unsigned on the edited message so it seems like the client is doing something smart sometimes. What's the differentiator?

The client had all of the information before. It had the reaction, the target event, and even had the reaction within the thread according to the matrix-js-sdk view of things.

[t3chguy]

Before the fix you commented on last week there was a race condition, that should be solved now given the async code is gone. If the reaction is seen via /sync or the /messages stream it'll be correctly related, if its in unsigned then it will not be due to lack of #9743.

[MadLittleMods]

Before the fix you commented on last week there was a race condition, that should be solved now given the async code is gone.

When did that fix ship? (I assume last week) Which one is it?

If the reaction is seen via /sync or the /messages stream it'll be correctly related, if its in unsigned then it will not be due to lack of #9743.

According to this, I feel like I shouldn't have seen the original reproduction which was only a couple days ago, given that Synapse wasn't bundling the reaction in unsigned under the edit before either and the client had the reaction event from somewhere (probably /messages).

[t3chguy]

When did that fix ship? (I assume last week) Which one is it?

matrix-org/matrix-js-sdk#2331 (comment)

According to this, I feel like I shouldn't have seen the original reproduction which was only a couple days ago, given that Synapse wasn't bundling the reaction under the edit before either and the client had the reaction event from somewhere (probably /messages).

In the OP of this issue you shared the event JSON which showed Synapse bundling the reaction.

[MadLittleMods]

hmm, you mean bundled in unsigned under the the original message, not the edit.

I feel like in any case, the data available in the situation isn't different from today (where I can see it), to a couple days ago (where I couldn't).

I guess the differentiator could be:

1. Context: The original message and edit are available on the homeserver time-wise by the time I send the reaction.
2. My client is showing the outdated original message
3. I reacted on the original
4. The next day, the edit is slurped up somehow and available to show
5. I view the thread and the messages come from the cache where they the original message won't have unsigned because it's cached from before I added my reaction.
   • I have no idea if the cache actually came into play here. Just trying to come up with some way to explain the difference.

But it's still not clear to me why the reaction wouldn't be shown in this case. It still seems possible given the constraints (we're not fighting against deduplicating unsigned here and even if the reaction was in unsigned, that's still my reaction I expect and everything would display correctly).

[t3chguy]

I don't think it would ever be bundled under the edit, bundled relationships are only 1st-order at the root level

[MadLittleMods]

I don't think it would ever be bundled under the edit, bundled relationships are only 1st-order at the root level

That makes sense. Thanks for the clarification 🙂

If the reaction is seen via /sync or the /messages stream it'll be correctly related, if its in unsigned then it will not be due to lack of #9743.

-- #22157 (comment)

Are these two conditions mutually exclusive?

To clarify this point, the original message did have the reaction bundled under unsigned which is biasing it to not be shown according to that statement.

If we discover reactions from /sync or /messages, will the reactions be added to the target message even if it has unsigned? It feels like we will show reactions given that the reaction is showing today.

And from a few days ago, I could see the hidden reaction event in the timeline (which means it was discovered via /messages), while looking at the thread where the reaction was missing. This seems like a valid bug?

[t3chguy]

If we discover reactions from /sync or /messages, will the reactions be added to the target message even if it has unsigned? It feels like we will show reactions given that the reaction is showing today.

Yes

And from a few days ago, I could see the hidden reaction event in the timeline (which means it was discovered via /messages), while looking at the thread where the reaction was missing. This seems like a valid bug?

That should be the bug fixed by the async->sync js-sdk changes