From f920300f28d1395ad45a393cec60d32293c723c0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 18 Apr 2017 11:50:55 -0400 Subject: [PATCH 1/5] Improve handling of call service edge cases. // FREEBIE --- Signal/src/call/CallService.swift | 188 ++++++++++++------ .../Speakerbox/CallKitCallUIAdaptee.swift | 2 +- 2 files changed, 125 insertions(+), 65 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 3d8140776..d13c61b91 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -75,6 +75,7 @@ enum CallError: Error { case disconnected case externalError(underlyingError: Error) case timeout(description: String, call: SignalCall) + case obsoleteCall(description: String, call: SignalCall) } // Should be roughly synced with Android client for consistency @@ -290,6 +291,10 @@ protocol CallServiceObserver: class { return getIceServers().then { iceServers -> Promise in Logger.debug("\(self.TAG) got ice servers:\(iceServers)") + guard self.call == call else { + return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)", call:call)) + } + let useTurnOnly = Environment.getCurrent().preferences.doCallsHideIPAddress() let peerConnectionClient = PeerConnectionClient(iceServers: iceServers, delegate: self, callDirection: .outgoing, useTurnOnly: useTurnOnly) @@ -300,12 +305,20 @@ protocol CallServiceObserver: class { return self.peerConnectionClient!.createOffer() }.then { (sessionDescription: HardenedRTCSessionDescription) -> Promise in + guard self.call == call else { + return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)", call:call)) + } + return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then { let offerMessage = OWSCallOfferMessage(callId: call.signalingId, sessionDescription: sessionDescription.sdp) let callMessage = OWSOutgoingCallMessage(thread: thread, offerMessage: offerMessage) return self.messageSender.sendCallMessage(callMessage) } }.then { + guard self.call == call else { + return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)", call:call)) + } + let (callConnectedPromise, fulfill, _) = Promise.pending() self.fulfillCallConnectedPromise = fulfill @@ -317,15 +330,25 @@ protocol CallServiceObserver: class { return race(timeout, callConnectedPromise) }.then { + guard self.call == call else { + Logger.debug("\(self.TAG) obsolete call in \(#function)") + throw CallError.assertionError(description:"obsolete call in \(#function)") + } + Logger.info("\(self.TAG) outgoing call connected.") + + // TODO: There must be a better way to return a fulfilled promise. + let (promise, fulfill, _) = Promise.pending() + fulfill() + return promise }.catch { error in Logger.error("\(self.TAG) placing call failed with error: \(error)") if let callError = error as? CallError { - self.handleFailedCall(error: callError) + self.handleFailedCall(failedCall:call, error: callError) } else { let externalError = CallError.externalError(underlyingError: error) - self.handleFailedCall(error: externalError) + self.handleFailedCall(failedCall:call, error: externalError) } } } @@ -338,13 +361,12 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"call was unexpectedly nil in \(#function)")) + Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") return } guard call.signalingId == callId else { - let description: String = "received answer for call: \(callId) but current call has id: \(call.signalingId)" - handleFailedCall(error: .assertionError(description: description)) + Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") return } @@ -359,7 +381,7 @@ protocol CallServiceObserver: class { } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: CallError.assertionError(description: "peerConnectionClient was unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:call, error: CallError.assertionError(description: "peerConnectionClient was unexpectedly nil in \(#function)")) return } @@ -368,10 +390,10 @@ protocol CallServiceObserver: class { Logger.debug("\(self.TAG) successfully set remote description") }.catch { error in if let callError = error as? CallError { - self.handleFailedCall(error: callError) + self.handleFailedCall(failedCall:call, error: callError) } else { let externalError = CallError.externalError(underlyingError: error) - self.handleFailedCall(error: externalError) + self.handleFailedCall(failedCall:call, error: externalError) } } } @@ -421,8 +443,14 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) \(#function) for thread: \(thread)") AssertIsOnMainThread() + // Assume this event pertains to the current call. guard let call = self.call else { - handleFailedCall(error: .assertionError(description: "call unexpectedly nil in \(#function)")) + handleFailedCall(failedCall: self.call, error: .assertionError(description: "call unexpectedly nil in \(#function)")) + return + } + + guard thread.contactIdentifier() == call.remotePhoneNumber else { + Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") return } @@ -443,7 +471,7 @@ protocol CallServiceObserver: class { guard call == nil else { // TODO on iOS10+ we can use CallKit to swap calls rather than just returning busy immediately. - Logger.verbose("\(TAG) receivedCallOffer for thread: \(thread) but we're already in call: \(call)") + Logger.verbose("\(TAG) receivedCallOffer for thread: \(thread) but we're already in call: \(call!)") handleLocalBusyCall(newCall, thread: thread) return @@ -456,9 +484,10 @@ protocol CallServiceObserver: class { let timeout = CallError.timeout(description: "background task time ran out before call connected.", call: newCall) DispatchQueue.main.async { guard self.call == newCall else { + Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") return } - self.handleFailedCall(error: timeout) + self.handleFailedCall(failedCall:newCall, error: timeout) } } @@ -468,7 +497,7 @@ protocol CallServiceObserver: class { // FIXME for first time call recipients I think we'll see mic/camera permission requests here, // even though, from the users perspective, no incoming call is yet visible. guard self.call == newCall else { - throw CallError.assertionError(description: "getIceServers() response for obsolete call") + throw CallError.obsoleteCall(description: "getIceServers() response for obsolete call", call:newCall) } assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance") @@ -488,7 +517,7 @@ protocol CallServiceObserver: class { return self.peerConnectionClient!.negotiateSessionDescription(remoteDescription: offerSessionDescription, constraints: constraints) }.then { (negotiatedSessionDescription: HardenedRTCSessionDescription) in guard self.call == newCall else { - throw CallError.assertionError(description: "negotiateSessionDescription() response for obsolete call") + throw CallError.obsoleteCall(description: "negotiateSessionDescription() response for obsolete call", call:newCall) } Logger.debug("\(self.TAG) set the remote description") @@ -498,7 +527,7 @@ protocol CallServiceObserver: class { return self.messageSender.sendCallMessage(callAnswerMessage) }.then { guard self.call == newCall else { - throw CallError.assertionError(description: "sendCallMessage() response for obsolete call") + throw CallError.obsoleteCall(description: "sendCallMessage() response for obsolete call", call:newCall) } Logger.debug("\(self.TAG) successfully sent callAnswerMessage") @@ -514,17 +543,25 @@ protocol CallServiceObserver: class { return race(promise, timeout) }.then { + guard self.call == newCall else { + throw CallError.obsoleteCall(description: "time out for obsolete incoming call", call:newCall) + } + Logger.info("\(self.TAG) incoming call connected.") + + let (promise, fulfill, _) = Promise.pending() + fulfill() + return promise }.catch { error in guard self.call == newCall else { Logger.debug("\(self.TAG) error for obsolete call: \(error)") return } if let callError = error as? CallError { - self.handleFailedCall(error: callError) + self.handleFailedCall(failedCall : newCall, error: callError) } else { let externalError = CallError.externalError(underlyingError: error) - self.handleFailedCall(error: externalError) + self.handleFailedCall(failedCall : newCall, error: externalError) } }.always { Logger.debug("\(self.TAG) ending background task awaiting inbound call connection") @@ -540,27 +577,27 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) called \(#function)") guard self.thread != nil else { - handleFailedCall(error: .assertionError(description: "ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?")) + handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?")) return } guard thread.contactIdentifier() == self.thread!.contactIdentifier() else { - handleFailedCall(error: .assertionError(description: "ignoring remote ice update for thread: \(thread.uniqueId) since the current call is for thread: \(self.thread!.uniqueId)")) + handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for thread: \(thread.uniqueId) since the current call is for thread: \(self.thread!.uniqueId)")) return } guard let call = self.call else { - handleFailedCall(error: .assertionError(description: "ignoring remote ice update for callId: \(callId), since there is no current call.")) + handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for callId: \(callId), since there is no current call.")) return } guard call.signalingId == callId else { - handleFailedCall(error: .assertionError(description: "ignoring remote ice update for call: \(callId) since the current call is: \(call.signalingId)")) + handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for call: \(callId) since the current call is: \(call.signalingId)")) return } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: .assertionError(description: "ignoring remote ice update for thread: \(thread) since the current call hasn't initialized it's peerConnectionClient")) + handleFailedCall(failedCall: call, error: .assertionError(description: "ignoring remote ice update for thread: \(thread) since the current call hasn't initialized it's peerConnectionClient")) return } @@ -575,17 +612,17 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(error: .assertionError(description: "ignoring local ice candidate, since there is no current call.")) + handleFailedCall(failedCall:self.call, error: .assertionError(description: "ignoring local ice candidate, since there is no current call.")) return } guard call.state != .idle else { - handleFailedCall(error: .assertionError(description: "ignoring local ice candidate, since call is now idle.")) + handleFailedCall(failedCall:self.call, error: .assertionError(description: "ignoring local ice candidate, since call is now idle.")) return } guard let thread = self.thread else { - handleFailedCall(error: .assertionError(description: "ignoring local ice candidate, because there was no current TSContactThread.")) + handleFailedCall(failedCall:self.call, error: .assertionError(description: "ignoring local ice candidate, because there was no current TSContactThread.")) return } @@ -616,12 +653,12 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) in \(#function)") guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call.")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call.")) return } guard let thread = self.thread else { - handleFailedCall(error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current thread.")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current thread.")) return } @@ -655,7 +692,7 @@ protocol CallServiceObserver: class { } guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:nil, error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } @@ -683,12 +720,12 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } guard call.localId == localId else { - handleFailedCall(error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) return } @@ -704,7 +741,7 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) in \(#function)") guard self.call != nil else { - handleFailedCall(error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) return } @@ -716,12 +753,12 @@ protocol CallServiceObserver: class { } guard let thread = self.thread else { - handleFailedCall(error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) return } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) return } @@ -744,7 +781,7 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } @@ -770,12 +807,12 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:nil, error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } guard call.localId == localId else { - handleFailedCall(error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) + handleFailedCall(failedCall:nil, error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) return } @@ -805,22 +842,22 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard self.call != nil else { - handleFailedCall(error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) return } guard call == self.call! else { - handleFailedCall(error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) return } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) return } guard let thread = self.thread else { - handleFailedCall(error: .assertionError(description:"\(TAG) missing thread in \(#function)")) + handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) missing thread in \(#function)")) return } @@ -855,12 +892,12 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) return } @@ -907,12 +944,12 @@ protocol CallServiceObserver: class { } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) return } @@ -940,7 +977,7 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(error: .assertionError(description:"\(TAG) received data message, but there is no current call. Ignoring.")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) received data message, but there is no current call. Ignoring.")) return } @@ -950,7 +987,7 @@ protocol CallServiceObserver: class { let connected = message.connected! guard connected.id == call.signalingId else { - handleFailedCall(error: .assertionError(description:"\(TAG) received connected message for call with id:\(connected.id) but current call has id:\(call.signalingId)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) received connected message for call with id:\(connected.id) but current call has id:\(call.signalingId)")) return } @@ -963,12 +1000,12 @@ protocol CallServiceObserver: class { let hangup = message.hangup! guard hangup.id == call.signalingId else { - handleFailedCall(error: .assertionError(description:"\(TAG) received hangup message for call with id:\(hangup.id) but current call has id:\(call.signalingId)")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) received hangup message for call with id:\(hangup.id) but current call has id:\(call.signalingId)")) return } guard let thread = self.thread else { - handleFailedCall(error: .assertionError(description:"\(TAG) current contact thread is unexpectedly nil when receiving hangup DataChannelMessage")) + handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) current contact thread is unexpectedly nil when receiving hangup DataChannelMessage")) return } @@ -1009,7 +1046,7 @@ protocol CallServiceObserver: class { return } - self.handleFailedCall(error: CallError.disconnected) + self.handleFailedCall(failedCall:nil, error: CallError.disconnected) } /** @@ -1096,29 +1133,52 @@ protocol CallServiceObserver: class { } } - public func handleFailedCall(error: CallError) { + // TODO: +// private func handleFailedCall(error: CallError) { +// handleFailedCall(failedCall: nil, error: error) +// } +// +// private func handleFailedCall(callId: UInt64, error: CallError) { +// AssertIsOnMainThread() +// +// guard let call = self.call else { +// handleFailedCall(failedCall:nil, error: error) +// return +// } +// +// guard call.signalingId == callId else { +// handleFailedCall(failedCall:nil, error: error) +// return +// } +// +// handleFailedCall(failedCall: call, error: error) +// } + + public func handleFailedCurrentCall(error: CallError) { + handleFailedCall(failedCall: self.call, error: error) + } + + public func handleFailedCall(failedCall: SignalCall?, error: CallError) { AssertIsOnMainThread() Logger.error("\(TAG) call failed with error: \(error)") - if let call = self.call { + guard let failedCall = failedCall else { + Logger.debug("\(TAG) in \(#function) ignoring obsolete call.") + return + } - if case .timeout(description: _, call: let timedOutCall) = error { - guard timedOutCall == call else { - Logger.debug("Ignoring timeout for previous call") - return - } - } + // It's essential to set call.state before terminateCall, because terminateCall nils self.call + failedCall.error = error + failedCall.state = .localFailure + self.callUIAdapter.failCall(failedCall, error: error) - // It's essential to set call.state before terminateCall, because terminateCall nils self.call - call.error = error - call.state = .localFailure - self.callUIAdapter.failCall(call, error: error) - } else { - // This can happen when we receive an out of band signaling message (e.g. IceUpdate) - // after the call has ended - Logger.debug("\(TAG) in \(#function) but there was no call to fail.") + // Only terminate the current call if it is the current call. + guard failedCall == self.call else { + Logger.debug("\(TAG) in \(#function) ignoring obsolete call.") + return } + // Only terminate the call if it is the current call. terminateCall() } diff --git a/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift b/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift index cd31f4e17..521787178 100644 --- a/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift +++ b/Signal/src/call/Speakerbox/CallKitCallUIAdaptee.swift @@ -220,7 +220,7 @@ final class CallKitCallUIAdaptee: NSObject, CallUIAdaptee, CXProviderDelegate { // End any ongoing calls if the provider resets, and remove them from the app's list of calls, // since they are no longer valid. - callService.handleFailedCall(error: .providerReset) + callService.handleFailedCurrentCall(error: .providerReset) // Remove all calls from the app's list of calls. callManager.removeAllCalls() From 3c4955840d6bf3200f5598d312604d8d28918975 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 18 Apr 2017 11:51:14 -0400 Subject: [PATCH 2/5] Improve handling of call service edge cases. // FREEBIE --- Signal/src/call/CallService.swift | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index d13c61b91..682355f9e 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -74,8 +74,8 @@ enum CallError: Error { case assertionError(description: String) case disconnected case externalError(underlyingError: Error) - case timeout(description: String, call: SignalCall) - case obsoleteCall(description: String, call: SignalCall) + case timeout(description: String) + case obsoleteCall(description: String) } // Should be roughly synced with Android client for consistency @@ -292,7 +292,7 @@ protocol CallServiceObserver: class { Logger.debug("\(self.TAG) got ice servers:\(iceServers)") guard self.call == call else { - return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)", call:call)) + return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)")) } let useTurnOnly = Environment.getCurrent().preferences.doCallsHideIPAddress() @@ -306,7 +306,7 @@ protocol CallServiceObserver: class { return self.peerConnectionClient!.createOffer() }.then { (sessionDescription: HardenedRTCSessionDescription) -> Promise in guard self.call == call else { - return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)", call:call)) + return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)")) } return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then { @@ -316,7 +316,7 @@ protocol CallServiceObserver: class { } }.then { guard self.call == call else { - return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)", call:call)) + return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)")) } let (callConnectedPromise, fulfill, _) = Promise.pending() @@ -325,7 +325,7 @@ protocol CallServiceObserver: class { // Don't let the outgoing call ring forever. We don't support inbound ringing forever anyway. let timeout: Promise = after(interval: TimeInterval(connectingTimeoutSeconds)).then { () -> Void in // rejecting a promise by throwing is safely a no-op if the promise has already been fulfilled - throw CallError.timeout(description: "timed out waiting to receive call answer", call: call) + throw CallError.timeout(description: "timed out waiting to receive call answer") } return race(timeout, callConnectedPromise) @@ -481,7 +481,7 @@ protocol CallServiceObserver: class { call = newCall let backgroundTask = UIApplication.shared.beginBackgroundTask { - let timeout = CallError.timeout(description: "background task time ran out before call connected.", call: newCall) + let timeout = CallError.timeout(description: "background task time ran out before call connected.") DispatchQueue.main.async { guard self.call == newCall else { Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") @@ -497,7 +497,7 @@ protocol CallServiceObserver: class { // FIXME for first time call recipients I think we'll see mic/camera permission requests here, // even though, from the users perspective, no incoming call is yet visible. guard self.call == newCall else { - throw CallError.obsoleteCall(description: "getIceServers() response for obsolete call", call:newCall) + throw CallError.obsoleteCall(description: "getIceServers() response for obsolete call") } assert(self.peerConnectionClient == nil, "Unexpected PeerConnectionClient instance") @@ -517,7 +517,7 @@ protocol CallServiceObserver: class { return self.peerConnectionClient!.negotiateSessionDescription(remoteDescription: offerSessionDescription, constraints: constraints) }.then { (negotiatedSessionDescription: HardenedRTCSessionDescription) in guard self.call == newCall else { - throw CallError.obsoleteCall(description: "negotiateSessionDescription() response for obsolete call", call:newCall) + throw CallError.obsoleteCall(description: "negotiateSessionDescription() response for obsolete call") } Logger.debug("\(self.TAG) set the remote description") @@ -527,7 +527,7 @@ protocol CallServiceObserver: class { return self.messageSender.sendCallMessage(callAnswerMessage) }.then { guard self.call == newCall else { - throw CallError.obsoleteCall(description: "sendCallMessage() response for obsolete call", call:newCall) + throw CallError.obsoleteCall(description: "sendCallMessage() response for obsolete call") } Logger.debug("\(self.TAG) successfully sent callAnswerMessage") @@ -535,7 +535,7 @@ protocol CallServiceObserver: class { let timeout: Promise = after(interval: TimeInterval(connectingTimeoutSeconds)).then { () -> Void in // rejecting a promise by throwing is safely a no-op if the promise has already been fulfilled - throw CallError.timeout(description: "timed out waiting for call to connect", call: newCall) + throw CallError.timeout(description: "timed out waiting for call to connect") } // This will be fulfilled (potentially) by the RTCDataChannel delegate method @@ -544,7 +544,7 @@ protocol CallServiceObserver: class { return race(promise, timeout) }.then { guard self.call == newCall else { - throw CallError.obsoleteCall(description: "time out for obsolete incoming call", call:newCall) + throw CallError.obsoleteCall(description: "time out for obsolete incoming call") } Logger.info("\(self.TAG) incoming call connected.") From b4464f91a288458c919c10ed3f5b46d50b5b5a11 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 18 Apr 2017 12:12:17 -0400 Subject: [PATCH 3/5] Improve handling of call service edge cases. // FREEBIE --- Signal/src/call/CallService.swift | 153 ++++++++++++++++-------------- 1 file changed, 83 insertions(+), 70 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 682355f9e..8878da875 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -331,8 +331,7 @@ protocol CallServiceObserver: class { return race(timeout, callConnectedPromise) }.then { guard self.call == call else { - Logger.debug("\(self.TAG) obsolete call in \(#function)") - throw CallError.assertionError(description:"obsolete call in \(#function)") + throw CallError.obsoleteCall(description: "obsolete outgoing call connected") } Logger.info("\(self.TAG) outgoing call connected.") @@ -345,10 +344,10 @@ protocol CallServiceObserver: class { Logger.error("\(self.TAG) placing call failed with error: \(error)") if let callError = error as? CallError { - self.handleFailedCall(failedCall:call, error: callError) + self.handleFailedCall(failedCall: call, error: callError) } else { let externalError = CallError.externalError(underlyingError: error) - self.handleFailedCall(failedCall:call, error: externalError) + self.handleFailedCall(failedCall: call, error: externalError) } } } @@ -381,7 +380,7 @@ protocol CallServiceObserver: class { } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall:call, error: CallError.assertionError(description: "peerConnectionClient was unexpectedly nil in \(#function)")) + handleFailedCall(failedCall: call, error: CallError.assertionError(description: "peerConnectionClient was unexpectedly nil in \(#function)")) return } @@ -390,10 +389,10 @@ protocol CallServiceObserver: class { Logger.debug("\(self.TAG) successfully set remote description") }.catch { error in if let callError = error as? CallError { - self.handleFailedCall(failedCall:call, error: callError) + self.handleFailedCall(failedCall: call, error: callError) } else { let externalError = CallError.externalError(underlyingError: error) - self.handleFailedCall(failedCall:call, error: externalError) + self.handleFailedCall(failedCall: call, error: externalError) } } } @@ -443,9 +442,9 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) \(#function) for thread: \(thread)") AssertIsOnMainThread() - // Assume this event pertains to the current call. guard let call = self.call else { - handleFailedCall(failedCall: self.call, error: .assertionError(description: "call unexpectedly nil in \(#function)")) + // Assume this event pertains to the current call. + handleFailedCurrentCall(error: .assertionError(description: "call unexpectedly nil in \(#function)")) return } @@ -487,7 +486,7 @@ protocol CallServiceObserver: class { Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") return } - self.handleFailedCall(failedCall:newCall, error: timeout) + self.handleFailedCall(failedCall: newCall, error: timeout) } } @@ -544,7 +543,7 @@ protocol CallServiceObserver: class { return race(promise, timeout) }.then { guard self.call == newCall else { - throw CallError.obsoleteCall(description: "time out for obsolete incoming call") + throw CallError.obsoleteCall(description: "obsolete incoming call connected") } Logger.info("\(self.TAG) incoming call connected.") @@ -558,10 +557,10 @@ protocol CallServiceObserver: class { return } if let callError = error as? CallError { - self.handleFailedCall(failedCall : newCall, error: callError) + self.handleFailedCall(failedCall: newCall, error: callError) } else { let externalError = CallError.externalError(underlyingError: error) - self.handleFailedCall(failedCall : newCall, error: externalError) + self.handleFailedCall(failedCall: newCall, error: externalError) } }.always { Logger.debug("\(self.TAG) ending background task awaiting inbound call connection") @@ -577,27 +576,27 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) called \(#function)") guard self.thread != nil else { - handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?")) + Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?") return } guard thread.contactIdentifier() == self.thread!.contactIdentifier() else { - handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for thread: \(thread.uniqueId) since the current call is for thread: \(self.thread!.uniqueId)")) + Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?") return } guard let call = self.call else { - handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for callId: \(callId), since there is no current call.")) + Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?") return } guard call.signalingId == callId else { - handleFailedCall(failedCall : nil, error: .assertionError(description: "ignoring remote ice update for call: \(callId) since the current call is: \(call.signalingId)")) + Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?") return } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall: call, error: .assertionError(description: "ignoring remote ice update for thread: \(thread) since the current call hasn't initialized it's peerConnectionClient")) + Logger.warn("ignoring remote ice update for thread: \(thread.uniqueId) since there is no current thread. Call already ended?") return } @@ -612,17 +611,23 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(failedCall:self.call, error: .assertionError(description: "ignoring local ice candidate, since there is no current call.")) + // This will only be called for the current peerConnectionClient, so + // fail the current call. + handleFailedCurrentCall(error: .assertionError(description: "ignoring local ice candidate, since there is no current call.")) return } guard call.state != .idle else { - handleFailedCall(failedCall:self.call, error: .assertionError(description: "ignoring local ice candidate, since call is now idle.")) + // This will only be called for the current peerConnectionClient, so + // fail the current call. + handleFailedCurrentCall(error: .assertionError(description: "ignoring local ice candidate, since call is now idle.")) return } guard let thread = self.thread else { - handleFailedCall(failedCall:self.call, error: .assertionError(description: "ignoring local ice candidate, because there was no current TSContactThread.")) + // This will only be called for the current peerConnectionClient, so + // fail the current call. + handleFailedCurrentCall(error: .assertionError(description: "ignoring local ice candidate, because there was no current TSContactThread.")) return } @@ -653,12 +658,16 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) in \(#function)") guard let call = self.call else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call.")) + // This will only be called for the current peerConnectionClient, so + // fail the current call. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call.")) return } guard let thread = self.thread else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current thread.")) + // This will only be called for the current peerConnectionClient, so + // fail the current call. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current thread.")) return } @@ -692,7 +701,8 @@ protocol CallServiceObserver: class { } guard let call = self.call else { - handleFailedCall(failedCall:nil, error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } @@ -720,12 +730,14 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } guard call.localId == localId else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) return } @@ -741,7 +753,7 @@ protocol CallServiceObserver: class { Logger.debug("\(TAG) in \(#function)") guard self.call != nil else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) return } @@ -753,12 +765,12 @@ protocol CallServiceObserver: class { } guard let thread = self.thread else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) return } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) return } @@ -781,7 +793,7 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } @@ -807,12 +819,14 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(failedCall:nil, error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } guard call.localId == localId else { - handleFailedCall(failedCall:nil, error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) return } @@ -842,22 +856,22 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard self.call != nil else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) ignoring \(#function) since there is no current call")) return } guard call == self.call! else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) ignoring \(#function) for call other than current call")) return } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) missing peerconnection client in \(#function)")) return } guard let thread = self.thread else { - handleFailedCall(failedCall:call, error: .assertionError(description:"\(TAG) missing thread in \(#function)")) + handleFailedCall(failedCall: call, error: .assertionError(description:"\(TAG) missing thread in \(#function)")) return } @@ -892,12 +906,14 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } guard let call = self.call else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) return } @@ -944,12 +960,14 @@ protocol CallServiceObserver: class { } guard let peerConnectionClient = self.peerConnectionClient else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } guard let call = self.call else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) return } @@ -977,7 +995,8 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) received data message, but there is no current call. Ignoring.")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) received data message, but there is no current call. Ignoring.")) return } @@ -987,7 +1006,8 @@ protocol CallServiceObserver: class { let connected = message.connected! guard connected.id == call.signalingId else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) received connected message for call with id:\(connected.id) but current call has id:\(call.signalingId)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) received connected message for call with id:\(connected.id) but current call has id:\(call.signalingId)")) return } @@ -1000,12 +1020,14 @@ protocol CallServiceObserver: class { let hangup = message.hangup! guard hangup.id == call.signalingId else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) received hangup message for call with id:\(hangup.id) but current call has id:\(call.signalingId)")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) received hangup message for call with id:\(hangup.id) but current call has id:\(call.signalingId)")) return } guard let thread = self.thread else { - handleFailedCall(failedCall:self.call, error: .assertionError(description:"\(TAG) current contact thread is unexpectedly nil when receiving hangup DataChannelMessage")) + // This should never happen; return to a known good state. + handleFailedCurrentCall(error: .assertionError(description:"\(TAG) current contact thread is unexpectedly nil when receiving hangup DataChannelMessage")) return } @@ -1046,7 +1068,8 @@ protocol CallServiceObserver: class { return } - self.handleFailedCall(failedCall:nil, error: CallError.disconnected) + // Return to a known good state. + self.handleFailedCurrentCall(error: CallError.disconnected) } /** @@ -1133,32 +1156,20 @@ protocol CallServiceObserver: class { } } - // TODO: -// private func handleFailedCall(error: CallError) { -// handleFailedCall(failedCall: nil, error: error) -// } -// -// private func handleFailedCall(callId: UInt64, error: CallError) { -// AssertIsOnMainThread() -// -// guard let call = self.call else { -// handleFailedCall(failedCall:nil, error: error) -// return -// } -// -// guard call.signalingId == callId else { -// handleFailedCall(failedCall:nil, error: error) -// return -// } -// -// handleFailedCall(failedCall: call, error: error) -// } - + // This method should be called when either: a) we know or assume that + // the error is related to the current call. b) the error is so serious + // that we want to terminate the current call (if any) in order to + // return to a known good state. public func handleFailedCurrentCall(error: CallError) { - handleFailedCall(failedCall: self.call, error: error) + handleFailedCall(failedCall: self.call, error: error, forceTerminate:true) } - public func handleFailedCall(failedCall: SignalCall?, error: CallError) { + // This method should be called when a fatal error occurred for a call. + // + // * If we know which call it was, we should update that call's state + // to reflect the error. + // * IFF that call is the current call, we want to terminate it. + public func handleFailedCall(failedCall: SignalCall?, error: CallError, forceTerminate: Bool = false) { AssertIsOnMainThread() Logger.error("\(TAG) call failed with error: \(error)") @@ -1172,8 +1183,10 @@ protocol CallServiceObserver: class { failedCall.state = .localFailure self.callUIAdapter.failCall(failedCall, error: error) - // Only terminate the current call if it is the current call. - guard failedCall == self.call else { + // Only terminate the current call if the error pertains to the current call, + // or if we're trying to return to a known good state. + let shouldTerminate = forceTerminate || failedCall == self.call + guard shouldTerminate else { Logger.debug("\(TAG) in \(#function) ignoring obsolete call.") return } From 3368659d870f0e16d15bd4be5eee6d3fd274012e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 18 Apr 2017 12:26:18 -0400 Subject: [PATCH 4/5] Improve handling of call service edge cases. // FREEBIE --- Signal/src/call/CallService.swift | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 8878da875..99aca8ad3 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -330,16 +330,9 @@ protocol CallServiceObserver: class { return race(timeout, callConnectedPromise) }.then { - guard self.call == call else { - throw CallError.obsoleteCall(description: "obsolete outgoing call connected") - } - - Logger.info("\(self.TAG) outgoing call connected.") - - // TODO: There must be a better way to return a fulfilled promise. - let (promise, fulfill, _) = Promise.pending() - fulfill() - return promise + Logger.info(self.call == call + ? "\(self.TAG) outgoing call connected." + : "\(self.TAG) obsolete outgoing call connected.") }.catch { error in Logger.error("\(self.TAG) placing call failed with error: \(error)") @@ -443,8 +436,7 @@ protocol CallServiceObserver: class { AssertIsOnMainThread() guard let call = self.call else { - // Assume this event pertains to the current call. - handleFailedCurrentCall(error: .assertionError(description: "call unexpectedly nil in \(#function)")) + Logger.warn("\(self.TAG) ignoring obsolete call in \(#function)") return } @@ -542,15 +534,9 @@ protocol CallServiceObserver: class { return race(promise, timeout) }.then { - guard self.call == newCall else { - throw CallError.obsoleteCall(description: "obsolete incoming call connected") - } - - Logger.info("\(self.TAG) incoming call connected.") - - let (promise, fulfill, _) = Promise.pending() - fulfill() - return promise + Logger.info(self.call == newCall + ? "\(self.TAG) incoming call connected." + : "\(self.TAG) obsolete incoming call connected.") }.catch { error in guard self.call == newCall else { Logger.debug("\(self.TAG) error for obsolete call: \(error)") From 791dba92423ec208480b5a418a8ed7db41e54132 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 19 Apr 2017 10:18:03 -0400 Subject: [PATCH 5/5] Respond to CR. // FREEBIE --- Signal/src/call/CallService.swift | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 99aca8ad3..023f2d5ba 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -292,7 +292,7 @@ protocol CallServiceObserver: class { Logger.debug("\(self.TAG) got ice servers:\(iceServers)") guard self.call == call else { - return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)")) + throw CallError.obsoleteCall(description:"obsolete call in \(#function)") } let useTurnOnly = Environment.getCurrent().preferences.doCallsHideIPAddress() @@ -306,7 +306,7 @@ protocol CallServiceObserver: class { return self.peerConnectionClient!.createOffer() }.then { (sessionDescription: HardenedRTCSessionDescription) -> Promise in guard self.call == call else { - return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)")) + throw CallError.obsoleteCall(description:"obsolete call in \(#function)") } return self.peerConnectionClient!.setLocalSessionDescription(sessionDescription).then { @@ -316,7 +316,7 @@ protocol CallServiceObserver: class { } }.then { guard self.call == call else { - return Promise(error: CallError.obsoleteCall(description:"obsolete call in \(#function)")) + throw CallError.obsoleteCall(description:"obsolete call in \(#function)") } let (callConnectedPromise, fulfill, _) = Promise.pending() @@ -688,6 +688,7 @@ protocol CallServiceObserver: class { guard let call = self.call else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) call was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } @@ -717,12 +718,14 @@ protocol CallServiceObserver: class { guard let call = self.call else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) call was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } guard call.localId == localId else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) return } @@ -806,12 +809,14 @@ protocol CallServiceObserver: class { guard let call = self.call else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) call was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call was unexpectedly nil in \(#function)")) return } guard call.localId == localId else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) callLocalId:\(localId) doesn't match current calls: \(call.localId)")) return } @@ -893,12 +898,14 @@ protocol CallServiceObserver: class { guard let peerConnectionClient = self.peerConnectionClient else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) peerConnectionClient was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } guard let call = self.call else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) call was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) return } @@ -947,12 +954,14 @@ protocol CallServiceObserver: class { guard let peerConnectionClient = self.peerConnectionClient else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) peerConnectionClient was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) peerConnectionClient unexpectedly nil in \(#function)")) return } guard let call = self.call else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) call was unexpectedly nil in \(#function)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) call unexpectedly nil in \(#function)")) return } @@ -982,6 +991,7 @@ protocol CallServiceObserver: class { guard let call = self.call else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) received data message, but there is no current call. Ignoring.") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) received data message, but there is no current call. Ignoring.")) return } @@ -993,6 +1003,7 @@ protocol CallServiceObserver: class { guard connected.id == call.signalingId else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) received connected message for call with id:\(connected.id) but current call has id:\(call.signalingId)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) received connected message for call with id:\(connected.id) but current call has id:\(call.signalingId)")) return } @@ -1007,12 +1018,14 @@ protocol CallServiceObserver: class { guard hangup.id == call.signalingId else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) received hangup message for call with id:\(hangup.id) but current call has id:\(call.signalingId)") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) received hangup message for call with id:\(hangup.id) but current call has id:\(call.signalingId)")) return } guard let thread = self.thread else { // This should never happen; return to a known good state. + assertionFailure("\(TAG) current contact thread is unexpectedly nil when receiving hangup DataChannelMessage") handleFailedCurrentCall(error: .assertionError(description:"\(TAG) current contact thread is unexpectedly nil when receiving hangup DataChannelMessage")) return }