fix: Delete empty dm room on createRoom error #1912
Conversation
mErrenst
force-pushed
the
malin/delete-empty-dm-room-on-create-room-error
branch
from
632a54b to
c32ab79
Compare
mErrenst
force-pushed
the
malin/delete-empty-dm-room-on-create-room-error
branch
from
c32ab79 to
252d7cb
Compare
|
|
||
| return roomId; | ||
| return roomId; | ||
| } catch (e) { |
There was a problem hiding this comment.
This is catching any error, not just the room creation error. You should narrowly scope the try catch and handle specific errors, not all of them and assume it is the one error you intended to handle.
| /// The room is empty since createRoom threw an error. To make sure not to | ||
| /// delete other empty rooms by accident, the creation time is checked. | ||
| /// Should be fixed in Synapse and then be removed |
There was a problem hiding this comment.
You should really document, why this throws an error, because the synapse code tries to handle all errors ahead of time, so it is not clear, why you are getting an error on room creation. If the problem is invites, we can for example move those out of the room creation.
| final sortedRooms = rooms.sortedBy((room) => room.timeCreated).reversed; | ||
| final emptyRoom = sortedRooms.firstWhereOrNull((room) => | ||
| room.name.isEmpty && | ||
| !room.isDirectChat && | ||
| room.summary.mJoinedMemberCount == 1 && | ||
| room.summary.mInvitedMemberCount == 0 && | ||
| room.timeCreated | ||
| .isAfter(DateTime.now().subtract(Duration(seconds: 20)))); | ||
| final roomId = emptyRoom?.id; |
There was a problem hiding this comment.
it's probably more robust to handle the room id list differences from before and after the room creation instead of going purely by timestamp?
| await leaveRoom(roomId); | ||
| await forgetRoom(roomId); |
There was a problem hiding this comment.
This can throw and needs error handling to not cause weird error messages.
| /// The room is empty since createRoom threw an error. To make sure not to | ||
| /// delete other empty rooms by accident, the creation time is checked. | ||
| /// Should be fixed in Synapse and then be removed | ||
| final sortedRooms = rooms.sortedBy((room) => room.timeCreated).reversed; |
There was a problem hiding this comment.
How do you know, that the room is already synced back to the client?
| await forgetRoom(roomId); | ||
| } | ||
| rethrow; | ||
| } | ||
| } |
|
I think I agree with td now, that if this only happens for invites rejected by the invite checker, that we should fix this in Synapse. |
Closes https://github.com/famedly/product-management/issues/2385