-
Notifications
You must be signed in to change notification settings - Fork 401
MSC4292: Handling incompatible room versions in clients #4292
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
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.
Implementation requirements:
- Client
- Server
the effect similar to v6. | ||
* [Room Version 10](https://spec.matrix.org/v1.14/rooms/v10/) - Adds another join rule, similar to | ||
v7 and v8. | ||
* [Room Version 11](https://spec.matrix.org/v1.14/rooms/v11/) - Reduces the set of redacted fields on |
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.
V11 as far as I can recall removes the creator field from m.room.create and broke element iOS pre patch.
* [Room Version 6](https://spec.matrix.org/v1.14/rooms/v6/) - While the redaction algorithm did change, | ||
it is not backwards incompatible for clients. The effects of redactions may be greater than intended, | ||
however can be relatively easily fixed after a client bugfix. |
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 double negative here is confusing?
* [Room Version 6](https://spec.matrix.org/v1.14/rooms/v6/) - While the redaction algorithm did change, | |
it is not backwards incompatible for clients. The effects of redactions may be greater than intended, | |
however can be relatively easily fixed after a client bugfix. | |
* [Room Version 6](https://spec.matrix.org/v1.14/rooms/v6/) - While the redaction algorithm did change, | |
it is backwards compatible for clients. The effects of redactions may be greater than intended, | |
however can be relatively easily fixed after a client bugfix. |
Or maybe "mostly backwards compatible"?
versions are compatible with that set. The compatible set may be larger than the supported set if | ||
there are room versions which don't contain breaking changes for clients. These room versions will | ||
be identified in the spec, and currently include the following: | ||
|
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 think it might be valuable to document if we have any examples of cases where we know a client experienced problems due to a room version. This would serve to make it more concrete when we know for a fact this isnt just a theoretical breakage but a verified clients in the real world did indeed break.
Examples being that we know that Element iOS specifically broke on room version 11 being introduced when you attempted to interact with a room version 11 room due to that it validated the create event in a way that lead to a crash from the removal of the creator field.
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 hope is that the system will be pre-break identification rather than post-break. We're in an awkward spot though because we've already got 11 room versions, some of which have (and do) cause problems for clients.
Ideally, we refine the definition of "breaking" within the spec and hold implementations up to that standard, I think. This would allow us to more easily identify when a change is breaking, and have a sign to point to when clients break in ways that we don't consider to be a problem (ie: "you parsed a room ID when you're not supposed to - not a breaking change on our side"). This MSC might not be the place to refine that definition, but could potentially be considered blocked by it, assuming it's not as much of a nightmare as I think it would 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.
I was thinking about this in the sense of that we should definetively document problems clients have run in to in the past when we define the known ways a client might have to adapt to the existing 11 room versions.
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.
ah, that might not be something for the spec itself but rather a process adjacent to the spec. Possibly commenting on the original MSC or amending that original MSC to include a "hey, this happened" where it feels relevant.
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 respond to the initial comment here: LibMatrix breaks on any schema change, most room versions are currently compatible if no funky stuff has happened (ie string/bool PLs).
Generally, I tend to implement support for something only when either I need something or something breaks. I would be okay opting out of everything except the latest room version, though on the other hand having to update the SDK to add explicit support for a new room version can be quite annoying.
Rendered
Disclosure: I am Director of Standards Development at The Matrix.org Foundation C.I.C., Matrix Spec Core Team (SCT) member, employed by Element, and operate the t2bot.io service. This proposal is written and published as a Trust & Safety team member allocated in full to the Foundation.