diff --git a/spec/test-utils/webrtc.ts b/spec/test-utils/webrtc.ts index c1a2c2436..46b90b7a5 100644 --- a/spec/test-utils/webrtc.ts +++ b/spec/test-utils/webrtc.ts @@ -123,6 +123,7 @@ export class MockRTCPeerConnection { public iceCandidateListener?: (e: RTCPeerConnectionIceEvent) => void; public iceConnectionStateChangeListener?: () => void; public onTrackListener?: (e: RTCTrackEvent) => void; + public onDataChannelListener?: (ev: RTCDataChannelEvent) => void; public needsNegotiation = false; public readyToNegotiate: Promise; private onReadyToNegotiate?: () => void; @@ -168,6 +169,8 @@ export class MockRTCPeerConnection { this.iceConnectionStateChangeListener = listener; } else if (type == "track") { this.onTrackListener = listener; + } else if (type == "datachannel") { + this.onDataChannelListener = listener; } } public createDataChannel(label: string, opts: RTCDataChannelInit) { @@ -232,6 +235,10 @@ export class MockRTCPeerConnection { this.negotiationNeededListener(); } } + + public triggerIncomingDataChannel(): void { + this.onDataChannelListener?.({ channel: {} } as RTCDataChannelEvent); + } } export class MockRTCRtpSender { diff --git a/spec/unit/webrtc/call.spec.ts b/spec/unit/webrtc/call.spec.ts index 51798d441..db84ee540 100644 --- a/spec/unit/webrtc/call.spec.ts +++ b/spec/unit/webrtc/call.spec.ts @@ -431,6 +431,33 @@ describe("Call", function () { expect(transceivers.get("m.usermedia:video")!.sender.track!.id).toBe("usermedia_video_track"); }); + it("should handle error on call upgrade", async () => { + const onError = jest.fn(); + call.on(CallEvent.Error, onError); + + await startVoiceCall(client, call); + + await call.onAnswerReceived( + makeMockEvent("@test:foo", { + version: 1, + call_id: call.callId, + party_id: "party_id", + answer: { + sdp: DUMMY_SDP, + }, + [SDPStreamMetadataKey]: {}, + }), + ); + + const mockGetUserMediaStream = jest.fn().mockRejectedValue(new Error("Test error")); + client.client.getMediaHandler().getUserMediaStream = mockGetUserMediaStream; + + // then unmute which should cause an upgrade + await call.setLocalVideoMuted(false); + + expect(onError).toHaveBeenCalled(); + }); + it("should unmute video after upgrading to video call", async () => { // Regression test for https://github.com/vector-im/element-call/issues/925 await startVoiceCall(client, call); @@ -737,11 +764,22 @@ describe("Call", function () { const dataChannel = call.createDataChannel("data_channel_label", { id: 123 }); - expect(dataChannelCallback).toHaveBeenCalledWith(dataChannel); + expect(dataChannelCallback).toHaveBeenCalledWith(dataChannel, call); expect(dataChannel.label).toBe("data_channel_label"); expect(dataChannel.id).toBe(123); }); + it("should emit a data channel event when the other side adds a data channel", async () => { + await startVoiceCall(client, call); + + const dataChannelCallback = jest.fn(); + call.on(CallEvent.DataChannel, dataChannelCallback); + + (call.peerConn as unknown as MockRTCPeerConnection).triggerIncomingDataChannel(); + + expect(dataChannelCallback).toHaveBeenCalled(); + }); + describe("supportsMatrixCall", () => { it("should return true when the environment is right", () => { expect(supportsMatrixCall()).toBe(true); @@ -1604,7 +1642,7 @@ describe("Call", function () { hasAdvancedBy += advanceBy; expect(lengthChangedListener).toHaveBeenCalledTimes(hasAdvancedBy); - expect(lengthChangedListener).toHaveBeenCalledWith(hasAdvancedBy); + expect(lengthChangedListener).toHaveBeenCalledWith(hasAdvancedBy, call); } }); @@ -1634,4 +1672,24 @@ describe("Call", function () { expect(call.hangup).not.toHaveBeenCalled(); }); }); + + describe("Call replace", () => { + it("Fires event when call replaced", async () => { + const onReplace = jest.fn(); + call.on(CallEvent.Replaced, onReplace); + + await call.placeVoiceCall(); + + const call2 = new MatrixCall({ + client: client.client, + roomId: FAKE_ROOM_ID, + }); + call2.on(CallEvent.Error, errorListener); + await fakeIncomingCall(client, call2); + + call.replacedBy(call2); + + expect(onReplace).toHaveBeenCalled(); + }); + }); }); diff --git a/spec/unit/webrtc/callFeed.spec.ts b/spec/unit/webrtc/callFeed.spec.ts index 803e4648e..ea80d5267 100644 --- a/spec/unit/webrtc/callFeed.spec.ts +++ b/spec/unit/webrtc/callFeed.spec.ts @@ -102,7 +102,7 @@ describe("CallFeed", () => { [CallState.Connected, true], [CallState.Connecting, false], ])("should react to call state, when !isLocal()", (state: CallState, expected: Boolean) => { - call.emit(CallEvent.State, state); + call.emit(CallEvent.State, state, CallState.InviteSent, call.typed()); expect(feed.connected).toBe(expected); }); diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 031f8eef3..22af26e0c 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -794,7 +794,7 @@ describe("Group Call", function () { call.isLocalVideoMuted = jest.fn().mockReturnValue(true); call.setLocalVideoMuted = jest.fn(); - call.emit(CallEvent.State, CallState.Connected); + call.emit(CallEvent.State, CallState.Connected, CallState.InviteSent, call); expect(call.setMicrophoneMuted).toHaveBeenCalledWith(false); expect(call.setLocalVideoMuted).toHaveBeenCalledWith(false); @@ -1154,7 +1154,7 @@ describe("Group Call", function () { }); it("handles regular case", () => { - oldMockCall.emit(CallEvent.Replaced, newMockCall.typed()); + oldMockCall.emit(CallEvent.Replaced, newMockCall.typed(), oldMockCall.typed()); expect(oldMockCall.hangup).toHaveBeenCalled(); expect(callChangedListener).toHaveBeenCalledWith(newCallsMap); @@ -1165,7 +1165,7 @@ describe("Group Call", function () { it("handles case where call is missing from the calls map", () => { // @ts-ignore groupCall.calls = new Map(); - oldMockCall.emit(CallEvent.Replaced, newMockCall.typed()); + oldMockCall.emit(CallEvent.Replaced, newMockCall.typed(), oldMockCall.typed()); expect(oldMockCall.hangup).toHaveBeenCalled(); expect(callChangedListener).toHaveBeenCalledWith(newCallsMap); diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts index 1c75e92f0..80851909f 100644 --- a/src/webrtc/call.ts +++ b/src/webrtc/call.ts @@ -296,20 +296,34 @@ export interface VoipEvent { content: Record; } +/** + * These now all have the call object as an argument. Why? Well, to know which call a given event is + * about you have three options: + * 1. Use a closure as the callback that remembers what call it's listening to. This can be + * a pain because you need to pass the listener function again when you remove the listener, + * which might be somewhere else. + * 2. Use not-very-well-known fact that EventEmitter sets 'this' to the emitter object in the + * callback. This doesn't really play well with modern Typescript and eslint and doesn't work + * with our pattern of re-emitting events. + * 3. Pass the object in question as an argument to the callback. + * + * Now that we have group calls which have to deal with multiple call objects, this will + * become more important, and I think methods 1 and 2 are just going to cause issues. + */ export type CallEventHandlerMap = { - [CallEvent.DataChannel]: (channel: RTCDataChannel) => void; - [CallEvent.FeedsChanged]: (feeds: CallFeed[]) => void; - [CallEvent.Replaced]: (newCall: MatrixCall) => void; - [CallEvent.Error]: (error: CallError) => void; - [CallEvent.RemoteHoldUnhold]: (onHold: boolean) => void; - [CallEvent.LocalHoldUnhold]: (onHold: boolean) => void; - [CallEvent.LengthChanged]: (length: number) => void; - [CallEvent.State]: (state: CallState, oldState?: CallState) => void; + [CallEvent.DataChannel]: (channel: RTCDataChannel, call: MatrixCall) => void; + [CallEvent.FeedsChanged]: (feeds: CallFeed[], call: MatrixCall) => void; + [CallEvent.Replaced]: (newCall: MatrixCall, oldCall: MatrixCall) => void; + [CallEvent.Error]: (error: CallError, call: MatrixCall) => void; + [CallEvent.RemoteHoldUnhold]: (onHold: boolean, call: MatrixCall) => void; + [CallEvent.LocalHoldUnhold]: (onHold: boolean, call: MatrixCall) => void; + [CallEvent.LengthChanged]: (length: number, call: MatrixCall) => void; + [CallEvent.State]: (state: CallState, oldState: CallState, call: MatrixCall) => void; [CallEvent.Hangup]: (call: MatrixCall) => void; - [CallEvent.AssertedIdentityChanged]: () => void; + [CallEvent.AssertedIdentityChanged]: (call: MatrixCall) => void; /* @deprecated */ [CallEvent.HoldUnhold]: (onHold: boolean) => void; - [CallEvent.SendVoipEvent]: (event: VoipEvent) => void; + [CallEvent.SendVoipEvent]: (event: VoipEvent, call: MatrixCall) => void; }; // The key of the transceiver map (purpose + media type, separated by ':') @@ -459,7 +473,7 @@ export class MatrixCall extends TypedEventEmittererror), + this, ); } } @@ -1513,7 +1528,7 @@ export class MatrixCall extends TypedEventEmittererror)); + this.emit(CallEvent.Error, new CallError(code, message, error), this); throw error; } @@ -1987,7 +2002,7 @@ export class MatrixCall extends TypedEventEmitter { this.makingOffer = true; try { + // XXX: in what situations do we believe gotLocalOffer actually throws? It appears + // to handle most of its exceptions itself and terminate the call. I'm not entirely + // sure it would ever throw, so I can't add a test for these lines. + // Also the tense is different between "gotLocalOffer" and "getLocalOfferFailed" so + // it's not entirely clear whether getLocalOfferFailed is just misnamed or whether + // they've been cross-polinated somehow at some point. await this.gotLocalOffer(); } catch (e) { this.getLocalOfferFailed(e as Error); @@ -2134,7 +2155,7 @@ export class MatrixCall extends TypedEventEmittererror)); + this.emit(CallEvent.Error, new CallError(code, message, error), this); this.terminate(CallParty.Local, code, false); // no need to carry on & send the candidate queue, but we also @@ -2158,7 +2179,11 @@ export class MatrixCall extends TypedEventEmitter { logger.error(`Call ${this.callId} getLocalOfferFailed() running`, err); - this.emit(CallEvent.Error, new CallError(CallErrorCode.LocalOfferFailed, "Failed to get local offer!", err)); + this.emit( + CallEvent.Error, + new CallError(CallErrorCode.LocalOfferFailed, "Failed to get local offer!", err), + this, + ); this.terminate(CallParty.Local, CallErrorCode.LocalOfferFailed, false); }; @@ -2177,6 +2202,7 @@ export class MatrixCall extends TypedEventEmitter { - this.emit(CallEvent.LengthChanged, Math.round((Date.now() - this.callStartTime!) / 1000)); + this.emit(CallEvent.LengthChanged, Math.round((Date.now() - this.callStartTime!) / 1000), this); }, CALL_LENGTH_INTERVAL); } } else if (this.peerConn?.iceConnectionState == "failed") { @@ -2267,7 +2293,7 @@ export class MatrixCall extends TypedEventEmitter { - this.emit(CallEvent.DataChannel, ev.channel); + this.emit(CallEvent.DataChannel, ev.channel, this); }; /** @@ -2380,13 +2406,17 @@ export class MatrixCall extends TypedEventEmittererror)); + this.emit(CallEvent.Error, new CallError(code, message, error), this); this.hangup(code, false); return; diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index adae5cac6..d7bf15e99 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -40,6 +40,11 @@ export enum GroupCallTerminationReason { CallEnded = "call_ended", } +/** + * Because event names are just strings, they do need + * to be unique over all event types of event emitter. + * Some objects could emit more then one set of events. + */ export enum GroupCallEvent { GroupCallStateChanged = "group_call_state_changed", ActiveSpeakerChanged = "active_speaker_changed", @@ -49,7 +54,7 @@ export enum GroupCallEvent { LocalScreenshareStateChanged = "local_screenshare_state_changed", LocalMuteStateChanged = "local_mute_state_changed", ParticipantsChanged = "participants_changed", - Error = "error", + Error = "group_call_error", } export type GroupCallEventHandlerMap = {