From b9cd858ec78878970ef1f5d62478826ec31d196a Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Thu, 19 Sep 2024 09:29:14 +1000 Subject: [PATCH] Make startCallLobby resilient to re-entrant calls --- ts/state/ducks/calling.ts | 425 ++++++++++++------ ts/state/selectors/calling.ts | 15 +- ts/state/selectors/user.ts | 4 +- ts/state/smart/CallManager.tsx | 3 +- ts/test-electron/state/ducks/calling_test.ts | 200 ++++++++- .../state/selectors/calling_test.ts | 1 + 6 files changed, 498 insertions(+), 150 deletions(-) diff --git a/ts/state/ducks/calling.ts b/ts/state/ducks/calling.ts index 6a6ebdc53a84..0ec40d80ec04 100644 --- a/ts/state/ducks/calling.ts +++ b/ts/state/ducks/calling.ts @@ -167,6 +167,7 @@ export type GroupCallStateType = { // eslint-disable-next-line local-rules/type-alias-readonlydeep export type ActiveCallStateType = { + state: 'Active'; callMode: CallMode; conversationId: string; hasLocalAudio: boolean; @@ -184,6 +185,10 @@ export type ActiveCallStateType = { showParticipantsList: boolean; reactions?: ActiveCallReactionsType; }; +export type WaitingCallStateType = ReadonlyDeep<{ + state: 'Waiting'; + conversationId: string; +}>; // eslint-disable-next-line local-rules/type-alias-readonlydeep export type CallsByConversationType = { @@ -204,7 +209,7 @@ export type CallingStateType = MediaDeviceSettings & { callsByConversation: CallsByConversationType; adhocCalls: AdhocCallsType; callLinks: CallLinksByRoomIdType; - activeCallState?: ActiveCallStateType; + activeCallState?: ActiveCallStateType | WaitingCallStateType; }; export type AcceptCallType = ReadonlyDeep<{ @@ -430,8 +435,12 @@ export const getActiveCall = ({ return; } - const { callMode, conversationId } = activeCallState; - return callMode === CallMode.Adhoc + const { state, conversationId } = activeCallState; + if (state === 'Waiting') { + return; + } + + return activeCallState.callMode === CallMode.Adhoc ? getOwn(adhocCalls, conversationId) : getOwn(callsByConversation, conversationId); }; @@ -600,7 +609,10 @@ const CANCEL_INCOMING_GROUP_CALL_RING = const CHANGE_CALL_VIEW = 'calling/CHANGE_CALL_VIEW'; const DENY_USER = 'calling/DENY_USER'; const START_CALLING_LOBBY = 'calling/START_CALLING_LOBBY'; +const WAITING_FOR_CALLING_LOBBY = 'calling/WAITING_FOR_CALLING_LOBBY'; const START_CALL_LINK_LOBBY = 'calling/START_CALL_LINK_LOBBY'; +const WAITING_FOR_CALL_LINK_LOBBY = 'calling/WAITING_FOR_CALL_LINK_LOBBY'; +const CALL_LOBBY_FAILED = 'calling/CALL_LOBBY_FAILED'; const CALL_STATE_CHANGE_FULFILLED = 'calling/CALL_STATE_CHANGE_FULFILLED'; const CHANGE_IO_DEVICE_FULFILLED = 'calling/CHANGE_IO_DEVICE_FULFILLED'; const CLOSE_NEED_PERMISSION_SCREEN = 'calling/CLOSE_NEED_PERMISSION_SCREEN'; @@ -663,16 +675,30 @@ type DenyUserActionType = ReadonlyDeep<{ // eslint-disable-next-line local-rules/type-alias-readonlydeep type StartCallingLobbyActionType = { - type: 'calling/START_CALLING_LOBBY'; + type: typeof START_CALLING_LOBBY; payload: StartCallingLobbyPayloadType; }; +type WaitingForCallingLobbyActionType = ReadonlyDeep<{ + type: typeof WAITING_FOR_CALLING_LOBBY; + payload: { conversationId: string }; +}>; + // eslint-disable-next-line local-rules/type-alias-readonlydeep type StartCallLinkLobbyActionType = { - type: 'calling/START_CALL_LINK_LOBBY'; + type: typeof START_CALL_LINK_LOBBY; payload: StartCallLinkLobbyPayloadType; }; +type WaitingForCallLinkLobbyActionType = ReadonlyDeep<{ + type: typeof WAITING_FOR_CALL_LINK_LOBBY; + payload: { roomId: string }; +}>; +type CallLobbyFailedActionType = ReadonlyDeep<{ + type: typeof CALL_LOBBY_FAILED; + payload: { conversationId: string }; +}>; + type CallStateChangeFulfilledActionType = ReadonlyDeep<{ type: 'calling/CALL_STATE_CHANGE_FULFILLED'; payload: CallStateChangeType; @@ -904,6 +930,7 @@ type SwitchFromPresentationViewActionType = ReadonlyDeep<{ export type CallingActionType = | ApproveUserActionType | AcceptCallPendingActionType + | CallLobbyFailedActionType | CancelCallActionType | CancelIncomingGroupCallRingActionType | ChangeCallViewActionType @@ -946,7 +973,9 @@ export type CallingActionType = | SetPresentingFulfilledActionType | ToggleSettingsActionType | SwitchToPresentationViewActionType - | SwitchFromPresentationViewActionType; + | SwitchFromPresentationViewActionType + | WaitingForCallingLobbyActionType + | WaitingForCallLinkLobbyActionType; // Action Creators @@ -1851,7 +1880,11 @@ function setPresenting( const { activeCallState } = callingState; const activeCall = getActiveCall(callingState); - if (!activeCall || !activeCallState) { + if ( + !activeCall || + !activeCallState || + activeCallState.state === 'Waiting' + ) { log.warn('Trying to present when no call is active'); return; } @@ -1939,7 +1972,7 @@ function onOutgoingVideoCallInConversation( if (await isCallSafe(conversation.attributes, source)) { log.info( - 'onOutgoingVideoCallInConversation: call is deemed "safe". Making call' + 'onOutgoingVideoCallInConversation: call is deemed "safe". Starting lobby' ); dispatch( startCallingLobby({ @@ -1947,7 +1980,6 @@ function onOutgoingVideoCallInConversation( isVideoCall: true, }) ); - log.info('onOutgoingVideoCallInConversation: started the call'); } else { log.info( 'onOutgoingVideoCallInConversation: call is deemed "unsafe". Stopping' @@ -1980,13 +2012,12 @@ function onOutgoingAudioCallInConversation( if (await isCallSafe(conversation.attributes, source)) { log.info( - 'onOutgoingAudioCallInConversation: call is deemed "safe". Making call' + 'onOutgoingAudioCallInConversation: call is deemed "safe". Starting lobby' ); startCallingLobby({ conversationId, isVideoCall: false, })(dispatch, getState, undefined); - log.info('onOutgoingAudioCallInConversation: started the call'); } else { log.info( 'onOutgoingAudioCallInConversation: call is deemed "unsafe". Stopping' @@ -2118,10 +2149,12 @@ const _startCallLinkLobby = async ({ dispatch: ThunkDispatch< RootStateType, unknown, + | CallLobbyFailedActionType | StartCallLinkLobbyActionType | ShowErrorModalActionType | ToggleConfirmLeaveCallModalActionType | TogglePipActionType + | WaitingForCallLinkLobbyActionType >; getState: () => RootStateType; }) => { @@ -2129,9 +2162,17 @@ const _startCallLinkLobby = async ({ const roomId = getRoomIdFromRootKey(callLinkRootKey); const state = getState(); + const logId = `_startCallLinkLobby(${roomId})`; + const { activeCallState } = state.calling; if (activeCallState && activeCallState.conversationId === roomId) { - dispatch(togglePip()); + if (activeCallState.state === 'Active') { + dispatch(togglePip()); + } else { + log.warn( + `${logId}: Attempted to start lobby while already waiting for it!` + ); + } return; } if (activeCallState) { @@ -2144,50 +2185,51 @@ const _startCallLinkLobby = async ({ return; } - let callLinkState: CallLinkStateType | null = null; try { + dispatch({ + type: WAITING_FOR_CALL_LINK_LOBBY, + payload: { + roomId, + }, + }); + + let callLinkState: CallLinkStateType | null = null; callLinkState = await calling.readCallLink(callLinkRootKey); - } catch (error) { - log.error( - 'startCallLinkLobby: Error fetching call link state', - Errors.toLogFormat(error) - ); - } - if (callLinkState == null) { - const i18n = getIntl(getState()); - dispatch({ - type: SHOW_ERROR_MODAL, - payload: { - title: i18n('icu:calling__cant-join'), - description: i18n('icu:calling__call-link-connection-issues'), - buttonVariant: ButtonVariant.Primary, - }, - }); - return; - } - if ( - callLinkState.revoked || - callLinkState.expiration == null || - callLinkState.expiration < new Date().getTime() - ) { - const i18n = getIntl(getState()); - dispatch({ - type: SHOW_ERROR_MODAL, - payload: { - title: i18n('icu:calling__cant-join'), - description: i18n('icu:calling__call-link-no-longer-valid'), - buttonVariant: ButtonVariant.Primary, - }, - }); - return; - } + if (callLinkState == null) { + const i18n = getIntl(getState()); + dispatch({ + type: SHOW_ERROR_MODAL, + payload: { + title: i18n('icu:calling__cant-join'), + description: i18n('icu:calling__call-link-connection-issues'), + buttonVariant: ButtonVariant.Primary, + }, + }); + return; + } + + if ( + callLinkState.revoked || + callLinkState.expiration == null || + callLinkState.expiration < new Date().getTime() + ) { + const i18n = getIntl(getState()); + dispatch({ + type: SHOW_ERROR_MODAL, + payload: { + title: i18n('icu:calling__cant-join'), + description: i18n('icu:calling__call-link-no-longer-valid'), + buttonVariant: ButtonVariant.Primary, + }, + }); + return; + } - try { const callLinkExists = await DataReader.callLinkExists(roomId); if (callLinkExists) { await DataWriter.updateCallLinkState(roomId, callLinkState); - log.info('startCallLinkLobby: Updated existing call link', roomId); + log.info(`${logId}: Updated existing call link`); } else { const { name, restrictions, expiration, revoked } = callLinkState; await DataWriter.insertCallLink({ @@ -2200,44 +2242,56 @@ const _startCallLinkLobby = async ({ expiration, storageNeedsSync: false, }); - log.info('startCallLinkLobby: Saved new call link', roomId); + log.info(`${logId}: Saved new call link`); } - } catch (err) { - log.error( - 'startCallLinkLobby: Call link DB error', - Errors.toLogFormat(err) - ); + + const groupCall = getGroupCall(roomId, state.calling, CallMode.Adhoc); + const groupCallDeviceCount = + groupCall?.peekInfo?.deviceCount || + groupCall?.remoteParticipants.length || + 0; + + const { adminKey } = getOwn(state.calling.callLinks, roomId) ?? {}; + const adminPasskey = adminKey ? toAdminKeyBytes(adminKey) : undefined; + + const callLobbyData = await calling.startCallLinkLobby({ + callLinkRootKey, + adminPasskey, + hasLocalAudio: + groupCallDeviceCount < MAX_CALL_PARTICIPANTS_FOR_DEFAULT_MUTE, + }); + if (!callLobbyData) { + throw new Error('Failed to start call lobby'); + } + + dispatch({ + type: START_CALL_LINK_LOBBY, + payload: { + ...callLobbyData, + callLinkState, + callLinkRoomId: roomId, + callLinkRootKey: rootKey, + conversationId: roomId, + isConversationTooBigToRing: false, + }, + }); + } catch (error) { + log.error(`${logId}: Failed to start lobby`, Errors.toLogFormat(error)); + + try { + calling.stopCallingLobby(roomId); + } catch (innerError) { + log.error( + `${logId}: Failed to stop calling lobby`, + Errors.toLogFormat(innerError) + ); + } + + dispatch({ + type: CALL_LOBBY_FAILED, + payload: { conversationId: roomId }, + }); } - - const groupCall = getGroupCall(roomId, state.calling, CallMode.Adhoc); - const groupCallDeviceCount = - groupCall?.peekInfo?.deviceCount || - groupCall?.remoteParticipants.length || - 0; - - const { adminKey } = getOwn(state.calling.callLinks, roomId) ?? {}; - const adminPasskey = adminKey ? toAdminKeyBytes(adminKey) : undefined; - const callLobbyData = await calling.startCallLinkLobby({ - callLinkRootKey, - adminPasskey, - hasLocalAudio: - groupCallDeviceCount < MAX_CALL_PARTICIPANTS_FOR_DEFAULT_MUTE, - }); - if (!callLobbyData) { - return; - } - - dispatch({ - type: START_CALL_LINK_LOBBY, - payload: { - ...callLobbyData, - callLinkState, - callLinkRoomId: roomId, - callLinkRootKey: rootKey, - conversationId: roomId, - isConversationTooBigToRing: false, - }, - }); }; function leaveCurrentCallAndStartCallingLobby( @@ -2275,9 +2329,11 @@ function startCallingLobby({ void, RootStateType, unknown, + | CallLobbyFailedActionType | StartCallingLobbyActionType | ToggleConfirmLeaveCallModalActionType | TogglePipActionType + | WaitingForCallingLobbyActionType > { return async (dispatch, getState) => { const state = getState(); @@ -2290,10 +2346,17 @@ function startCallingLobby({ "startCallingLobby: can't start lobby without a conversation" ); + const logId = `startCallingLobby(${getConversationIdForLogging(conversation)})`; const { activeCallState } = state.calling; if (activeCallState && activeCallState.conversationId === conversationId) { - dispatch(togglePip()); + if (activeCallState.state === 'Active') { + dispatch(togglePip()); + } else { + log.warn( + `${logId}: Attempted to start lobby while already waiting for it!` + ); + } return; } if (activeCallState) { @@ -2307,35 +2370,59 @@ function startCallingLobby({ return; } - // The group call device count is considered 0 for a direct call. - const groupCall = getGroupCall( - conversationId, - state.calling, - CallMode.Group - ); - const groupCallDeviceCount = - groupCall?.peekInfo?.deviceCount || - groupCall?.remoteParticipants.length || - 0; + try { + dispatch({ + type: WAITING_FOR_CALLING_LOBBY, + payload: { + conversationId, + }, + }); - const callLobbyData = await calling.startCallingLobby({ - conversation, - hasLocalAudio: - groupCallDeviceCount < MAX_CALL_PARTICIPANTS_FOR_DEFAULT_MUTE, - hasLocalVideo: isVideoCall, - }); - if (!callLobbyData) { - return; - } - - dispatch({ - type: START_CALLING_LOBBY, - payload: { - ...callLobbyData, + // The group call device count is considered 0 for a direct call. + const groupCall = getGroupCall( conversationId, - isConversationTooBigToRing: isConversationTooBigToRing(conversation), - }, - }); + state.calling, + CallMode.Group + ); + const groupCallDeviceCount = + groupCall?.peekInfo?.deviceCount || + groupCall?.remoteParticipants.length || + 0; + const callLobbyData = await calling.startCallingLobby({ + conversation, + hasLocalAudio: + groupCallDeviceCount < MAX_CALL_PARTICIPANTS_FOR_DEFAULT_MUTE, + hasLocalVideo: isVideoCall, + }); + if (!callLobbyData) { + throw new Error('Failed to start call lobby'); + } + + dispatch({ + type: START_CALLING_LOBBY, + payload: { + ...callLobbyData, + conversationId, + isConversationTooBigToRing: isConversationTooBigToRing(conversation), + }, + }); + } catch (error) { + log.error(`${logId}: Failed to start lobby`, Errors.toLogFormat(error)); + + try { + calling.stopCallingLobby(conversationId); + } catch (innerError) { + log.error( + `${logId}: Failed to stop calling lobby`, + Errors.toLogFormat(innerError) + ); + } + + dispatch({ + type: CALL_LOBBY_FAILED, + payload: { conversationId }, + }); + } }; } @@ -2345,10 +2432,16 @@ function startCall( return async (dispatch, getState) => { const { callMode, conversationId, hasLocalAudio, hasLocalVideo } = payload; + const logId = `startCall(${conversationId})`; const state = getState(); const { activeCallState } = state.calling; - log.info(`startCall for conversation ${conversationId}, mode ${callMode}`); + log.info(`${logId}: starting, mode ${callMode}`); + + if (activeCallState?.state === 'Waiting') { + log.error(`${logId}: Call is not ready; `); + return; + } switch (callMode) { case CallMode.Direct: @@ -2609,6 +2702,40 @@ export function reducer( ): CallingStateType { const { callsByConversation, adhocCalls } = state; + if (action.type === WAITING_FOR_CALLING_LOBBY) { + const { conversationId } = action.payload; + + return { + ...state, + activeCallState: { + state: 'Waiting', + conversationId, + }, + }; + } + if (action.type === WAITING_FOR_CALL_LINK_LOBBY) { + const { roomId } = action.payload; + + return { + ...state, + activeCallState: { + state: 'Waiting', + conversationId: roomId, + }, + }; + } + if (action.type === CALL_LOBBY_FAILED) { + const { conversationId } = action.payload; + + const { activeCallState } = state; + if (!activeCallState || activeCallState.conversationId !== conversationId) { + log.warn( + `${action.type}: Active call does not match target conversation` + ); + } + + return removeConversationFromState(state, conversationId); + } if ( action.type === START_CALLING_LOBBY || action.type === START_CALL_LINK_LOBBY @@ -2708,6 +2835,7 @@ export function reducer( } : callLinks, activeCallState: { + state: 'Active', callMode, conversationId, hasLocalAudio: action.payload.hasLocalAudio, @@ -2737,6 +2865,7 @@ export function reducer( }, }, activeCallState: { + state: 'Active', callMode: CallMode.Direct, conversationId: action.payload.conversationId, hasLocalAudio: action.payload.hasLocalAudio, @@ -2765,6 +2894,7 @@ export function reducer( return { ...state, activeCallState: { + state: 'Active', callMode: call.callMode, conversationId: action.payload.conversationId, hasLocalAudio: true, @@ -2787,7 +2917,7 @@ export function reducer( ) { const activeCall = getActiveCall(state); if (!activeCall) { - log.warn('No active call to remove'); + log.warn(`${action.type}: No active call to remove`); return state; } switch (activeCall.callMode) { @@ -2822,6 +2952,7 @@ export function reducer( const activeCall = getActiveCall(state); const { activeCallState } = state; if ( + activeCallState?.state === 'Waiting' || !activeCallState?.outgoingRing || activeCallState.conversationId !== action.payload.id || !isGroupOrAdhocCallState(activeCall) || @@ -2927,6 +3058,7 @@ export function reducer( }, }, activeCallState: { + state: 'Active', callMode: CallMode.Direct, conversationId: action.payload.conversationId, hasLocalAudio: action.payload.hasLocalAudio, @@ -2956,7 +3088,9 @@ export function reducer( calling.notifyScreenShareStatus({ callMode: CallMode.Direct, callState: action.payload.callState, - isPresenting: state.activeCallState?.presentingSource != null, + isPresenting: + state.activeCallState?.state === 'Active' && + state.activeCallState?.presentingSource != null, conversationId: state.activeCallState?.conversationId, }) ); @@ -2977,9 +3111,10 @@ export function reducer( return state; } - let activeCallState: undefined | ActiveCallStateType; + let activeCallState: undefined | ActiveCallStateType | WaitingCallStateType; if ( - state.activeCallState?.conversationId === action.payload.conversationId + state.activeCallState?.conversationId === action.payload.conversationId && + state.activeCallState.state === 'Active' ) { activeCallState = { ...state.activeCallState, @@ -3011,7 +3146,12 @@ export function reducer( // The PiP check is an optimization. We don't need to update audio levels if the user // cannot see them. - if (!activeCallState || activeCallState.pip || !existingCall) { + if ( + !activeCallState || + activeCallState.state === 'Waiting' || + activeCallState.pip || + !existingCall + ) { return state; } @@ -3113,8 +3253,14 @@ export function reducer( deviceCount: remoteParticipants.length, }; - let newActiveCallState: ActiveCallStateType | undefined; - if (state.activeCallState?.conversationId === conversationId) { + let newActiveCallState: + | undefined + | ActiveCallStateType + | WaitingCallStateType; + if ( + state.activeCallState?.state === 'Active' && + state.activeCallState?.conversationId === conversationId + ) { newActiveCallState = connectionState === GroupCallConnectionState.NotConnected ? undefined @@ -3140,6 +3286,7 @@ export function reducer( if ( newActiveCallState && + newActiveCallState.state === 'Active' && newActiveCallState.outgoingRing && newActiveCallState.conversationId === conversationId && isAnybodyElseInGroupCall(newPeekInfo, ourAci) @@ -3174,7 +3321,9 @@ export function reducer( calling.notifyScreenShareStatus({ callMode, connectionState, - isPresenting: state.activeCallState?.presentingSource != null, + isPresenting: + state.activeCallState?.state === 'Active' && + state.activeCallState?.presentingSource != null, conversationId: state.activeCallState?.conversationId, }) ); @@ -3246,7 +3395,10 @@ export function reducer( action.type === GROUP_CALL_REACTIONS_RECEIVED ) { const { callMode, conversationId, timestamp } = action.payload; - if (state.activeCallState?.conversationId !== conversationId) { + if ( + state.activeCallState?.state === 'Waiting' || + state.activeCallState?.conversationId !== conversationId + ) { return state; } @@ -3300,6 +3452,7 @@ export function reducer( if (action.type === GROUP_CALL_REACTIONS_EXPIRED) { const { conversationId, timestamp: receivedAt } = action.payload; if ( + state.activeCallState?.state === 'Waiting' || state.activeCallState?.conversationId !== conversationId || !state.activeCallState?.reactions ) { @@ -3386,7 +3539,7 @@ export function reducer( if (action.type === RETURN_TO_ACTIVE_CALL) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot return to active call if there is no active call'); return state; } @@ -3401,7 +3554,7 @@ export function reducer( } if (action.type === SET_LOCAL_AUDIO_FULFILLED) { - if (!state.activeCallState) { + if (state.activeCallState?.state !== 'Active') { log.warn('Cannot set local audio with no active call'); return state; } @@ -3416,7 +3569,7 @@ export function reducer( } if (action.type === SET_LOCAL_VIDEO_FULFILLED) { - if (!state.activeCallState) { + if (state.activeCallState?.state !== 'Active') { log.warn('Cannot set local video with no active call'); return state; } @@ -3471,7 +3624,7 @@ export function reducer( if (action.type === TOGGLE_SETTINGS) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot toggle settings when there is no active call'); return state; } @@ -3487,7 +3640,7 @@ export function reducer( if (action.type === TOGGLE_PARTICIPANTS) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot toggle participants list when there is no active call'); return state; } @@ -3503,7 +3656,7 @@ export function reducer( if (action.type === TOGGLE_PIP) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot toggle PiP when there is no active call'); return state; } @@ -3519,7 +3672,7 @@ export function reducer( if (action.type === SET_PRESENTING) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot toggle presenting when there is no active call'); return state; } @@ -3536,7 +3689,7 @@ export function reducer( if (action.type === SET_PRESENTING_SOURCES) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot set presenting sources when there is no active call'); return state; } @@ -3552,7 +3705,7 @@ export function reducer( if (action.type === SET_OUTGOING_RING) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot set outgoing ring when there is no active call'); return state; } @@ -3568,7 +3721,7 @@ export function reducer( if (action.type === TOGGLE_NEEDS_SCREEN_RECORDING_PERMISSIONS) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot set presenting sources when there is no active call'); return state; } @@ -3585,7 +3738,7 @@ export function reducer( if (action.type === CHANGE_CALL_VIEW) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot change call view when there is no active call'); return state; } @@ -3609,7 +3762,7 @@ export function reducer( if (action.type === SWITCH_TO_PRESENTATION_VIEW) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot switch to speaker view when there is no active call'); return state; } @@ -3630,7 +3783,7 @@ export function reducer( if (action.type === SWITCH_FROM_PRESENTATION_VIEW) { const { activeCallState } = state; - if (!activeCallState) { + if (activeCallState?.state !== 'Active') { log.warn('Cannot switch to speaker view when there is no active call'); return state; } diff --git a/ts/state/selectors/calling.ts b/ts/state/selectors/calling.ts index 4b974a8175dd..3bffa2308edd 100644 --- a/ts/state/selectors/calling.ts +++ b/ts/state/selectors/calling.ts @@ -11,6 +11,7 @@ import type { CallLinksByRoomIdType, DirectCallStateType, GroupCallStateType, + ActiveCallStateType, } from '../ducks/calling'; import { getIncomingCall as getIncomingCallHelper } from '../ducks/callingHelpers'; import { CallMode } from '../../types/CallDisposition'; @@ -55,7 +56,13 @@ export const getSelectedCamera = createSelector( export const getActiveCallState = createSelector( getCalling, - (state: CallingStateType) => state.activeCallState + (state: CallingStateType) => { + if (state.activeCallState?.state !== 'Active') { + return undefined; + } + + return state.activeCallState; + } ); export const getCallsByConversation = createSelector( @@ -134,9 +141,9 @@ export const isInCall = createSelector( ); export const isInFullScreenCall = createSelector( - getCalling, - (state: CallingStateType): boolean => - Boolean(state.activeCallState && !state.activeCallState.pip) + getActiveCallState, + (activeCallState: undefined | ActiveCallStateType): boolean => + Boolean(activeCallState?.pip) ); export const getIncomingCall = createSelector( diff --git a/ts/state/selectors/user.ts b/ts/state/selectors/user.ts index b10ae99dd996..1a4ff25830cb 100644 --- a/ts/state/selectors/user.ts +++ b/ts/state/selectors/user.ts @@ -90,7 +90,9 @@ export const getPreferredTheme = createSelector( const getIsInFullScreenCall = createSelector( (state: StateType): CallingStateType => state.calling, (state: CallingStateType): boolean => - Boolean(state.activeCallState && !state.activeCallState.pip) + Boolean( + state.activeCallState?.state === 'Active' && !state.activeCallState.pip + ) ); export const getTheme = createSelector( diff --git a/ts/state/smart/CallManager.tsx b/ts/state/smart/CallManager.tsx index 383499b6e1f1..57946110d2e3 100644 --- a/ts/state/smart/CallManager.tsx +++ b/ts/state/smart/CallManager.tsx @@ -46,6 +46,7 @@ import type { ConversationType } from '../ducks/conversations'; import type { StateType } from '../reducer'; import { getHasInitialLoadCompleted } from '../selectors/app'; import { + getActiveCallState, getAvailableCameras, getCallLinkSelector, getIncomingCall, @@ -122,7 +123,7 @@ const mapStateToActiveCallProp = ( state: StateType ): undefined | ActiveCallType => { const { calling } = state; - const { activeCallState } = calling; + const activeCallState = getActiveCallState(state); if (!activeCallState) { return undefined; diff --git a/ts/test-electron/state/ducks/calling_test.ts b/ts/test-electron/state/ducks/calling_test.ts index 1749e41abfd2..75079fe5ed18 100644 --- a/ts/test-electron/state/ducks/calling_test.ts +++ b/ts/test-electron/state/ducks/calling_test.ts @@ -44,6 +44,7 @@ import { FAKE_CALL_LINK_WITH_ADMIN_KEY, getCallLinkState, } from '../../../test-both/helpers/fakeCallLink'; +import { strictAssert } from '../../../util/assert'; const ACI_1 = generateAci(); const NOW = new Date('2020-01-23T04:56:00.000'); @@ -71,6 +72,7 @@ describe('calling duck', () => { const stateWithActiveDirectCall: CallingStateTypeWithActiveCall = { ...stateWithDirectCall, activeCallState: { + state: 'Active', callMode: CallMode.Direct, conversationId: directCallState.conversationId, hasLocalAudio: true, @@ -174,6 +176,7 @@ describe('calling duck', () => { }; const groupCallActiveCallState: ActiveCallStateType = { + state: 'Active', callMode: CallMode.Group, conversationId: 'fake-group-call-conversation-id', hasLocalAudio: true, @@ -383,6 +386,10 @@ describe('calling duck', () => { const nextState = reducer(getState().calling, action); assert.isDefined(nextState.activeCallState); + strictAssert( + nextState.activeCallState?.state === 'Active', + 'state is active' + ); assert.equal( nextState.activeCallState?.presentingSource, presentedSource @@ -410,6 +417,10 @@ describe('calling duck', () => { const nextState = reducer(getState().calling, action); assert.isDefined(nextState.activeCallState); + strictAssert( + nextState.activeCallState?.state === 'Active', + 'state is active' + ); assert.isUndefined(nextState.activeCallState?.presentingSource); assert.isUndefined( nextState.activeCallState?.presentingSourcesAvailable @@ -506,6 +517,7 @@ describe('calling duck', () => { const result = reducer(stateWithIncomingDirectCall, action); assert.deepEqual(result.activeCallState, { + state: 'Active', callMode: CallMode.Direct, conversationId: 'fake-direct-call-conversation-id', hasLocalAudio: true, @@ -600,6 +612,7 @@ describe('calling duck', () => { const result = reducer(stateWithIncomingGroupCall, action); assert.deepEqual(result.activeCallState, { + state: 'Active', callMode: CallMode.Group, conversationId: 'fake-group-call-conversation-id', hasLocalAudio: true, @@ -893,6 +906,10 @@ describe('calling duck', () => { }); const result = reducer(stateWithActiveGroupCall, action); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.strictEqual( result.activeCallState?.localAudioLevel, truncateAudioLevel(0.8) @@ -1228,6 +1245,7 @@ describe('calling duck', () => { ); assert.deepEqual(result.activeCallState, { + state: 'Active', callMode: CallMode.Group, conversationId: 'fake-group-call-conversation-id', hasLocalAudio: true, @@ -1278,6 +1296,10 @@ describe('calling duck', () => { result.activeCallState?.conversationId, 'fake-group-call-conversation-id' ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isTrue(result.activeCallState?.hasLocalAudio); assert.isTrue(result.activeCallState?.hasLocalVideo); }); @@ -1310,6 +1332,10 @@ describe('calling duck', () => { }) ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isTrue(result.activeCallState?.outgoingRing); }); @@ -1341,6 +1367,10 @@ describe('calling duck', () => { }) ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isFalse(result.activeCallState?.outgoingRing); }); @@ -1368,6 +1398,10 @@ describe('calling duck', () => { }) ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isFalse(result.activeCallState?.hasLocalAudio); }); @@ -1395,6 +1429,10 @@ describe('calling duck', () => { }) ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isTrue(result.activeCallState?.hasLocalAudio); }); @@ -1419,6 +1457,10 @@ describe('calling duck', () => { }) ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isTrue(result.activeCallState?.hasLocalAudio); }); }); @@ -1549,7 +1591,13 @@ describe('calling duck', () => { const { roomId, rootKey } = FAKE_CALL_LINK; const { dispatch } = await doAction({ rootKey }); - sinon.assert.calledOnce(dispatch); + sinon.assert.calledTwice(dispatch); + sinon.assert.calledWith(dispatch, { + type: 'calling/WAITING_FOR_CALL_LINK_LOBBY', + payload: { + roomId, + }, + }); sinon.assert.calledWith(dispatch, { type: 'calling/START_CALL_LINK_LOBBY', payload: { @@ -1789,6 +1837,11 @@ describe('calling duck', () => { ], }); const firstResult = reducer(getState().calling, firstAction); + + strictAssert( + firstResult.activeCallState?.state === 'Active', + 'state is active' + ); assert.deepEqual(firstResult.activeCallState?.reactions, [ { timestamp: NOW.getTime(), @@ -1810,6 +1863,11 @@ describe('calling duck', () => { ], }); const secondResult = reducer(firstResult, secondAction); + + strictAssert( + secondResult.activeCallState?.state === 'Active', + 'state is active' + ); assert.deepEqual(secondResult.activeCallState?.reactions, [ { timestamp: NOW.getTime(), @@ -1840,6 +1898,11 @@ describe('calling duck', () => { ], }); const result = reducer(getState().calling, action); + + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.deepEqual(result.activeCallState?.reactions, [ { timestamp: NOW.getTime(), @@ -1892,6 +1955,10 @@ describe('calling duck', () => { }); const result = reducer(getState().calling, action); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.deepEqual(result.activeCallState?.reactions, [ { timestamp: NOW.getTime(), @@ -1981,6 +2048,10 @@ describe('calling duck', () => { const result = reducer(stateWithActiveDirectCall, action); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isFalse(result.activeCallState?.hasLocalAudio); }); }); @@ -1992,6 +2063,10 @@ describe('calling duck', () => { const action = setOutgoingRing(true); const result = reducer(stateWithActiveGroupCall, action); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isTrue(result.activeCallState?.outgoingRing); }); @@ -1999,6 +2074,10 @@ describe('calling duck', () => { const action = setOutgoingRing(false); const result = reducer(stateWithActiveDirectCall, action); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isFalse(result.activeCallState?.outgoingRing); }); }); @@ -2092,7 +2171,7 @@ describe('calling duck', () => { }); }); - it('dispatches an action if the calling lobby returns something', async () => { + it('dispatches two actions if the calling lobby returns something', async () => { startCallingLobbyStub.resolves({ callMode: CallMode.Direct, hasLocalAudio: true, @@ -2101,23 +2180,55 @@ describe('calling duck', () => { const dispatch = sinon.stub(); + const conversationId = 'fake-conversation-id'; await startCallingLobby({ - conversationId: 'fake-conversation-id', + conversationId, isVideoCall: true, })(dispatch, () => rootState, null); - sinon.assert.calledOnce(dispatch); + sinon.assert.calledTwice(dispatch); + + sinon.assert.calledWith(dispatch, { + type: 'calling/WAITING_FOR_CALLING_LOBBY', + payload: { + conversationId, + }, + }); + sinon.assert.calledWith(dispatch, { + type: 'calling/START_CALLING_LOBBY', + payload: { + callMode: 'Direct', + hasLocalAudio: true, + hasLocalVideo: true, + conversationId, + isConversationTooBigToRing: false, + }, + }); }); - it("doesn't dispatch an action if the calling lobby returns nothing", async () => { + it('dispatches two actions if the calling lobby returns nothing', async () => { const dispatch = sinon.stub(); + const conversationId = 'fake-conversation-id'; await startCallingLobby({ - conversationId: 'fake-conversation-id', + conversationId, isVideoCall: true, })(dispatch, () => rootState, null); - sinon.assert.notCalled(dispatch); + sinon.assert.calledTwice(dispatch); + + sinon.assert.calledWith(dispatch, { + type: 'calling/WAITING_FOR_CALLING_LOBBY', + payload: { + conversationId, + }, + }); + sinon.assert.calledWith(dispatch, { + type: 'calling/CALL_LOBBY_FAILED', + payload: { + conversationId, + }, + }); }); }); @@ -2138,7 +2249,10 @@ describe('calling duck', () => { isVideoCall: true, })(dispatch, () => ({ ...rootState, calling: callingState }), null); - const action = dispatch.getCall(0).args[0]; + const waitingAction = dispatch.getCall(0).args[0]; + assert.equal(waitingAction.type, 'calling/WAITING_FOR_CALLING_LOBBY'); + + const action = dispatch.getCall(1).args[0]; return reducer(callingState, action); }; @@ -2157,6 +2271,7 @@ describe('calling duck', () => { isVideoCall: true, }); assert.deepEqual(result.activeCallState, { + state: 'Active', callMode: CallMode.Direct, conversationId: 'fake-conversation-id', hasLocalAudio: true, @@ -2231,6 +2346,10 @@ describe('calling duck', () => { result.activeCallState?.conversationId, 'fake-conversation-id' ); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isFalse(result.activeCallState?.outgoingRing); }); @@ -2397,6 +2516,10 @@ describe('calling duck', () => { remoteParticipants: [], }); + strictAssert( + result.activeCallState?.state === 'Active', + 'state is active' + ); assert.isTrue(result.activeCallState?.outgoingRing); }); }); @@ -2470,6 +2593,7 @@ describe('calling duck', () => { isVideoCall: false, }); assert.deepEqual(result.activeCallState, { + state: 'Active', callMode: CallMode.Direct, conversationId: 'fake-conversation-id', hasLocalAudio: true, @@ -2508,8 +2632,22 @@ describe('calling duck', () => { const afterTwoToggles = reducer(afterOneToggle, toggleSettings()); const afterThreeToggles = reducer(afterTwoToggles, toggleSettings()); + strictAssert( + afterOneToggle.activeCallState?.state === 'Active', + 'state is active #1' + ); assert.isTrue(afterOneToggle.activeCallState?.settingsDialogOpen); + + strictAssert( + afterTwoToggles.activeCallState?.state === 'Active', + 'state is active #2' + ); assert.isFalse(afterTwoToggles.activeCallState?.settingsDialogOpen); + + strictAssert( + afterThreeToggles.activeCallState?.state === 'Active', + 'state is active #3' + ); assert.isTrue(afterThreeToggles.activeCallState?.settingsDialogOpen); }); }); @@ -2528,8 +2666,22 @@ describe('calling duck', () => { toggleParticipants() ); + strictAssert( + afterOneToggle.activeCallState?.state === 'Active', + 'state is active #1' + ); assert.isTrue(afterOneToggle.activeCallState?.showParticipantsList); + + strictAssert( + afterTwoToggles.activeCallState?.state === 'Active', + 'state is active #2' + ); assert.isFalse(afterTwoToggles.activeCallState?.showParticipantsList); + + strictAssert( + afterThreeToggles.activeCallState?.state === 'Active', + 'state is active #3' + ); assert.isTrue(afterThreeToggles.activeCallState?.showParticipantsList); }); }); @@ -2542,8 +2694,22 @@ describe('calling duck', () => { const afterTwoToggles = reducer(afterOneToggle, togglePip()); const afterThreeToggles = reducer(afterTwoToggles, togglePip()); + strictAssert( + afterOneToggle.activeCallState?.state === 'Active', + 'state is active #1' + ); assert.isTrue(afterOneToggle.activeCallState?.pip); + + strictAssert( + afterTwoToggles.activeCallState?.state === 'Active', + 'state is active #2' + ); assert.isFalse(afterTwoToggles.activeCallState?.pip); + + strictAssert( + afterThreeToggles.activeCallState?.state === 'Active', + 'state is active #3' + ); assert.isTrue(afterThreeToggles.activeCallState?.pip); }); }); @@ -2569,14 +2735,28 @@ describe('calling duck', () => { switchFromPresentationView() ); + strictAssert( + afterOneToggle.activeCallState?.state === 'Active', + 'state is active #1' + ); assert.strictEqual( afterOneToggle.activeCallState?.viewMode, CallViewMode.Presentation ); + + strictAssert( + afterTwoToggles.activeCallState?.state === 'Active', + 'state is active #2' + ); assert.strictEqual( afterTwoToggles.activeCallState?.viewMode, CallViewMode.Presentation ); + + strictAssert( + afterThreeToggles.activeCallState?.state === 'Active', + 'state is active #3' + ); assert.strictEqual( afterThreeToggles.activeCallState?.viewMode, CallViewMode.Paginated @@ -2597,6 +2777,10 @@ describe('calling duck', () => { switchFromPresentationView() ); + strictAssert( + stateAfterPresentation.activeCallState?.state === 'Active', + 'state is active' + ); assert.strictEqual( stateAfterPresentation.activeCallState?.viewMode, CallViewMode.Overflow diff --git a/ts/test-electron/state/selectors/calling_test.ts b/ts/test-electron/state/selectors/calling_test.ts index 63b9755db97b..78dfb35516ee 100644 --- a/ts/test-electron/state/selectors/calling_test.ts +++ b/ts/test-electron/state/selectors/calling_test.ts @@ -62,6 +62,7 @@ describe('state/selectors/calling', () => { const stateWithActiveDirectCall: CallingStateType = { ...stateWithDirectCall, activeCallState: { + state: 'Active', callMode: CallMode.Direct, conversationId: 'fake-direct-call-conversation-id', hasLocalAudio: true,