Skip to content

Commit a79bae9

Browse files
committed
refactor: introduce typed RTC exceptions and acquireModificationLock mutex
- Add pc_exceptions.dart with PCWrongSignalingState hierarchy and RtcJsepErrorParser - Remove duplicate SDPConfigurationError from user_media_builder.dart - Replace on String catch substring checks in RenegotiationHandler with typed exceptions - Add acquireModificationLock to PeerConnectionManager for serialising concurrent modifications - Add missing _CallActionRenegotiate event class to call_event.dart
1 parent 261eb97 commit a79bae9

8 files changed

Lines changed: 164 additions & 24 deletions

File tree

lib/features/call/bloc/call_bloc.dart

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'dart:async';
22
import 'dart:io';
3+
import 'dart:math';
34

45
import 'package:flutter/widgets.dart' hide Notification;
56

@@ -153,6 +154,7 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
153154
on<_PeerConnectionEvent>(_onPeerConnectionEvent, transformer: sequential());
154155
on<CallScreenEvent>(_onCallScreenEvent, transformer: sequential());
155156
on<CallConfigEvent>(_onConfigEvent, transformer: sequential());
157+
on<_CallActionRenegotiate>(_onCallActionRenegotiate, transformer: droppable());
156158

157159
navigator.mediaDevices.ondevicechange = (event) {
158160
add(const _NavigatorMediaDevicesChange());
@@ -1119,6 +1121,8 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
11191121
proximityEnabled: state.shouldListenToProximity,
11201122
);
11211123

1124+
final releaseLock = await _peerConnectionManager.acquireModificationLock(activeCall.callId);
1125+
11221126
try {
11231127
final jsep = event.jsep;
11241128
if (jsep != null) {
@@ -1141,6 +1145,7 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
11411145
'__onCallSignalingEventUpdating: glare detected via pre-check (signalingState=$signalingState), rolling back local offer',
11421146
);
11431147
await peerConnection.setLocalDescription(RTCSessionDescription('', 'rollback'));
1148+
_dispatchRenegotiation(event.callId);
11441149
}
11451150

11461151
try {
@@ -1154,6 +1159,7 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
11541159
'__onCallSignalingEventUpdating: glare detected via catch ($e), rolling back local offer and retrying',
11551160
);
11561161
await peerConnection.setLocalDescription(RTCSessionDescription('', 'rollback'));
1162+
_dispatchRenegotiation(event.callId);
11571163
await peerConnection.setRemoteDescription(remoteDescription);
11581164
} else {
11591165
rethrow;
@@ -1164,7 +1170,9 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
11641170

11651171
// According to RFC 8829 5.6 (https://datatracker.ietf.org/doc/html/rfc8829#section-5.6),
11661172
// localDescription should be set before sending the answer to transition into stable state.
1167-
await peerConnection.setLocalDescription(localDescription);
1173+
await peerConnection
1174+
.setLocalDescription(localDescription)
1175+
.catchError((error) => throw RtcJsepErrorParser.parse(error));
11681176

11691177
await _signalingClient?.execute(
11701178
UpdateRequest(
@@ -1178,10 +1186,13 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
11781186
});
11791187
}
11801188
} catch (e, s) {
1189+
_logger.warning('__onCallSignalingEventUpdating - error:', e, s);
11811190
callErrorReporter.handle(e, s, '__onCallSignalingEventUpdating && jsep error:');
11821191

11831192
_peerConnectionManager.completeError(event.callId, e);
11841193
add(_ResetStateEvent.completeCall(event.callId));
1194+
} finally {
1195+
releaseLock();
11851196
}
11861197
}
11871198

@@ -1482,6 +1493,8 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
14821493
if (currentVideoTrack != null) {
14831494
currentVideoTrack.enabled = event.enabled;
14841495
emit(state.copyWithMappedActiveCall(event.callId, (call) => call.copyWith(video: event.enabled)));
1496+
// Helps recover from video-stuck states when user taps to toggle video
1497+
_dispatchRenegotiation(event.callId);
14851498
return;
14861499
}
14871500

