MSC4140: add dedicated endpoint for delayed events#5133
MSC4140: add dedicated endpoint for delayed events#5133AndrewFerr wants to merge 4 commits intodevelopfrom
Conversation
Use a new, dedicated endpoint for scheduling delayed events. This should deprecate the `delay` query parameter on the `/send` and `/state` endpoints.
In methods that were updated to await a promise, don't later directly return a promise without awaiting it.
| // For backwards compatibility with implementations of MSC4140 that | ||
| // do not support a dedicated endpoint for adding delayed events | ||
| if (!(e instanceof MatrixError && e.errcode === "M_UNRECOGNIZED")) { | ||
| throw e; | ||
| } | ||
| return await this.http.authedRequest( | ||
| Method.Put, | ||
| utils.encodeUri("/rooms/$roomId/state/$eventType/$stateKey", { | ||
| $stateKey: stateKey, | ||
| ...pathParams, | ||
| }), | ||
| getUnstableDelayQueryOpts(delayOpts), | ||
| content, | ||
| opts, | ||
| ); |
There was a problem hiding this comment.
It'd be better to adapt the MSC to communicate over /versions or /capabilities so as to not need to make 2 calls every time for the majority of servers
There was a problem hiding this comment.
Using /capabilities is a great idea. I wasn't aware that it allowed custom identifiers.
The MSC already defines a /versions key for whether the feature is enabled or not (which Synapse and the js-sdk make use of), so /capabilities is a good place to put more fine-grained details on the implementation of it.
There was a problem hiding this comment.
Update: spec folks have informed me that this is more under the purview of /versions than /capabilities. I'll update the MSC with appropriate new /versions keys before putting them in this PR.
| private async sendEventHttpRequest( | ||
| event: MatrixEvent, | ||
| queryOrDelayOpts?: SendDelayedEventRequestOpts | QueryDict, | ||
| queryDict?: QueryDict, | ||
| ): Promise<ISendEventResponse | SendDelayedEventResponse> { |
There was a problem hiding this comment.
Since we now separate the two endpoints (delay vs send) I am not sure I am at all happy with this function.
- We do complicated overloads which are only used to differentiate between the return types (delayId vs eventId)
- And in the body we then completely split the impl based on the params.
In other words: It all looks to me like there should be sendEventHttpRequest and sendDelayedEventHttpRequest
There was a problem hiding this comment.
I am not sure how deep we should go. I think encrypt and send event is suffering from the same overload concept. and sendCompleteEvent event as well. Not sure if they both should also be seperate functions?
3 participants
Use a new, dedicated endpoint for scheduling delayed events. This should deprecate the
delayquery parameter on the/sendand/stateendpoints.See:
Checklist
public/exportedsymbols have accurate TSDoc documentation.