Revert "Revert "Add the call object to Call events""

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<void>;
     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 {

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();
+        });
+    });
 });

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);
         });

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);

src/webrtc/call.ts

@@ -296,20 +296,34 @@ export interface VoipEvent {
     content: Record<string, unknown>;
 }
 
+/**
+ * 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 TypedEventEmitter<CallEvent, CallEventHandlerMap
      */
     public createDataChannel(label: string, options: RTCDataChannelInit | undefined): RTCDataChannel {
         const dataChannel = this.peerConn!.createDataChannel(label, options);
-        this.emit(CallEvent.DataChannel, dataChannel);
+        this.emit(CallEvent.DataChannel, dataChannel, this);
         return dataChannel;
     }
 
@@ -494,7 +508,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
     private set state(state: CallState) {
         const oldState = this._state;
         this._state = state;
-        this.emit(CallEvent.State, state, oldState);
+        this.emit(CallEvent.State, state, oldState, this);
     }
 
     public get type(): CallType {
@@ -684,7 +698,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             }),
         );
 
-        this.emit(CallEvent.FeedsChanged, this.feeds);
+        this.emit(CallEvent.FeedsChanged, this.feeds, this);
 
         logger.info(
             `Call ${this.callId} pushRemoteFeed() pushed stream (streamId=${stream.id}, active=${stream.active}, purpose=${purpose})`,
@@ -732,7 +746,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             }),
         );
 
-        this.emit(CallEvent.FeedsChanged, this.feeds);
+        this.emit(CallEvent.FeedsChanged, this.feeds, this);
 
         logger.info(
             `Call ${this.callId} pushRemoteFeedWithoutMetadata() pushed stream (streamId=${stream.id}, active=${stream.active})`,
@@ -832,7 +846,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             `Call ${this.callId} pushLocalFeed() pushed stream (id=${callFeed.stream.id}, active=${callFeed.stream.active}, purpose=${callFeed.purpose})`,
         );
 