@@ -2003,7 +2016,9 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
20032016

20042017
// According to RFC 8829 5.6 (https://datatracker.ietf.org/doc/html/rfc8829#section-5.6),
20052018
// localDescription should be set before sending the answer to transition into stable state.
2006-
await peerConnection.setLocalDescription(localDescription).catchError((e) => throw SDPConfigurationError(e));
2019+
await peerConnection
2020+
.setLocalDescription(localDescription)
2021+
.catchError((error) => throw RtcJsepErrorParser.parse(error));
20072022

20082023
await _signalingClient?.execute(
20092024
AcceptRequest(
@@ -2906,6 +2921,78 @@ class CallBloc extends Bloc<CallEvent, CallState> with WidgetsBindingObserver im
29062921
await _signalingClient?.execute(updateRequest);
29072922
}
29082923

2924+
/// Proactive renegotiation handler for recovery scenarios (glare rollback, video toggle, etc.).
2925+
///
2926+
/// Uses [acquireModificationLock] to serialize with [__onCallSignalingEventUpdating],
2927+
/// and [droppable] transformer to discard redundant retries queued during an active cycle.
2928+
Future<void> _onCallActionRenegotiate(_CallActionRenegotiate event, Emitter<CallState> emit) async {
2929+
final callId = event.callId;
2930+
final activeCall = state.activeCalls.firstWhereOrNull((call) => call.callId == callId);
2931+
if (activeCall == null) return;
2932+
2933+
final lineId = activeCall.line;
2934+
2935+
final peerConnection = await _peerConnectionManager.retrieve(callId);
2936+
if (peerConnection == null) return;
2937+
2938+
final releaseLock = await _peerConnectionManager.acquireModificationLock(callId);
2939+
2940+
final pcSignalingState = await peerConnection.getSignalingState();
2941+
_logger.warning(() => '_onCallActionRenegotiate signalingState: $pcSignalingState');
2942+
final connectionState = await peerConnection.getConnectionState();
2943+
_logger.warning(() => '_onCallActionRenegotiate connectionState: $connectionState');
2944+
2945+
if (connectionState == RTCPeerConnectionState.RTCPeerConnectionStateNew) {
2946+
_logger.warning('_onCallActionRenegotiate skipped due to new connection state');
2947+
releaseLock();
2948+
return;
2949+
}
2950+
2951+
if (pcSignalingState != RTCSignalingState.RTCSignalingStateStable) {
2952+
_logger.warning('_onCallActionRenegotiate skipped due to non-stable signaling state');
2953+
releaseLock();
2954+
return;
2955+
}
2956+
2957+
try {
2958+
final localDescription = await peerConnection.createOffer({});
2959+
sdpMunger?.apply(localDescription);
2960+
// According to RFC 8829 5.6 (https://datatracker.ietf.org/doc/html/rfc8829#section-5.6),
2961+
// localDescription should be set before sending the offer to transition into have-local-offer state.
2962+
await peerConnection
2963+
.setLocalDescription(localDescription)
2964+
.catchError((error) => throw RtcJsepErrorParser.parse(error));
2965+
2966+
final updateRequest = UpdateRequest(
2967+
transaction: WebtritSignalingClient.generateTransactionId(),
2968+
line: lineId,
2969+
callId: callId,
2970+
jsep: localDescription.toMap(),
2971+
);
2972+
await _signalingClient?.execute(updateRequest);
2973+
} catch (e, s) {
2974+
_logger.warning('_onCallActionRenegotiate error', e, s);
2975+
callErrorReporter.handle(e, s, '_onCallActionRenegotiate error:');
2976+
} finally {
2977+
releaseLock();
2978+
}
2979+
}
2980+
2981+
/// Schedules a renegotiation with a random delay to reduce glare probability.
2982+
///
2983+
/// Inspired by the 491 Request Pending handling described in RFC 5407.
2984+
void _dispatchRenegotiation(String callId, {Duration? delayOverride}) async {
2985+
final randomDelay = delayOverride ?? Duration(milliseconds: Random().nextInt(6000).clamp(1000, 5000));
2986+
_logger.warning(() => '_dispatchRenegotiation for callId: $callId with random delay: $randomDelay');
2987+
Future.delayed(randomDelay).then((_) {
2988+
if (isClosed) return;
2989+
final activeCall = state.activeCalls.firstWhereOrNull((call) => call.callId == callId);
2990+
if (activeCall == null) return;
2991+
2992+
add(_CallActionRenegotiate(callId));
2993+
});
2994+
}
2995+
29092996
void _addToRecents(ActiveCall activeCall) {
29102997
final number = activeCall.handle.value;
29112998
final username = activeCall.displayName;

lib/features/call/bloc/call_event.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,3 +1056,12 @@ class _CallConfigEventUpdated extends CallConfigEvent {
10561056
@override
10571057
List<Object?> get props => [monitorCheckInterval];
10581058
}
1059+
1060+
class _CallActionRenegotiate extends CallEvent {
1061+
const _CallActionRenegotiate(this.callId);
1062+
1063+
final String callId;
1064+
1065+
@override
1066+
List<Object?> get props => [callId];
1067+
}

lib/features/call/utils/call_error_reporter.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:webtrit_signaling/webtrit_signaling.dart';
77
import 'package:webtrit_phone/app/notifications/models/notification.dart';
88

99
import '../models/notification.dart';
10+
import 'pc_exceptions.dart';
1011
import 'user_media_builder.dart';
1112

1213
final _logger = Logger('CallBloc:SignalingErrorReporter');
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
abstract class PCWrongSignalingState implements Exception {
2+
final String message;
3+
const PCWrongSignalingState(this.message);
4+
}
5+
6+
class RtcSetRemoteDescriptionWhileHaveLocalOffer extends PCWrongSignalingState {
7+
const RtcSetRemoteDescriptionWhileHaveLocalOffer(super.message);
8+
}
9+
10+
class RtcCreateAnswerWhileWrongState extends PCWrongSignalingState {
11+
const RtcCreateAnswerWhileWrongState(super.message);
12+
}
13+
14+
class SDPConfigurationError implements Exception {
15+
final String message;
16+
const SDPConfigurationError(this.message);
17+
@override
18+
String toString() => 'SDPConfigurationError: $message';
19+
}
20+
21+
class RtcJsepErrorParser {
22+
static Exception parse(Object error) {
23+
final msg = error.toString();
24+
if (msg.contains('have-local-offer') || msg.contains('setRemoteDescription')) {
25+
return RtcSetRemoteDescriptionWhileHaveLocalOffer(msg);
26+
}
27+
if (msg.contains('createAnswer') || msg.contains('wrong state')) {
28+
return RtcCreateAnswerWhileWrongState(msg);
29+
}
30+
return SDPConfigurationError(msg);
31+
}
32+
}

lib/features/call/utils/peer_connection_manager.dart

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ final class PeerConnectionManager {
8181
/// Stores the [_ConnectionState] for each active call, keyed by [CallId].
8282
final _states = <CallId, _ConnectionState>{};
8383

84+
/// Serialises concurrent modifications to a single call's peer connection.
85+
final Map<String, Completer<void>> _modificationLocks = {};
86+
8487
/// Tracks active disposal futures.
8588
///
8689
/// Acts as a "Disposal Barrier": [createPeerConnection] checks this map
@@ -103,6 +106,22 @@ final class PeerConnectionManager {
103106
}
104107
}
105108

109+
/// Acquires an exclusive modification lock for [callId].
110+
///
111+
/// Returns a release function. Caller must call it in a `finally` block to
112+
/// unblock any concurrent waiters.
113+
Future<void Function()> acquireModificationLock(String callId) async {
114+
while (_modificationLocks.containsKey(callId)) {
115+
await _modificationLocks[callId]!.future;
116+
}
117+
final completer = Completer<void>();
118+
_modificationLocks[callId] = completer;
119+
return () {
120+
_modificationLocks.remove(callId);
121+
if (!completer.isCompleted) completer.complete();
122+
};
123+
}
124+
106125
/// Reserves a slot for a call. Must be called before [retrieve].
107126
void add(CallId callId) {
108127
if (_states.containsKey(callId)) {

lib/features/call/utils/renegotiation_handler.dart

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:flutter_webrtc/flutter_webrtc.dart';
44
import 'package:logging/logging.dart';
55
import 'package:webtrit_signaling/webtrit_signaling.dart';
66
import 'call_error_reporter.dart';
7+
import 'pc_exceptions.dart';
78
import 'sdp_munger.dart';
89

910
final _logger = Logger('RenegotiationHandler');
@@ -112,30 +113,29 @@ class RenegotiationHandler {
112113

113114
// According to RFC 8829 5.6 (https://datatracker.ietf.org/doc/html/rfc8829#section-5.6),
114115
// localDescription should be set before sending the offer to transition into have-local-offer state.
115-
await peerConnection.setLocalDescription(localDescription);
116-
117-
await execute(callId, lineId, localDescription);
116+
try {
117+
await peerConnection.setLocalDescription(localDescription);
118+
await execute(callId, lineId, localDescription);
119+
} on String catch (e) {
120+
throw RtcJsepErrorParser.parse(e);
121+
}
118122
} on WebtritSignalingErrorException catch (e) {
119123
_logger.warning(
120124
() => 'onRenegotiationNeeded: UpdateRequest rejected by server (callId=$callId, lineId=$lineId): $e',
121125
);
122126
callErrorReporter.handle(e, null, 'RenegotiationHandler.handle error (callId=$callId, lineId=$lineId)');
123-
} on String catch (e) {
127+
} on PCWrongSignalingState catch (e) {
124128
// flutter_webrtc surfaces native errors as plain strings. A "wrong state" failure
125129
// on setLocalDescription means a concurrent setRemoteDescription (e.g. from an
126130
// incoming updating_call) moved the PC out of stable between the TOCTOU guard and
127131
// the setLocalDescription call. This is a transient race — libwebrtc keeps the
128132
// [[NegotiationNeeded]] flag set and will re-fire onRenegotiationNeeded once the
129133
// PC returns to stable. No user notification is needed.
130-
if (e.contains('wrong state') || e.contains('have-remote-offer') || e.contains('have-local-offer')) {
131-
_logger.warning(
132-
() =>
133-
'onRenegotiationNeeded: setLocalDescription failed in wrong state ($e) '
134-
'— libwebrtc will re-fire onRenegotiationNeeded when stable',
135-
);
136-
} else {
137-
callErrorReporter.handle(e, null, 'RenegotiationHandler.handle error (callId=$callId, lineId=$lineId)');
138-
}
134+
_logger.warning(
135+
() =>
136+
'RenegotiationHandler: signaling state error — ${e.message} '
137+
'— libwebrtc will re-fire onRenegotiationNeeded when stable',
138+
);
139139
} catch (e, s) {
140140
callErrorReporter.handle(e, s, 'RenegotiationHandler.handle error (callId=$callId, lineId=$lineId)');
141141
} finally {

lib/features/call/utils/user_media_builder.dart

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,3 @@ class UserMediaError implements Exception {
6767
@override
6868
String toString() => 'UserMediaError: $message';
6969
}
70-
71-
class SDPConfigurationError implements Exception {
72-
final String message;
73-
74-
SDPConfigurationError(this.message);
75-
76-
@override
77-
String toString() => 'SDPConfigurationError: $message';
78-
}

lib/features/call/utils/utils.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export 'contact_resolver.dart';
66
export 'ice_filter.dart';
77
export 'logging_rtp_traffic_monitor_delegate.dart';
88
export 'peer_connection_factory.dart';
9+
export 'pc_exceptions.dart';
910
export 'peer_connection_manager.dart';
1011
export 'renegotiation_handler.dart';
1112
export 'peer_connection_policy_applier.dart';

0 commit comments

Comments
 (0)