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

Trigger call rejoin attempt only if you were in it #4524

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

AndrewFerr
Copy link
Member

Many membership events may get synced while your own join attempt is being processed, during which you will not be in the call membership list. So, only try to rejoin if it seems that you have left the call, not when you are not (yet) in the call.

Followup of #4342

Signed-off-by: Andrew Ferrazzutti [email protected]

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Many membership events may get synced while your own join attempt is
being processed, during which you will not be in the call membership
list.  So, only try to rejoin if it seems that you have _left_ the call,
not when you are not (yet) in the call.
@AndrewFerr
Copy link
Member Author

The consequence of not having this in is calling triggerCallMembershipEventUpdate more often than needed -- but that's not so bad, because that function deduplicates redundant membership update attempts (as only one update can be queued at a time). So this PR is more for efficiency than correctness.

src/matrixrtc/MatrixRTCSession.ts Outdated Show resolved Hide resolved
@AndrewFerr
Copy link
Member Author

Actually, this has a gap: if the delayed leave were to fire before the join gets sent, then there will be no /sync response for the delayed leave (as there is no state transition for leave -> leave) and clients won't realize that their join isn't backed by a delayed leave, meaning their join can become stuck.

What would help somewhat is for /sync to notify clients of finalized (i.e. sent or cancelled) delayed events (as propsed by MSC4140), but even then it's possible for clients to race between getting a finalized delayed leave & sending a join.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants