Skip to content
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

MSC2762: Allowing widgets to send/receive events #2762

Open
wants to merge 12 commits into
base: old_master
Choose a base branch
from

Conversation

@turt2live turt2live changed the title MSC0000: Allowing widgets to send/receive events MSC2762: Allowing widgets to send/receive events Sep 2, 2020
@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels Sep 2, 2020
@turt2live turt2live mentioned this pull request Sep 2, 2020
6 tasks
turt2live added a commit to matrix-org/matrix-widget-api that referenced this pull request Nov 3, 2020
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this pull request Nov 3, 2020
Part of MSC2762: matrix-org/matrix-spec-proposals#2762
Requires: matrix-org/matrix-widget-api#9

This is the bare minimum required to send an event to a widget and receive events from widgets. Like the view_room action, this is controlled by a well-known permission key.

**Danger**: This allows widgets to potentially modify room state. Use the permissions with care.
@turt2live
Copy link
Member Author

This has been implemented in element-web: matrix-org/matrix-widget-api#9 & matrix-org/matrix-react-sdk#5385

@turt2live turt2live added the widgets anything to do with widgets label Nov 23, 2020
are introduced, with the same formatting requirements as the `m.send.event` and `m.send.state_event`
capabilities above (ie: `m.receive.event:m.room.message#m.text`).

For each event type requested and approved, the client sends a `toWidget` request with action `event`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should local echos be sent to widgets? I.e, if a widget sends an event, should this event be sent back to them?

Current behavior by Element is to echo an event after the send request to the homeserver succeeds. This is sent along with a response to the send message sent by the widget. There is no way for widgets to ignore local echos unless they maintain a list of IDs that they have sent and ignore events with those IDs when they are received.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they shouldn't receive the local echo but they should receive the server's echo (if the widget requested the capability to do so)

Copy link

@toger5 toger5 Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really like to have local echos for the events. Would it be beneficial to include local echos in this msc? Or can this be achieved easier in some (future) widget-sdk build on top of the matrix-widget-api and we can keep this msc simpler?

@turt2live
Copy link
Member Author

ftr this has absorbed #2876 given receiving is essentially the same as reading, so we make them the same action here.

Comment on lines +286 to +287
The `events` array is simply the array of events requested. When no matching events are found, this
array must be defined but can be empty.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should define the order of this array too

Copy link

