Automattic / chatrix

Matrix client for WordPress
https://wordpress.org/plugins/chatrix
GNU General Public License v2.0
41 stars 0 forks source link

Handle failure in guest registration #211

Closed ashfame closed 1 year ago

ashfame commented 1 year ago

When a server doesn't have guest account registration enabled, it fails with this error.

Screenshot 2023-03-24 at 6 32 33 PM

It should show unknownRoomView with a login button

psrpinto commented 1 year ago

There's another error message than can be shown that is apparently also related to this:

null is not an object (evaluating 'this._session.isGuest')

Screenshot 2023-06-01 at 3 34 37 PM

psrpinto commented 1 year ago

I found another error message than seems to be related to this:

Something went wrong
Cannot read properties of null (reading 'isGuest')

Steps to reproduce:

  1. Logout of all chatrix sessions
  2. Go to the block editor and add a chatrix block
  3. Change room in block inspector to #meta:community.wordpress.org
  4. Error shows
psrpinto commented 1 year ago

I think this issue manifests itself differently according to whether the user has a session open or not, and also depends on if they have "previously-opened" sessions. That explains why there are different error messages.

I think we might need to rework how single-room mode is handled, specifically in what concerns the case where a previous session exists. In this code chosenSession is not defined, so that's surely part of the issue.

Here's all the use cases I could think of (can you think of any others?), and what I think the behaviour should be for each:

I think we should rework the code to implement this ^, I would be happy to do so.

akirk commented 1 year ago

Great. Please proceed on working on this, it's a blocker as we experienced in today's meeting.

ashfame commented 1 year ago

@psrpinto Can you elaborate on what you mean with "session open" here, as I don't understand this branching:

- User does not have a session open
    - User has exactly one previously-opened session

I would add another condition while doing guest login. When its a 403 (meaning guest registrations on server is disabled), we behave as if the room is not world-readable and navigate to login.

psrpinto commented 1 year ago

Good catch, I edited my comment to add the condition for when homeserver has guest registration disabled.

Can you elaborate on what you mean with "session open"

By "session open" I mean that the user went past the session picker and is "inside" a session, prior to loading chatrix in single-user mode. (The case where they have a single session is a special case of this, where the session picker isn't displayed, but from a code perspective it's the same, they are "inside" the session even if they don't see the session picker).

psrpinto commented 1 year ago

In addition, when I say "previously-opened", I mean they see the session picker with previously used sessions, but they aren't "inside" any session.

ashfame commented 1 year ago

Gotcha, thanks for explaining! The complete branching makes sense now & I think that covers everything 👍

ashfame commented 1 year ago

Related: https://github.com/Automattic/chatrix/issues/209