-
-
Notifications
You must be signed in to change notification settings - Fork 833
Always pass recent decryption retry successes to widgets #12890
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semantics seem a bit confused now: there's a comment saying that this code is meant to prevent out-of-order delivery of events, but this new branch appears to deliberately do the opposite, forwarding events in whatever order they're decrypted. I would expect to see this code block the delivery of future events on whether past events have been decrypted, I think.
I can also see why this behavior could be more desirable though (everything gets through without any head-of-line blocking). Maybe I need to read up on the semantics that the widget API prescribes again.
Yeah, it wasn't obvious to me what the original intention was. If the intention is that a Widget should only receive events from the point at which it was added/started on the room then a simpler, time based filtering could be applied. The current implementation appears to give some history and then I don't think it allows for late decryptions. |
Okay, I finally found the MSC that defines how widgets receive events: matrix-org/matrix-spec-proposals#2762 Notably it says,
So that suggests that we shouldn't be sending historical messages at all. The same MSC gives a way for widgets to backfill events on-demand, so there's still a proper way for widgets to get this information if they want it. However, the MSC ultimately has nothing to say about the order in which events are pushed to the widget. So, this situation is still just as tricky: clearly, an effort was made to have this code push events strictly in the same order that they're received, but clearly, this also means that you must accept either head-of-line blocking or unreliable delivery as a trade-off. The possible ways forward I see are:
|
Notes from discussion:
This PR doesn't implement this preferred way, so we're going to close this PR. Too big of a can of worms for now 😛 |
Potential fix for element-hq/element-call#2561.
It looks like the current behaviour was set by #6695 for element-hq/element-web#18060. But, I don't know if the scenario I am encountering was considered. So, this might be the wrong way to go about it.
I tried to add some test cases that cover the basic encrypted event semantics too.
Checklist
public
/exported
symbols have accurate TSDoc documentation.