fix: end call when the last connected peer hangs up
Scenario: Alice calls {bob, carol} as a group. Bob answers, Bob and
Alice are talking, Carol never joins. Before Carol's 30 s watchdog
fires, Alice hangs up. Bob's state had peerPubKeys={alice} and
pendingPeerPubKeys={carol}. The old onPeerHangup handler only ended
the call when BOTH sets became empty, so Bob stayed in Connected
alone staring at a black screen until Carol's watchdog finally
dropped her — and even then the state machine didn't terminate
because "no peers, no pending" wasn't true at the exact same step.
Change the termination rule in onPeerHangup (both Connected and
Connecting states) to end the call as soon as connectedRemaining
becomes empty, regardless of how many pending peers are still
tracked. Without a single connected peer the call has no one to
talk to and can't meaningfully continue.
Also publish a CallHangup to any peers in the pending set that WE
personally invited (peersInvitedByUs ∩ pendingPeerPubKeys) so their
devices stop ringing. Peers we did not invite (e.g. group members
the caller originally included) are the caller's responsibility —
the caller's own group hangup already reaches them — so we leave
them alone.
Adds three regression tests covering:
- Connected state: caller hangs up while one pending member remains
- Connecting state: same, before onPeerConnected fires
- Callee had invited someone mid-call: call ends AND Bob notifies
his invitee to stop ringing
This commit is contained in:
Claude
+44
-5
@@ -719,7 +719,7 @@ class CallManager(
|
||||
result.wraps.forEach { publishEvent(it) }
|
||||
}
|
||||
|
||||
private fun onPeerHangup(event: CallHangupEvent) {
|
||||
private suspend fun onPeerHangup(event: CallHangupEvent) {
|
||||
val current = _state.value
|
||||
val callId = event.callId() ?: return
|
||||
val leavingPeer = event.pubKey
|
||||
@@ -732,8 +732,17 @@ class CallManager(
|
||||
cancelPeerTimeout(leavingPeer)
|
||||
val connectedRemaining = current.peerPubKeys - leavingPeer
|
||||
val pendingRemaining = current.pendingPeerPubKeys - leavingPeer
|
||||
if (connectedRemaining.isEmpty() && pendingRemaining.isEmpty()) {
|
||||
Log.d("CallManager") { "onPeerHangup: last peer left, ending call" }
|
||||
if (connectedRemaining.isEmpty()) {
|
||||
// No connected peers left. Pending members we never
|
||||
// heard from can't sustain the call on their own, so
|
||||
// terminate — otherwise the caller (the only other
|
||||
// participant we were actually talking to) has hung up
|
||||
// and we'd be left staring at a black screen waiting
|
||||
// for someone who may never join.
|
||||
Log.d("CallManager") {
|
||||
"onPeerHangup: last connected peer left, ending call (pendingRemaining=${pendingRemaining.size})"
|
||||
}
|
||||
publishHangupToInvitedPendingPeers(callId, pendingRemaining)
|
||||
transitionToEnded(callId, current.allPeerPubKeys, EndReason.PEER_HANGUP)
|
||||
} else {
|
||||
Log.d("CallManager") { "onPeerHangup: ${leavingPeer.take(8)} left, remaining=${connectedRemaining.map { it.take(8) }}, pending=${pendingRemaining.map { it.take(8) }}" }
|
||||
@@ -751,8 +760,14 @@ class CallManager(
|
||||
cancelPeerTimeout(leavingPeer)
|
||||
val connectedRemaining = current.peerPubKeys - leavingPeer
|
||||
val pendingRemaining = current.pendingPeerPubKeys - leavingPeer
|
||||
if (connectedRemaining.isEmpty() && pendingRemaining.isEmpty()) {
|
||||
Log.d("CallManager") { "onPeerHangup: last peer left during connecting, ending call" }
|
||||
if (connectedRemaining.isEmpty()) {
|
||||
// Same rule as Connected: without a single connected
|
||||
// peer the call can't keep running. Pending members
|
||||
// are dropped regardless of how many are left.
|
||||
Log.d("CallManager") {
|
||||
"onPeerHangup: last connected peer left during connecting, ending call (pendingRemaining=${pendingRemaining.size})"
|
||||
}
|
||||
publishHangupToInvitedPendingPeers(callId, pendingRemaining)
|
||||
transitionToEnded(callId, current.peerPubKeys + current.pendingPeerPubKeys, EndReason.PEER_HANGUP)
|
||||
} else {
|
||||
Log.d("CallManager") { "onPeerHangup: ${leavingPeer.take(8)} left during connecting, remaining=${connectedRemaining.map { it.take(8) }}" }
|
||||
@@ -797,6 +812,30 @@ class CallManager(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* When a call is ending because its last connected peer left, publish
|
||||
* a CallHangup to every *pending* peer we had personally invited so
|
||||
* their devices stop ringing. Peers we did NOT invite (e.g. callee-side
|
||||
* group members) are the caller's responsibility and are left alone.
|
||||
*
|
||||
* Must be called while still inside the CallManager state mutex and
|
||||
* BEFORE [transitionToEnded], which clears [peersInvitedByUs].
|
||||
*/
|
||||
private suspend fun publishHangupToInvitedPendingPeers(
|
||||
callId: String,
|
||||
pendingPeers: Set<HexKey>,
|
||||
) {
|
||||
val invitedPending = peersInvitedByUs.intersect(pendingPeers)
|
||||
if (invitedPending.isEmpty()) return
|
||||
Log.d("CallManager") {
|
||||
"publishHangupToInvitedPendingPeers: notifying ${invitedPending.size} invited pending peers to stop ringing"
|
||||
}
|
||||
for (peer in invitedPending) {
|
||||
val result = factory.createHangup(peer, callId, signer = signer)
|
||||
publishEvent(result.wrap)
|
||||
}
|
||||
}
|
||||
|
||||
suspend fun onSignalingEvent(event: Event) {
|
||||
if (isEventTooOld(event)) {
|
||||
Log.d("CallManager") { "Discarding old event kind=${event.kind} age=${TimeUtils.now() - event.createdAt}s" }
|
||||
|
||||
+102
@@ -1577,6 +1577,108 @@ class CallManagerTest {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression: Alice calls {bob, carol} as a group, Bob accepts and
|
||||
* talks to Alice, Carol never joins. Alice hangs up before Carol's
|
||||
* watchdog timer fires. Bob must end the call immediately — NOT stay
|
||||
* in a Connected state alone waiting for Carol.
|
||||
*/
|
||||
@Test
|
||||
fun callerHangupEndsCalleeCallEvenWhenPendingPeersRemain() =
|
||||
runTest {
|
||||
val (manager, published) = createManager(localPubKey = bob, followedKeys = setOf(alice, carol))
|
||||
|
||||
// Alice calls {bob, carol}. Bob accepts and reaches Connected.
|
||||
manager.onSignalingEvent(makeGroupOffer(from = alice, members = setOf(bob, carol)))
|
||||
manager.acceptCall(sdpAnswer)
|
||||
manager.onPeerConnected()
|
||||
val connected = manager.state.value
|
||||
assertIs<CallState.Connected>(connected)
|
||||
assertTrue(alice in connected.peerPubKeys, "Alice must be the sole connected peer")
|
||||
assertTrue(carol in connected.pendingPeerPubKeys, "Carol must still be pending")
|
||||
published.clear()
|
||||
|
||||
// Alice hangs up well before Carol's 30 s watchdog fires.
|
||||
manager.onSignalingEvent(makeHangup(from = alice, to = bob))
|
||||
|
||||
// The call must terminate immediately — Bob has nobody left
|
||||
// to talk to, so staying in Connected alone is wrong.
|
||||
val after = manager.state.value
|
||||
assertIs<CallState.Ended>(after)
|
||||
assertEquals(EndReason.PEER_HANGUP, after.reason)
|
||||
|
||||
// Bob must not publish any hangup: Carol's ringing is Alice's
|
||||
// responsibility (Alice already broadcast her hangup to Carol
|
||||
// as part of her own group hangup).
|
||||
assertTrue(
|
||||
published.isEmpty(),
|
||||
"Callee must not publish anything when ending due to caller hangup",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression: when Bob is alone in a Connecting state (haven't reached
|
||||
* Connected yet) and the caller hangs up, the call must end rather
|
||||
* than lingering in Connecting with an empty peerPubKeys until the
|
||||
* watchdog fires.
|
||||
*/
|
||||
@Test
|
||||
fun callerHangupEndsCalleeCallInConnectingEvenWhenPendingPeersRemain() =
|
||||
runTest {
|
||||
val (manager, _) = createManager(localPubKey = bob, followedKeys = setOf(alice, carol))
|
||||
|
||||
manager.onSignalingEvent(makeGroupOffer(from = alice, members = setOf(bob, carol)))
|
||||
manager.acceptCall(sdpAnswer)
|
||||
// Intentionally NOT calling onPeerConnected — we want to stay in Connecting.
|
||||
assertIs<CallState.Connecting>(manager.state.value)
|
||||
|
||||
manager.onSignalingEvent(makeHangup(from = alice, to = bob))
|
||||
|
||||
val after = manager.state.value
|
||||
assertIs<CallState.Ended>(after)
|
||||
assertEquals(EndReason.PEER_HANGUP, after.reason)
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression: if Bob is in a Connected group call with Alice and has
|
||||
* personally invited Dave mid-call, and then Alice hangs up, the call
|
||||
* ends AND Bob publishes a hangup to Dave (who is still ringing on
|
||||
* Bob's invitation) so Dave's device stops ringing.
|
||||
*/
|
||||
@Test
|
||||
fun callerHangupEndsCallAndNotifiesInvitedPendingPeers() =
|
||||
runTest {
|
||||
val (manager, published) = createManager(localPubKey = bob, followedKeys = setOf(alice, carol))
|
||||
|
||||
// Alice ↔ Bob Connected (1-1).
|
||||
manager.onSignalingEvent(makeOffer(from = alice, to = bob))
|
||||
manager.acceptCall(sdpAnswer)
|
||||
manager.onPeerConnected()
|
||||
assertIs<CallState.Connected>(manager.state.value)
|
||||
|
||||
// Bob invites Carol mid-call — Bob becomes Carol's inviter.
|
||||
manager.invitePeer(carol, "bob-to-carol-sdp")
|
||||
val afterInvite = manager.state.value
|
||||
assertIs<CallState.Connected>(afterInvite)
|
||||
assertTrue(carol in afterInvite.pendingPeerPubKeys)
|
||||
published.clear()
|
||||
|
||||
// Alice hangs up before Carol accepts Bob's invite.
|
||||
manager.onSignalingEvent(makeHangup(from = alice, to = bob))
|
||||
|
||||
val after = manager.state.value
|
||||
assertIs<CallState.Ended>(after)
|
||||
assertEquals(EndReason.PEER_HANGUP, after.reason)
|
||||
|
||||
// Bob must publish a hangup to Carol (his invitee) so her
|
||||
// device stops ringing.
|
||||
assertEquals(
|
||||
1,
|
||||
published.size,
|
||||
"Must publish exactly one hangup to the invited-but-pending peer",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* When a group offer is made, each callee gets its own timer. If NEITHER
|
||||
* answers, both timers fire in turn and the whole call ends with
|
||||
|
||||
Reference in New Issue
Block a user