-
Couldn't load subscription status.
- Fork 414
MSC4140: Cancellable delayed events #4140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Timo K <[email protected]>
Signed-off-by: Timo K <[email protected]>
2bc07c4 to
0eb1abc
Compare
Signed-off-by: Timo K <[email protected]>
0eb1abc to
8bf6db7
Compare
Signed-off-by: Timo K <[email protected]>
Signed-off-by: Timo K <[email protected]>
3e54c2a to
c82adf7
Compare
Signed-off-by: Timo K <[email protected]>
c82adf7 to
54fff99
Compare
…is used to trigger on of the actions Signed-off-by: Timo K <[email protected]>
Signed-off-by: Timo K <[email protected]>
Add event type to the body Add event id template variable
Co-authored-by: Andrew Ferrazzutti <[email protected]>
Redundant because GitHub's Markdown viewer provides its own ToC. Stale because some of the headings refered to absent sections.
but state events are still used in the MatrixRTC section
It is either the send time, or the cancel/error time
… v11 using the /send endpoint (#18898) Implement [MSC4169](matrix-org/matrix-spec-proposals#4169) While there is a dedicated API endpoint for redactions, being able to send redactions using the normal send endpoint is useful when using [MSC4140](matrix-org/matrix-spec-proposals#4140) for sending delayed redactions to replicate expiring messages. Currently this would only work on rooms >= v11 but fail with an internal server error on older room versions when setting the `redacts` field in the content, since older rooms would require that field to be outside of `content`. We can address this by copying it over if necessary. Relevant spec at https://spec.matrix.org/v1.8/rooms/v11/#moving-the-redacts-property-of-mroomredaction-events-to-a-content-property --------- Co-authored-by: Tulir Asokan <[email protected]>
| Content-Type: application/json | ||
| { | ||
| "scheduled": [ |
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.
Looks like this is still called delayed_events in the Synapse implementation: https://github.com/element-hq/synapse/blob/v1.140.0rc1/synapse/rest/client/delayed_events.py#L103, should mention that in the unstable prefix section. It was renamed in 904e3d6
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.
That deviation (among others) is being addressed by element-hq/synapse#19038.
|
|
||
| ### Managing delayed events | ||
|
|
||
| A new authenticated client-server API endpoint at `POST /_matrix/client/v1/delayed_events/{delay_id}` allows scheduled events |
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.
To throw this on its head a bit, could we make this unauthenticated and instead make the delay_id a more secure random string. I have a fairly strong use case:
If I am building a RTC application and I want a backend service to be in control of whether one of my events expires, then I can pass my delayed event to the server and it can control the disconnect event on my behalf. Because the delay_id is scoped to one event that the user has already consented to sending, the auth doesn't really add much if the delay_id isn't trivally guessable.
The alternative to this is the backend service would need to get either my full token or a scoped access token (via as an as-of-yet unspec'd MSC) which adds some overhead.
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.
We discussed this out of band and decided it was a good idea, right @fkwp ?
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.
| data. Since the additional capability to use a template `event_id` parameter is also needed, | ||
| this probably is not a good fit. | ||
|
|
||
| ### Not reusing the `send`/`state` endpoint |
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.
I would highly prefer that.
- An endpoint should not return two completely different response types depending on the query parameter.
- Sending a delayed event is a different action than sending an event and should be more explicit.
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.
Its about the send body bein equivalent in both requests.
Sticky events also take the exact same appraoch.
I think the opinion on this very much depends on your mental model of delayed events
- they are normal event sending actions but with a configurable increase in latency (it will take the hs a little bit to send the event anyways, you can just further delay this manually)
- This endpoint schedules sth that is not a matrix event yet. Its a new entity that eveutally becomes a matrix event.
I like the first view on it since it makes it easier to justify how this is compatible with matrix and why this probably wont change anything fundamental (matrix already needs to be capable dealing with differen network delays)
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.
As an SDK developer, I prefer a type safe API. It would've been helpful to have this natively in Matrix instead making a workaround at the SDK level to get type-safety. At the end, I will provide a type safe API to the user anyway.
| Since the delayed event is sent first, a client can guarantee (at the time they are sending | ||
| the join event) that it will eventually leave. | ||
|
|
||
| ### Self-destructing messages |
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.
This is a really good use case, but I see a fundamental problem in this MSC: Delayed events are not passed down the sync. So even if one device sends a self destruction, other devices of the same account would not be notified about it in the sync. Therefore other devices would always need to poll delayed_events, which breaks the concept of having the Matrix sync and would unnecessary flood the homeserver with requests.
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.
I don't think there's any need to know about delayed events in that context. The sender creates the delay, other devices don't care. Beeper already implements disappearing messages like that.
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.
If this is just about the user experience and communicating your intent to only let this message life for X minutes, that seems like an easy addition in a new MSC. A content fieled: "scheduled_redaction_ms" could be used to share when you plan to let this message disappear and clients can render some UI around that.
This is off topic, but if a burn on read semantic (in a DM) is desired. one could even go as far as sharing a link with a scoped token that sends the delayed redaction. So once received the receiver can than delete the message by sending the redaction scheduled by the sender of the message.
What I am trying to say, this MSC supports all the fundamentals for a really good self-destruction implementation. I am not sure it needs fundamental changes, but maybe some metadata on top is reaquired to check of UX features.
But this MSC proposes the general delayed event logic and is not specific to self-destructing messages.
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.
other devices don't care
If that is the case, do we need the polling api GET delayed_events? (we could close this thread, because the other is regarding the same topic)
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.
Querying delayed events is necessary for other things like scheduled messages even though it's not needed for disappearing messages. Scheduled messages don't need a push mechanism, it's enough to be able to view and manage scheduled events when the user navigates to a specific view inside a room.
If this is just about the user experience and communicating your intent to only let this message life for X minutes, that seems like an easy addition in a new MSC. A content fieled: "scheduled_redaction_ms" could be used to share when you plan to let this message disappear and clients can render some UI around that.
This is indeed how Beeper works (using a com.beeper.disappearing_timer object) and that'll probably be MSC'd at some point
| The primary point of rate limiting is event sending when the delay times out or the event is sent using the `send` | ||
| action. However, servers can choose to rate limit the management endpoints themselves as well if necessary. | ||
|
|
||
| ### Getting delayed events |
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.
This is a poll mechanism. In my opinion, we would need a push (sync) mechanism to take really advantage of this MSC and be able to introduce a bunch of new Matrix features based on it.
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.
Similar to the above.
There even is a follow up msc moving this into the sync block.
But your comments implicitly ask the question who should be able to access the list of scheduled delayed events: "The sender" vs "All room members". Right now the idea is that only the sender knows about their schedueld delayed events. (A dag like federated data exachange is required to sync inforamtion to all room members on other homeservers so sharing a schedueld delayed event is not as trivial, there is a reason matrix writes things into a room dag.)
The current MSC only ever exposes the scheduled delayed events to the sender.
This also has privacy/security advantages.
Whenever it is desired to share shedued data with the room metadata in the conetent of antoher room event should be used.
This might be worth explicitly mentioning in the MSC.
I hope this approach makes sense and coveres all of the usecases you have in mind?
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.
I don't expect to share delayed_events between users, but between devices of the same user (like account data). If we don't want that, which is totally okay, do we want an endpoint to get all delayed_events on an account base.
The other problem I see is, that the MSC suggest a new endpoint for getting delayed_events. It is poll based. As far as I know in Matrix there is no other data structure, that needs polling. What is the use case to poll delayed_events instead of syncing them? Why this "downgrade" regarding the Matrix push-first principle.
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.
There is a follow up MSC to include it in sync (sorry that i confused user vs room delayed events sharing)
The "all devices for one user" push/sync based system has been moved to that msc:
#4309
The reason was to make this msc simpler.
It does only include finalized delayed events. So your point of syncing schedules to all devices via a push semantic is very valid.
What do you think about moving/discussing this functionality to the other MSC?
(It sadly is still in draft)
Also: - split them into one endpoint per management action - propose alternative of OAuth 2.0 scoped access for the endpoints
and list it as what the Synapse implementation currently uses
| ``` | ||
|
|
||
| Where the `action` is `send`, the homeserver **should** apply rate limiting to provide mitigation against the | ||
| These endpoints are unauthenticated so that control over a particular delayed event may be |
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.
Rendered
This could also supersede MSC2228 (by making it possible to send a redaction with the
/sendendpoint. This is the case as mentioned here)Implementations:
SCT stuff:
checklist
FCP not yet started
Blocked: #4140 (comment)