fix(WT-1083): use processingStatus to guard outgoing call reconciliation#1054
fix(WT-1083): use processingStatus to guard outgoing call reconciliation#1054SERDUN wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SERDUN
force-pushed
the
fix/WT-1083-outgoing-call-reconciliation
branch
2 times, most recently
from
be8862b to
13bd7ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final activeLineCallIds = [ | ||
| ...stateHandshake.lines, | ||
| stateHandshake.guestLine, | ||
| ].whereType<Line>().map((line) => line.callId).toList(); | ||
|
|
||
| for (final callId in state.callsToTerminate(activeLineCallIds)) { | ||
| final activeCall = state.retrieveActiveCall(callId); | ||
| if (activeCall == null) continue; | ||
| _peerConnectionManager.conditionalCompleteError(callId, 'Active call Request Terminated'); | ||
| add(_CallSignalingEvent.hangup(line: activeCall.line, callId: callId, code: 487, reason: 'Request Terminated')); |
There was a problem hiding this comment.
activeLineCallIds is built as a List<String> and then used only for membership checks (contains) inside CallState.callsToTerminate, which makes the reconciliation O(activeCalls * lineCount). Consider using a Set<String> here (e.g., toSet()) and updating callsToTerminate to accept a Set (or convert once inside the method) so lookups are O(1).
| List<String> callsToTerminate(List<String> activeLineCallIds) { | ||
| final result = <String>[]; | ||
| activeCallsLoop: | ||
| for (final activeCall in activeCalls) { | ||
| if (activeLineCallIds.contains(activeCall.callId)) continue activeCallsLoop; | ||
| if (activeCall.direction == CallDirection.outgoing && | ||
| activeCall.acceptedTime == null && | ||
| activeCall.hungUpTime == null && | ||
| activeCall.processingStatus.isPreOfferSent) { | ||
| continue activeCallsLoop; |
There was a problem hiding this comment.
callsToTerminate performs activeLineCallIds.contains(...) for every active call. If activeLineCallIds can be larger than a handful of entries, consider switching the parameter (or an internal local) to a Set<String> to make membership checks O(1) and avoid repeated linear scans.
Instead of skipping all unacknowledged outgoing calls during StateHandshake reconciliation, skip only those where OutgoingCallRequest was not yet sent. CallProcessingStatus.outgoingOfferSent is set immediately after a successful execute(OutgoingCallRequest), so it reliably indicates the server has received the call. Calls that reached outgoingOfferSent or outgoingRinging but are absent from the handshake are now correctly treated as dead and force-terminated.
Instead of skipping all unacknowledged outgoing calls during StateHandshake reconciliation, skip only those where OutgoingCallRequest was not yet sent. Adds isPreOfferSent getter to CallProcessingStatus enum with an explicit allowlist of pre-send statuses. Calls that have passed outgoingOfferSent but are absent from the handshake are treated as dead and force-terminated.
Extract callsToTerminate(List<String> activeLineCallIds) into CallState so the logic is testable without BLoC dependencies. CallBloc converts signaling Line objects to callId strings before delegating to the method. Add unit tests covering: skip in-flight outgoing, terminate post-offer outgoing, keep calls present in handshake lines.
…rminate Reduces membership checks from O(n) to O(1) per active call.
SERDUN
force-pushed
the
fix/WT-1083-outgoing-call-reconciliation
branch
from
dc3f3dc to
07610c6
Compare
|
Closing in favour of a fresh branch rebased on current develop. |
Summary
OutgoingCallRequestwould get permanently stuck duringStateHandshakereconciliationacceptedTime == null && hungUpTime == nullskip with a precise check againstprocessingStatusCallProcessingStatus.outgoingOfferSentalready reliably indicates the request reached the serverDetails
During handshake reconciliation, the BLoC previously skipped all unacknowledged outgoing calls, even those whose
OutgoingCallRequesthad silently failed. Such calls were never in the server's line list, so they would be skipped on every subsequent handshake and remain stuck forever.The fix: skip only calls where
processingStatusis earlier thanoutgoingOfferSent(i.e. the request was never sent). If the status isoutgoingOfferSentoroutgoingRingingbut the server has no record of the call → force-terminate it as dead.