Ensure that call events don't override existing activeCallState

This commit is contained in:
Scott Nonnenberg
2025-02-28 10:25:45 +10:00
committed by GitHub
parent 3f0f307c22
commit a00086f416
2 changed files with 242 additions and 38 deletions

View File

@@ -2274,12 +2274,18 @@ const _startCallLinkLobby = async ({
dispatch(togglePip()); dispatch(togglePip());
} else { } else {
log.warn( log.warn(
`${logId}: Attempted to start lobby while already waiting for it!` `${logId}: Attempted to start lobby while already waiting for this call!`
); );
} }
return; return;
} }
if (activeCallState) { if (activeCallState) {
if (activeCallState.state !== 'Active') {
log.warn(
`${logId}: Call wasn't active; still showing leave call modal`,
activeCallState
);
}
dispatch( dispatch(
toggleConfirmLeaveCallModal({ toggleConfirmLeaveCallModal({
type: 'adhoc-rootKey', type: 'adhoc-rootKey',
@@ -2462,12 +2468,18 @@ function startCallingLobby({
dispatch(togglePip()); dispatch(togglePip());
} else { } else {
log.warn( log.warn(
`${logId}: Attempted to start lobby while already waiting for it!` `${logId}: Attempted to start lobby while already waiting for this call!`
); );
} }
return; return;
} }
if (activeCallState) { if (activeCallState) {
if (activeCallState.state !== 'Active') {
log.warn(
`${logId}: Call wasn't active; still showing leave call modal`,
activeCallState
);
}
dispatch( dispatch(
toggleConfirmLeaveCallModal({ toggleConfirmLeaveCallModal({
type: 'conversation', type: 'conversation',
@@ -2550,8 +2562,12 @@ function startCall(
log.info(`${logId}: starting, mode ${callMode}`); log.info(`${logId}: starting, mode ${callMode}`);
if (activeCallState?.state === 'Waiting') { if (
log.error(`${logId}: Call is not ready; `); !activeCallState ||
activeCallState?.state === 'Waiting' ||
activeCallState?.conversationId !== conversationId
) {
log.error(`${logId}: Call is not ready`, activeCallState);
return; return;
} }
@@ -2840,6 +2856,14 @@ export function reducer(
if (action.type === WAITING_FOR_CALLING_LOBBY) { if (action.type === WAITING_FOR_CALLING_LOBBY) {
const { conversationId } = action.payload; const { conversationId } = action.payload;
if (state.activeCallState) {
log.warn(
`${action.type}: Already have an active call!`,
state.activeCallState
);
return state;
}
return { return {
...state, ...state,
activeCallState: { activeCallState: {
@@ -2848,9 +2872,18 @@ export function reducer(
}, },
}; };
} }
if (action.type === WAITING_FOR_CALL_LINK_LOBBY) { if (action.type === WAITING_FOR_CALL_LINK_LOBBY) {
const { roomId } = action.payload; const { roomId } = action.payload;
if (state.activeCallState) {
log.warn(
`${action.type}: Already have an active call!`,
state.activeCallState
);
return state;
}
return { return {
...state, ...state,
activeCallState: { activeCallState: {
@@ -2859,24 +2892,45 @@ export function reducer(
}, },
}; };
} }
if (action.type === CALL_LOBBY_FAILED) { if (action.type === CALL_LOBBY_FAILED) {
const { conversationId } = action.payload; const { conversationId } = action.payload;
const { activeCallState } = state; const { activeCallState } = state;
if (!activeCallState || activeCallState.conversationId !== conversationId) { if (
!activeCallState ||
activeCallState.conversationId !== conversationId ||
activeCallState.state !== 'Waiting'
) {
log.warn( log.warn(
`${action.type}: Active call does not match target conversation` `${action.type}: Active call does not match target conversation`,
activeCallState
); );
return state;
} }
return removeConversationFromState(state, conversationId); return removeConversationFromState(state, conversationId);
} }
if ( if (
action.type === START_CALLING_LOBBY || action.type === START_CALLING_LOBBY ||
action.type === START_CALL_LINK_LOBBY action.type === START_CALL_LINK_LOBBY
) { ) {
const { callMode, conversationId } = action.payload; const { callMode, conversationId } = action.payload;
const { activeCallState } = state;
if (
!activeCallState ||
activeCallState.conversationId !== conversationId ||
activeCallState.state !== 'Waiting'
) {
log.warn(
`${action.type}: Active call does not match target conversation`,
activeCallState
);
return state;
}
let call: DirectCallStateType | GroupCallStateType; let call: DirectCallStateType | GroupCallStateType;
let newAdhocCalls: AdhocCallsType; let newAdhocCalls: AdhocCallsType;
let outgoingRing: boolean; let outgoingRing: boolean;
@@ -2987,13 +3041,28 @@ export function reducer(
} }
if (action.type === START_DIRECT_CALL) { if (action.type === START_DIRECT_CALL) {
const { conversationId } = action.payload;
const { activeCallState } = state;
if (
activeCallState &&
(activeCallState.state === 'Waiting' ||
activeCallState.conversationId !== conversationId)
) {
log.warn(
`${action.type}: Cannot start call; activeCall doesn't match conversation`,
activeCallState
);
return state;
}
return { return {
...state, ...state,
callsByConversation: { callsByConversation: {
...callsByConversation, ...callsByConversation,
[action.payload.conversationId]: { [conversationId]: {
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: action.payload.conversationId, conversationId,
callState: CallState.Prering, callState: CallState.Prering,
isIncoming: false, isIncoming: false,
isVideoCall: action.payload.hasLocalVideo, isVideoCall: action.payload.hasLocalVideo,
@@ -3002,7 +3071,7 @@ export function reducer(
activeCallState: { activeCallState: {
state: 'Active', state: 'Active',
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: action.payload.conversationId, conversationId,
hasLocalAudio: action.payload.hasLocalAudio, hasLocalAudio: action.payload.hasLocalAudio,
hasLocalVideo: action.payload.hasLocalVideo, hasLocalVideo: action.payload.hasLocalVideo,
localAudioLevel: 0, localAudioLevel: 0,
@@ -3017,21 +3086,24 @@ export function reducer(
} }
if (action.type === ACCEPT_CALL_PENDING) { if (action.type === ACCEPT_CALL_PENDING) {
const call = getOwn( const { conversationId } = action.payload;
state.callsByConversation, const call = getOwn(state.callsByConversation, conversationId);
action.payload.conversationId
);
if (!call) { if (!call) {
log.warn('Unable to accept a non-existent call'); log.warn('Unable to accept a non-existent call');
return state; return state;
} }
const { activeCallState } = state;
if (!activeCallState || activeCallState.conversationId !== conversationId) {
log.warn(`${action.type}: Active call didn't match:`, activeCallState);
}
return { return {
...state, ...state,
activeCallState: { activeCallState: {
state: 'Active', state: 'Active',
callMode: call.callMode, callMode: call.callMode,
conversationId: action.payload.conversationId, conversationId,
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: action.payload.asVideoCall, hasLocalVideo: action.payload.asVideoCall,
localAudioLevel: 0, localAudioLevel: 0,
@@ -3129,13 +3201,27 @@ export function reducer(
} }
if (action.type === INCOMING_DIRECT_CALL) { if (action.type === INCOMING_DIRECT_CALL) {
const { conversationId } = action.payload;
const { activeCallState } = state;
if (activeCallState && activeCallState.conversationId !== conversationId) {
log.warn(
`${action.type}: activeCallState didn't match conversation; overriding.`,
activeCallState
);
}
return { return {
...state, ...state,
activeCallState: {
state: 'Waiting',
conversationId,
},
callsByConversation: { callsByConversation: {
...callsByConversation, ...callsByConversation,
[action.payload.conversationId]: { [conversationId]: {
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: action.payload.conversationId, conversationId,
callState: CallState.Prering, callState: CallState.Prering,
isIncoming: true, isIncoming: true,
isVideoCall: action.payload.isVideoCall, isVideoCall: action.payload.isVideoCall,
@@ -3197,13 +3283,27 @@ export function reducer(
} }
if (action.type === OUTGOING_CALL) { if (action.type === OUTGOING_CALL) {
const { conversationId } = action.payload;
const { activeCallState } = state;
if (
activeCallState &&
(activeCallState.state === 'Waiting' ||
activeCallState.conversationId !== conversationId)
) {
log.warn(
`${action.type}: Cannot start call; activeCall doesn't match conversation`
);
return state;
}
return { return {
...state, ...state,
callsByConversation: { callsByConversation: {
...callsByConversation, ...callsByConversation,
[action.payload.conversationId]: { [conversationId]: {
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: action.payload.conversationId, conversationId,
callState: CallState.Prering, callState: CallState.Prering,
isIncoming: false, isIncoming: false,
isVideoCall: action.payload.hasLocalVideo, isVideoCall: action.payload.hasLocalVideo,
@@ -3212,7 +3312,7 @@ export function reducer(
activeCallState: { activeCallState: {
state: 'Active', state: 'Active',
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: action.payload.conversationId, conversationId,
hasLocalAudio: action.payload.hasLocalAudio, hasLocalAudio: action.payload.hasLocalAudio,
hasLocalVideo: action.payload.hasLocalVideo, hasLocalVideo: action.payload.hasLocalVideo,
localAudioLevel: 0, localAudioLevel: 0,

View File

@@ -5,7 +5,10 @@ import { assert } from 'chai';
import * as sinon from 'sinon'; import * as sinon from 'sinon';
import { cloneDeep, noop } from 'lodash'; import { cloneDeep, noop } from 'lodash';
import type { PeekInfo } from '@signalapp/ringrtc'; import type { PeekInfo } from '@signalapp/ringrtc';
import type { StateType as RootStateType } from '../../../state/reducer'; import type {
StateType as RootStateType,
StateType,
} from '../../../state/reducer';
import { reducer as rootReducer } from '../../../state/reducer'; import { reducer as rootReducer } from '../../../state/reducer';
import { noopAction } from '../../../state/ducks/noop'; import { noopAction } from '../../../state/ducks/noop';
import type { import type {
@@ -199,7 +202,7 @@ describe('calling duck', () => {
const ourAci = generateAci(); const ourAci = generateAci();
const getEmptyRootState = () => { const getEmptyRootState = (): StateType => {
const rootState = rootReducer(undefined, noopAction()); const rootState = rootReducer(undefined, noopAction());
return { return {
...rootState, ...rootState,
@@ -2274,10 +2277,11 @@ describe('calling duck', () => {
const waitingAction = dispatch.getCall(0).args[0]; const waitingAction = dispatch.getCall(0).args[0];
assert.equal(waitingAction.type, 'calling/WAITING_FOR_CALLING_LOBBY'); assert.equal(waitingAction.type, 'calling/WAITING_FOR_CALLING_LOBBY');
const waitingState = reducer(callingState, waitingAction);
const action = dispatch.getCall(1).args[0]; const startLobbyAction = dispatch.getCall(1).args[0];
assert.equal(startLobbyAction.type, 'calling/START_CALLING_LOBBY');
return reducer(callingState, action); return reducer(waitingState, startLobbyAction);
}; };
it('saves a direct call and makes it active', async () => { it('saves a direct call and makes it active', async () => {
@@ -2563,17 +2567,40 @@ describe('calling duck', () => {
it('asks the calling service to start an outgoing direct call', async function (this: Mocha.Context) { it('asks the calling service to start an outgoing direct call', async function (this: Mocha.Context) {
const dispatch = sinon.spy(); const dispatch = sinon.spy();
const emptyState = getEmptyRootState();
const conversationId = '123';
const startState: StateType = {
...emptyState,
calling: {
...emptyState.calling,
activeCallState: {
state: 'Active',
callMode: CallMode.Direct,
conversationId,
hasLocalAudio: true,
hasLocalVideo: false,
localAudioLevel: 0,
viewMode: CallViewMode.Sidebar,
joinedAt: null,
outgoingRing: true,
pip: false,
settingsDialogOpen: false,
showParticipantsList: false,
},
},
};
await startCall({ await startCall({
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: '123', conversationId,
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: false, hasLocalVideo: false,
})(dispatch, getEmptyRootState, null); })(dispatch, () => startState, null);
sinon.assert.calledOnce(this.callingStartOutgoingDirectCall); sinon.assert.calledOnce(this.callingStartOutgoingDirectCall);
sinon.assert.calledWith( sinon.assert.calledWith(
this.callingStartOutgoingDirectCall, this.callingStartOutgoingDirectCall,
'123', conversationId,
true, true,
false false
); );
@@ -2583,34 +2610,87 @@ describe('calling duck', () => {
it('asks the calling service to join a group call', async function (this: Mocha.Context) { it('asks the calling service to join a group call', async function (this: Mocha.Context) {
const dispatch = sinon.spy(); const dispatch = sinon.spy();
const emptyState = getEmptyRootState();
const conversationId = '123';
const startState: StateType = {
...emptyState,
calling: {
...emptyState.calling,
activeCallState: {
state: 'Active',
callMode: CallMode.Group,
conversationId,
hasLocalAudio: true,
hasLocalVideo: false,
localAudioLevel: 0,
viewMode: CallViewMode.Sidebar,
joinedAt: null,
outgoingRing: true,
pip: false,
settingsDialogOpen: false,
showParticipantsList: false,
},
},
};
await startCall({ await startCall({
callMode: CallMode.Group, callMode: CallMode.Group,
conversationId: '123', conversationId,
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: false, hasLocalVideo: false,
})(dispatch, getEmptyRootState, null); })(dispatch, () => startState, null);
sinon.assert.calledOnce(this.callingJoinGroupCall); sinon.assert.calledOnce(this.callingJoinGroupCall);
sinon.assert.calledWith(this.callingJoinGroupCall, '123', true, false); sinon.assert.calledWith(
this.callingJoinGroupCall,
conversationId,
true,
false
);
sinon.assert.notCalled(this.callingStartOutgoingDirectCall); sinon.assert.notCalled(this.callingStartOutgoingDirectCall);
}); });
it('saves direct calls and makes them active', async () => { it('saves direct calls and makes them active', async () => {
const dispatch = sinon.spy(); const dispatch = sinon.spy();
const emptyState = getEmptyRootState();
const conversationId = '123';
const startState: StateType = {
...emptyState,
calling: {
...emptyState.calling,
activeCallState: {
state: 'Active',
callMode: CallMode.Direct,
conversationId,
hasLocalAudio: true,
hasLocalVideo: false,
localAudioLevel: 0,
viewMode: CallViewMode.Sidebar,
joinedAt: null,
outgoingRing: true,
pip: false,
settingsDialogOpen: false,
showParticipantsList: false,
},
},
};
await startCall({ await startCall({
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: 'fake-conversation-id', conversationId,
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: false, hasLocalVideo: false,
})(dispatch, getEmptyRootState, null); })(dispatch, () => startState, null);
const action = dispatch.getCall(0).args[0]; const action = dispatch.getCall(0).args[0];
const result = reducer(getEmptyState(), action); const result = reducer(startState.calling, action);
assert.deepEqual(result.callsByConversation['fake-conversation-id'], { assert.deepEqual(result.callsByConversation[conversationId], {
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: 'fake-conversation-id', conversationId,
callState: CallState.Prering, callState: CallState.Prering,
isIncoming: false, isIncoming: false,
isVideoCall: false, isVideoCall: false,
@@ -2618,7 +2698,7 @@ describe('calling duck', () => {
assert.deepEqual(result.activeCallState, { assert.deepEqual(result.activeCallState, {
state: 'Active', state: 'Active',
callMode: CallMode.Direct, callMode: CallMode.Direct,
conversationId: 'fake-conversation-id', conversationId,
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: false, hasLocalVideo: false,
localAudioLevel: 0, localAudioLevel: 0,
@@ -2633,12 +2713,36 @@ describe('calling duck', () => {
it("doesn't dispatch any actions for group calls", async () => { it("doesn't dispatch any actions for group calls", async () => {
const dispatch = sinon.spy(); const dispatch = sinon.spy();
const emptyState = getEmptyRootState();
const conversationId = '123';
const startState: StateType = {
...emptyState,
calling: {
...emptyState.calling,
activeCallState: {
state: 'Active',
callMode: CallMode.Group,
conversationId,
hasLocalAudio: true,
hasLocalVideo: false,
localAudioLevel: 0,
viewMode: CallViewMode.Sidebar,
joinedAt: null,
outgoingRing: true,
pip: false,
settingsDialogOpen: false,
showParticipantsList: false,
},
},
};
await startCall({ await startCall({
callMode: CallMode.Group, callMode: CallMode.Group,
conversationId: '123', conversationId,
hasLocalAudio: true, hasLocalAudio: true,
hasLocalVideo: false, hasLocalVideo: false,
})(dispatch, getEmptyRootState, null); })(dispatch, () => startState, null);
sinon.assert.notCalled(dispatch); sinon.assert.notCalled(dispatch);
}); });