@toger5 toger5 Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the addition of pagination tokes it would make the action more explicit if a dir parameter is introduced. (see: #2762 (comment))

Suggested change
The `events` array is simply the array of events requested. When no matching events are found, this
array must be defined but can be empty.
The `events` array is simply the array of events requested. When no matching events are found, this
array must be defined but can be empty. The order of this array is given by the `dir` parameter of the request.
The widget is informed if the end of the available events is reached by not providing an end token.

@turt2live turt2live force-pushed the travis/msc/widgets-send-receive-events branch from 12e6603 to aeadae8 Compare August 30, 2021 22:35
single canonical room, however.

To complement the send/receive event capabilities, a single capability is introduced to access the timelines
of other rooms: `m.timeline:<Room ID>`. The `<Room ID>` can either be an actual room ID, or a `*` to denote
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am understanding correctly, m.timeline:<Room ID> not only gives the widget access to the rooms timeline events but also to the state events. Because of the name timeline I am still not certain if this actually allows a widget to read the other rooms state events. Although the trick at the end of this paragraph and the calendar example in the introduction imply that this should be possible.
Something like m.roomAccess:<roomId> would be less confusing. Or adding:

... introduced to access the timeline and room state of other rooms: ...

to the spec text.

Copy link

@toger5 toger5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion contains three different proposed changes:

  • Adding the functionality that a widget can request an event with a specific id.
  • Adding pagination to the read_events action.
  • Propose a new format for the m.timeline capability to allow different send/receive permissions in different rooms.

`state_key` for `m.room.message` requests.

The `type` is simply the event type to go searching for.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `id` specifies an event id that the widget wants to load. If an `id` is present, all the other fields in `data` are not required anymore, but can still be provided.
The client will only send an event if an event with the requested id exists, the event conforms to the approved capabilities and to the other optionally provided fields in `data`.

Comment on lines +251 to +256
The `limit` is the number of events the widget is looking for. The client can arbitrarily decide to
return less than this limit, though should never return more than the limit. For example, a client
may decide that for privacy reasons a widget can only ever see the last 5 room messages - even though
the widget requested 25, it will only ever get 5 maximum back. When `limit` is not present it is
assumed that the widget wants as many events as the client will give it. When negative, the client
can reject the request with an error.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `limit` is the number of events the widget is looking for. The client can arbitrarily decide to
return less than this limit, though should never return more than the limit. For example, a client
may decide that for privacy reasons a widget can only ever see the last 5 room messages - even though
the widget requested 25, it will only ever get 5 maximum back. When `limit` is not present it is
assumed that the widget wants as many events as the client will give it. When negative, the client
can reject the request with an error.
The `limit` is the number of events the widget is looking for. The client can arbitrarily decide to
return less than this limit, though should never return more than the limit. For example, a client
may decide that for privacy reasons a widget can only ever see the last 5 room messages - even though
the widget requested 25, it will only ever get 5 maximum back. When `limit` is not present it is
assumed that the widget wants as many events as the client will give it. When negative, the client
can reject the request with an error.
This action can be paginated. The `from` parameter is a token generated by the client,
from which the widget wants to read events. The direction can be set by the `dir` property.
`f` for chronological order and `b` for reversed order. The token can either be a token from the
client sever api [`/messages`](https://spec.matrix.org/v1.1/client-server-api/#get_matrixclientv3roomsroomidmessages) endpoint or it can be generated by
the client in case the client already has some of the history.
It cannot be assumed, that the pagination token is in any way compatible with the client server api `/messages` endpoint.

API action:

```json
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
// Read state event
{

"type": "m.room.topic",
"limit": 25
}
}
Copy link

@toger5 toger5 Dec 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}
// Read room events
{
"api": "fromWidget",
"widgetId": "20200827_WidgetExample",
"requestid": "generated-id-1234",
"action": "read_events",
"data": {
"msgtype": "m.text",
"type": "m.room.message",
"limit": 50,
"id":"$EdUYYTBGSaLUNHGw9OlsVgtCO8fqfql519qJh0gfs4",
"from": "somePaginationTokenStringA",
"dir": "f" | "b",
}

the widget requested 25, it will only ever get 5 maximum back. When `limit` is not present it is
assumed that the widget wants as many events as the client will give it. When negative, the client
can reject the request with an error.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it could be discussed, if a new capability should be introduced. Something like: m.completeTimeline.

Suggested change
If the client approves the `m.completeTimeline` capability, it guarantees to give access to all events until
the time where the capability was approved. It is still possible to return less events in the response to a `read_events` action then the limit but it has to
return at least one event, if there is one available after the `from` pagination token and before the capability
approval. If this capability got approved the client is also responsible to query more events (this is NOT the case if if the widget does not have this capability) from the server
if the widget requests them and it is expected that non-gappy event lists are sent.

}
]
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}
// Response read_events (room events)
{
"api": "fromWidget",
"widgetId": "20200827_WidgetExample",
"requestid": "generated-id-1234",
"action": "read_events",
"data": {
"msgtype": "m.text",
"type": "m.room.message",
"limit": 50,
"id":"$EdUYYTBGSaLUNHGw9OlsVgtCO8fqfql519qJh0gfs4",
"from": "somePaginationTokenStringA",
"dir": "f" | "b"
}
"response": {
"start": "somePaginationTokenStringA",
"end": "somePaginationTokenStringC",
"events": [
{
"type": "m.room.message",
"sender": "@alice:example.org",
"event_id": "$example",
"room_id": "!room:example.org",
"origin_server_ts": 1574383781154,
"content": {
"msgtype": "m.text",
"body": "Hello"
},
"unsigned": {
"age": 12345
}
}
]
}
}

Comment on lines +286 to +287
The `events` array is simply the array of events requested. When no matching events are found, this
array must be defined but can be empty.
Copy link

@toger5 toger5 Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the addition of pagination tokes it would make the action more explicit if a dir parameter is introduced. (see: #2762 (comment))

Suggested change
The `events` array is simply the array of events requested. When no matching events are found, this
array must be defined but can be empty.
The `events` array is simply the array of events requested. When no matching events are found, this
array must be defined but can be empty. The order of this array is given by the `dir` parameter of the request.
The widget is informed if the end of the available events is reached by not providing an end token.

Comment on lines +319 to +327
To complement the send/receive event capabilities, a single capability is introduced to access the timelines
of other rooms: `m.timeline:<Room ID>`. The `<Room ID>` can either be an actual room ID, or a `*` to denote
all joined or invited rooms the client is able to see, current and future. The widget can limit its exposure
by simply requesting highly scoped send/receive capabilities to accompany the timeline capability.

Do note that a widget does not need to request capabilities for all rooms if it only ever interacts with the
user's currently viewed room. Widgets such as stickerpickers will not need to request timeline capabilities
because they'll always send events to the user's currently viewed room, and the client will let them do that
without special room timeline permissions.
Copy link

@toger5 toger5 Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion is not related to the others. Its about having different permissions in different rooms. Here is a list of examples, why I think it might be good to be able to define different capabilities for each room:

  • I want to check if the user is part of a specific public room or space to provide additional functionality in the widget. In case this widget has lots of capabilities (send/receive) in the current room. The widget would also be able to receiv and send messages in the public room although i just want to check the m.room.member list if the current user is part of that room/space.
  • A calendar widget that should query calendar events (containing information about meetings/appointments) from all rooms should also be able to ping me with a message in its associated room to notify me about the events. But it should not be able to post messages in all rooms.
Suggested change
To complement the send/receive event capabilities, a single capability is introduced to access the timelines
of other rooms: `m.timeline:<Room ID>`. The `<Room ID>` can either be an actual room ID, or a `*` to denote
all joined or invited rooms the client is able to see, current and future. The widget can limit its exposure
by simply requesting highly scoped send/receive capabilities to accompany the timeline capability.
Do note that a widget does not need to request capabilities for all rooms if it only ever interacts with the
user's currently viewed room. Widgets such as stickerpickers will not need to request timeline capabilities
because they'll always send events to the user's currently viewed room, and the client will let them do that
without special room timeline permissions.
To send/receive events in different rooms, capabilities can be extended with `:<Room ID>` to access the timelines or states of other rooms. The complete format of capabilities the introduced capabilities is:
`<m.send.event | m.receive.event>:<event type>:<Room ID>`
The `<Room ID>` can either be an actual room ID, or a `*` to denote all joined or invited rooms
the client is able to see, current and future. In the case, where `*` is used,
the widget can limit its exposure by requesting highly scoped `<event type>`'s for the send/receive capabilities.
As presented in the previous sections, omitting the `:<Room ID>` part of the capability implies that the widget can ONLY send/receive events in the room the widget is attached to.
Widgets such as sticker-pickers therefore will not need to provide the `:<Room ID>` part in its capabilities,
because they'll always send events to the user's currently viewed room, and the client will let them do that
without specifying a room id.
In cases where the widget needs access to multiple rooms, but not all rooms (`*`) it needs to request multiple capabilities (e.g. `m.receive.event:m.room.message:$roomA` and `m.receive.event:m.room.message:$roomB`). It is NOT possible to provide a list of room id's in a single capability.

object):

```json
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
// Response read_events (state events)
{

the [`/redact` endpoint](https://matrix.org/docs/spec/client_server/r0.6.1#put-matrix-client-r0-rooms-roomid-redact-eventid-txnid)
on behalf of the widget.

## Proposal (receiving events in a widget)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section needs to specify behaviour for encrypted events.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be covered somewhere in this proposal: the widget always receives decrypted events, and always sends unencrypted (to be encrypted by the client).

If you mean toDevice messages, those are #3819

Comment on lines +132 to +133
encryption. The widget is responsible for producing valid events - the client MUST pass through any
errors, such as permission errors, to the widget using the standard error response in the Widget API.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leaves ambiguous how errors sent in Widget API responses ought to be formatted.

The Widget API v2 spec says that error responses have a format of:

{
    error: {
        message: string;
        <Original Error Object>
    }
}

That leaves the (unspecified) as the place where clients would pass Matrix API errors, but there is more to send than just the standard error response -- some Matrix API errors use the HTTP status code and headers to carry some information that isn't present in the error body (such as the Retry-After header for responses with a 429 status code).

The error format used by matrix-org/matrix-widget-api#100 to include both the standard error response & HTTP response data is as follows:

{
    error: {
        message: string;
        matrix_api_error?: {
            http_status: number;
            http_headers: {[name: string]: string};
            url: string;
            response: {
                errcode: string;
                error: string;
                [key: string]: unknown;
            };
        };
    };
};

Copy link

@toger5 toger5 Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also added in: matrix-org/matrix-rust-sdk#4241 for the rust sdk widget driver.
We also had the discussion how much information is actually requried and if:

{
    error: {
        message: string;
        matrix_api_error?: {
          errcode: string;
          error: string;
          [key: string]: unknown;
    };
    };
};

might be enough?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the headers mentioned by the spec are really needed. i.e. If the spec defines an error response as including a certain set of response headers (as it does for 429 responses), clients should pass only those headers in the widget response & may silently drop any others.


Note that the client should also be sending the widget any events in rooms where the widget is permitted
to receive events from. The exact details of these permissions are covered later in this document.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For state events, it is expected, that the client sends an event, when the current state in the room changes and does not forward incoming state events form the timeline sync section.
The widget should be able to trust that a new state event represents the current room state.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a bit of trouble understanding this, I hope this phrasing captures the same meaning:

Suggested change
For state events, it is expected that the client sends an event when the current state in the room changes, rather than simply forwarding incoming state events from the timeline sync section.
Particularly, this includes "perceived" state changes e.g. due to state resolution.
The widget should be able to trust that a new state event received represents the actual current room state the client is aware of.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robintown Looks into another/cleaner solution to this where we send both: timeline & state section state with to the widget with labels so that the widget can decide what it wants to use.

bnjbvr added a commit to matrix-org/matrix-rust-sdk that referenced this pull request Dec 4, 2024
Currently the WidgetDriver just returns unspecified error strings to the
widget that can be used to display an issue description to the user. It
is not helpful to run code like a retry or other error mitigation logic.

Here it is proposed to add standardized errors for issues that every
widget driver implementation can run into (all matrix cs api errors):
matrix-org/matrix-spec-proposals#2762 (comment)

This PR forwards the errors that occur during the widget processing to
the widget in the correct format.

NOTE:
It does not include request Url and http Headers. See also:
matrix-org/matrix-spec-proposals#2762 (comment)

Co-authored-by: Benjamin Bouvier <[email protected]>
andybalaam pushed a commit to matrix-org/matrix-rust-sdk that referenced this pull request Dec 18, 2024
Currently the WidgetDriver just returns unspecified error strings to the
widget that can be used to display an issue description to the user. It
is not helpful to run code like a retry or other error mitigation logic.

Here it is proposed to add standardized errors for issues that every
widget driver implementation can run into (all matrix cs api errors):
matrix-org/matrix-spec-proposals#2762 (comment)

This PR forwards the errors that occur during the widget processing to
the widget in the correct format.

NOTE:
It does not include request Url and http Headers. See also:
matrix-org/matrix-spec-proposals#2762 (comment)

Co-authored-by: Benjamin Bouvier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal widgets anything to do with widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants