Call is lost when a participant is added to a one-to-one conversation during a call

[bakiburakogun]

Summary

Adding a participant to a one-to-one conversation while a call is running moves everyone to the newly created group conversation, but the call does not continue there. All participants end up out of the call and have to start a new one.

The switchto signal itself works: the clients leave the old room and join the new one within a second. What does not happen is the call in the new conversation.

Setup

• Nextcloud 34.0.3, Talk 24.0.4
• External signaling server (nextcloud-spreed-signaling v2.1.1), four servers in one cluster
• Reproduced with two participants, both on the web client, each connected to a different signaling server

What we see on the signaling servers

15:49:49  session A joined call <old-room>      (one-to-one conversation)
15:50:02  session B joined call <old-room>
          -> third participant is added to the conversation here
15:50:15  session A removed from room <old-room>
15:50:15  session B removed from room <old-room>
15:50:16  session A joined room <new-room>
15:50:17  session B joined room <new-room>
          -> no "joined call <new-room>" for either session

So both clients received and acted on the switchto event, and both are in the new conversation. Neither joins the call there.

Code path we followed

On the server side everything looks as intended. RoomController::createRoom() dispatches RoomExtendedEvent after the new room is created and the invitation list is added, and SignalingListener::notifyRoomExtended() collects the sessions that are in the call and calls BackendNotifier::switchToRoom(). The old room still has its call flag set at that point, so the early return is not taken and the sessions are passed on.

On the client side, switchto is turned into an EventBus switch-to-conversation event, and the handler awaits joinCallAutomatically(token) before navigating. That method starts with

async joinCallAutomatically(token, currentToken = this.token) {
    if (this.isInCall) {
        ...
    }
}

and sets up a watcher that dispatches joinCall for the new token.

What we could not determine

From the server side we cannot tell which of the two happened:

1. this.isInCall was already false when the handler ran, so nothing was set up at all, or
2. the watcher fired and the joinCall request failed.

The navigation to the new conversation did happen, which means the handler ran to completion without throwing.

Question

Is the call supposed to continue in the new conversation in this flow? If yes, we are happy to dig further on the client side and prepare a pull request. We can reproduce this on demand and have a four server signaling cluster to test patches on. A pointer on where you would expect the state to be lost would help.

[bakiburakogun]

Reproduced with the browser console open. There is an uncaught exception in the flow, raised from the switchto handling:

webrtc.js:528 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'destroy')
    at Function.<anonymous> (webrtc.js:528:25)
    at PN._trigger (signaling.js:115:12)
    at signaling.js:340:8
    at Base.leaveCall (signaling.js:334:9)
    at Base.leaveCurrentCall (signaling.js:213:9)
    at Base.leaveRoom (signaling.js:251:7)
    at participantsService.js:70
    at leaveConversation (participantsStore.js:1141)
    at App.vue:336
    ...
    at signaling.js:1439        <- case 'switchto'

webrtc.js:528 is in the beforeLeaveCall handler:

let localStateBroadcaster                                    // module scope, initially undefined

signaling.on('beforeJoinCall', function(token, reconnect) {
    localUserInCall = true
    if (signaling.hasFeature('mcu')) {
        localStateBroadcaster = new LocalStateBroadcasterMcu(...)
    } else {
        localStateBroadcaster = new LocalStateBroadcasterNoMcu(...)
    }
})

signaling.on('beforeLeaveCall', function(token, reconnect) {
    localUserInCall = false
    localStateBroadcaster.destroy()      // <- throws when it is null
    localStateBroadcaster = null
})

beforeLeaveCall assumes that beforeJoinCall ran before it and left an object behind. It is null whenever the call was not joined in this page session, or when the leave path runs a second time, since line 529 sets it back to null. leaveRoom() calls leaveCurrentCall() unconditionally, so leaving or switching a conversation reaches this handler either way.

A null check would make the handler safe:

signaling.on('beforeLeaveCall', function(token, reconnect) {
    localUserInCall = false

    if (localStateBroadcaster) {
        localStateBroadcaster.destroy()
        localStateBroadcaster = null
    }
})

We are not claiming this alone restores the call in the new conversation, we have not been able to test that yet. But the exception is raised inside the promise chain that the switchto handler awaits, so it does abort the sequence at that point, and guarding it is correct in any case.

For completeness: the browser used for this reproduction has no microphone or camera, and the console also shows Access to microphone & camera failed NotFoundError: Requested device not found earlier in the same session. That may be what left the broadcaster unset here. Happy to open a pull request for the null check if you want it.

[bakiburakogun]

Tested the null check from #19083 on the instance from this report, by applying the equivalent change to the shipped bundle, and it fixes the problem described here.

Timeline from the signaling servers for the same scenario, one-to-one conversation with an active call, third participant added:

16:13:04  session A joined call <one-to-one room>
          -> third participant is added here
16:13:24  session A and session B left call <one-to-one room>
16:13:25  session A and session B joined room <group room>
16:13:26  session B joined call <group room>          <- the call continues now
16:14:07  session C joined room <group room>
16:14:09  session C joined call <group room>
16:14:14  all sessions left call <group room>

Before the change the two sessions moved to the new conversation and no one joined the call there, which is what this issue describes. With the change the call is back one second after the switch, and the participant who was just added joins it as well.

The browser console no longer shows the TypeError: Cannot read properties of null (reading 'destroy'), and no new errors appeared in that flow.

One separate observation from the same test, in case it is related and useful: the moderator who added the participant reported that "End meeting for everyone" seemed to do nothing, and clicked it four times. The signaling servers show the call did end (the left call lines above, five seconds after the last participant joined), so the call state was correct and it looks like the call view did not update. We have not investigated that further and it may well be unrelated to this issue.

[nickvergessen]

• Requested colleagues reviews on fix(call): guard against a missing local state broadcaster when leaving a call #19083
• Let's see if that satisfies