-        this.emit(CallEvent.FeedsChanged, this.feeds);
+        this.emit(CallEvent.FeedsChanged, this.feeds, this);
     }
 
     /**
@@ -869,7 +883,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
         }
 
         this.feeds = [];
-        this.emit(CallEvent.FeedsChanged, this.feeds);
+        this.emit(CallEvent.FeedsChanged, this.feeds, this);
     }
 
     private deleteFeedByStream(stream: MediaStream): void {
@@ -886,7 +900,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
     private deleteFeed(feed: CallFeed): void {
         feed.dispose();
         this.feeds.splice(this.feeds.indexOf(feed), 1);
-        this.emit(CallEvent.FeedsChanged, this.feeds);
+        this.emit(CallEvent.FeedsChanged, this.feeds, this);
     }
 
     // The typescript definitions have this type as 'any' :(
@@ -1117,7 +1131,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             }
         }
         this.successor = newCall;
-        this.emit(CallEvent.Replaced, newCall);
+        this.emit(CallEvent.Replaced, newCall, this);
         this.hangup(CallErrorCode.Replaced, true);
     }
 
@@ -1188,6 +1202,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             this.emit(
                 CallEvent.Error,
                 new CallError(CallErrorCode.NoUserMedia, "Failed to get camera access: ", <Error>error),
+                this,
             );
         }
     }
@@ -1513,7 +1528,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
         this.updateMuteStatus();
         this.sendMetadataUpdate();
 
-        this.emit(CallEvent.RemoteHoldUnhold, this.remoteOnHold);
+        this.emit(CallEvent.RemoteHoldUnhold, this.remoteOnHold, this);
     }
 
     /**
@@ -1638,7 +1653,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
                 code = CallErrorCode.UnknownDevices;
                 message = "Unknown devices present in the room";
             }
-            this.emit(CallEvent.Error, new CallError(code, message, <Error>error));
+            this.emit(CallEvent.Error, new CallError(code, message, <Error>error), this);
             throw error;
         }
 
@@ -1987,7 +2002,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
 
         const newLocalOnHold = this.isLocalOnHold();
         if (prevLocalOnHold !== newLocalOnHold) {
-            this.emit(CallEvent.LocalHoldUnhold, newLocalOnHold);
+            this.emit(CallEvent.LocalHoldUnhold, newLocalOnHold, this);
             // also this one for backwards compat
             this.emit(CallEvent.HoldUnhold, newLocalOnHold);
         }
@@ -2018,7 +2033,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             id: content.asserted_identity.id,
             displayName: content.asserted_identity.display_name,
         };
-        this.emit(CallEvent.AssertedIdentityChanged);
+        this.emit(CallEvent.AssertedIdentityChanged, this);
     }
 
     public callHasEnded(): boolean {
@@ -2040,6 +2055,12 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
     private async wrappedGotLocalOffer(): Promise<void> {
         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 TypedEventEmitter<CallEvent, CallEventHandlerMap
                 message = "Unknown devices present in the room";
             }
 
-            this.emit(CallEvent.Error, new CallError(code, message, <Error>error));
+            this.emit(CallEvent.Error, new CallError(code, message, <Error>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<CallEvent, CallEventHandlerMap
     private getLocalOfferFailed = (err: Error): void => {
         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<CallEvent, CallEventHandlerMap
                 "Couldn't start capturing media! Is your microphone set up and " + "does this app have permission?",
                 err,
             ),
+            this,
         );
         this.terminate(CallParty.Local, CallErrorCode.NoUserMedia, false);
     };
@@ -2200,7 +2226,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
                 this.callStartTime = Date.now();
 
                 this.callLengthInterval = setInterval(() => {
-                    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<CallEvent, CallEventHandlerMap
     };
 
     private onDataChannel = (ev: RTCDataChannelEvent): void => {
-        this.emit(CallEvent.DataChannel, ev.channel);
+        this.emit(CallEvent.DataChannel, ev.channel, this);
     };
 
     /**
@@ -2380,13 +2406,17 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
                 [ToDeviceMessageId]: uuidv4(),
             };
 
-            this.emit(CallEvent.SendVoipEvent, {
-                type: "toDevice",
-                eventType,
-                userId: this.invitee || this.getOpponentMember()?.userId,
-                opponentDeviceId: this.opponentDeviceId,
-                content,
-            });
+            this.emit(
+                CallEvent.SendVoipEvent,
+                {
+                    type: "toDevice",
+                    eventType,
+                    userId: this.invitee || this.getOpponentMember()?.userId,
+                    opponentDeviceId: this.opponentDeviceId,
+                    content,
+                },
+                this,
+            );
 
             const userId = this.invitee || this.getOpponentMember()!.userId;
             if (this.client.getUseE2eForGroupCall()) {
@@ -2414,13 +2444,17 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
                 );
             }
         } else {
-            this.emit(CallEvent.SendVoipEvent, {
-                type: "sendEvent",
-                eventType,
-                roomId: this.roomId,
-                content: realContent,
-                userId: this.invitee || this.getOpponentMember()?.userId,
-            });
+            this.emit(
+                CallEvent.SendVoipEvent,
+                {
+                    type: "sendEvent",
+                    eventType,
+                    roomId: this.roomId,
+                    content: realContent,
+                    userId: this.invitee || this.getOpponentMember()?.userId,
+                },
+                this,
+            );
 
             await this.client.sendEvent(this.roomId!, eventType, realContent);
         }
@@ -2669,7 +2703,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
                 const code = CallErrorCode.SignallingFailed;
                 const message = "Signalling failed";
 
-                this.emit(CallEvent.Error, new CallError(code, message, <Error>error));
+                this.emit(CallEvent.Error, new CallError(code, message, <Error>error), this);
                 this.hangup(code, false);
 
                 return;

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 = {