Skip to content

Commit 4b31bd4

Browse files
agonza1cursoragent
andauthored
Fix false positives for no_connection WebRTC issue (#30)
* Fix false positives for no_connection WebRTC issue - Mirror ICE connected/completed into Connection.state in generic_event - When ending a session, treat connections as established if stats were received or if connected/completed was ever logged, so peer disconnect or missing connectionstatechange does not raise no_connection Co-authored-by: Cursor <cursoragent@cursor.com> * fix: Only treat stats as proof of connection when payload shows ICE/RTP Early /stats polls persist type=stats before connectivity; require succeeded pair, candidate pair details, or media byte/packet counters instead of any stats row. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: Require succeeded candidate-pair plus nomination/traffic/RTT signal Treat ICE candidate-pair stats as established only when state is succeeded and nominated/selected/bytes/cRTT indicate the pair is actually in use. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 64df5eb commit 4b31bd4

2 files changed

Lines changed: 117 additions & 2 deletions

File tree

app/models/generic_event.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,11 @@ def process_event(self, now, request_data):
326326
self.conference.should_stop_call(now)
327327

328328
elif self.type == 'oniceconnectionstatechange':
329-
if self.data == 'failed':
329+
# Mirror ICE success into Connection.state when onconnectionstatechange is missing.
330+
if self.data in ('connected', 'completed') and self.connection:
331+
self.connection.state = 'connected'
332+
self.connection.update_last_negotiation('connected', now)
333+
elif self.data == 'failed':
330334
Issue(
331335
code='ice_failed',
332336
type=Issue.TYPES_OF_ISSUES['error'],

app/models/issue.py

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,99 @@
33

44
from django.contrib.postgres.fields import JSONField
55
from django.db import models
6+
from django.db.models import Q
67

78
from .basemodel import BaseModel
89

910

11+
def _positive_stat_counter(val):
12+
try:
13+
return float(val) > 0
14+
except (TypeError, ValueError):
15+
return False
16+
17+
18+
def _candidate_pair_stats_indicate_established(conn):
19+
"""
20+
RTC candidate-pair stats (merged into parsed stats connection block): require
21+
succeeded plus a signal that the pair is actually in use, not only reported.
22+
"""
23+
if not isinstance(conn, dict) or conn.get('state') != 'succeeded':
24+
return False
25+
if conn.get('nominated') is True:
26+
return True
27+
if conn.get('selected') is True:
28+
return True
29+
if _positive_stat_counter(conn.get('bytesSent')) or _positive_stat_counter(conn.get('bytesReceived')):
30+
return True
31+
if conn.get('currentRoundTripTime') is not None:
32+
return True
33+
return False
34+
35+
36+
def _stats_payload_indicates_established(data):
37+
"""
38+
True only when parsed stats show ICE completion or media counters advancing —
39+
not the first getStats() poll right after RTCPeerConnection creation (those rows
40+
still use type=stats and carry minimal / empty connection snapshots).
41+
"""
42+
if not isinstance(data, dict):
43+
return False
44+
45+
conn = data.get('connection')
46+
if isinstance(conn, dict):
47+
if _candidate_pair_stats_indicate_established(conn):
48+
return True
49+
if _positive_stat_counter(conn.get('bytesReceived')) or _positive_stat_counter(
50+
conn.get('bytesSent')
51+
):
52+
return True
53+
local_ct = (conn.get('local') or {}).get('candidateType')
54+
remote_ct = (conn.get('remote') or {}).get('candidateType')
55+
if local_ct and remote_ct:
56+
return True
57+
58+
def media_blocks_have_traffic(block):
59+
if not isinstance(block, dict):
60+
return False
61+
for direction in ('inbound', 'outbound'):
62+
reports = block.get(direction)
63+
if not isinstance(reports, list):
64+
continue
65+
for report in reports:
66+
if not isinstance(report, dict):
67+
continue
68+
if (
69+
_positive_stat_counter(report.get('bytesReceived'))
70+
or _positive_stat_counter(report.get('bytesSent'))
71+
or _positive_stat_counter(report.get('packetsReceived'))
72+
or _positive_stat_counter(report.get('packetsSent'))
73+
):
74+
return True
75+
return False
76+
77+
for kind in ('audio', 'video'):
78+
if media_blocks_have_traffic(data.get(kind)):
79+
return True
80+
81+
remote = data.get('remote')
82+
if isinstance(remote, dict):
83+
for kind in ('audio', 'video'):
84+
if media_blocks_have_traffic(remote.get(kind)):
85+
return True
86+
87+
# Per-track stats rows (StatsView.save_event for each track) — flat RTP-ish dict
88+
if (
89+
_positive_stat_counter(data.get('bytesReceived'))
90+
or _positive_stat_counter(data.get('bytesSent'))
91+
or _positive_stat_counter(data.get('packetsReceived'))
92+
or _positive_stat_counter(data.get('packetsSent'))
93+
):
94+
return True
95+
96+
return False
97+
98+
1099
ISSUES = {
11100
# ERRORS
12101
'no_media_access': {
@@ -189,9 +278,31 @@ def check_end_session(session):
189278
# we're looking if all connections are in an unfinished state
190279
connection_bad_state = ['new', 'connecting', 'failed']
191280
connections = session.connections.all()
281+
# Only stats batches that actually show ICE/RTP progress (not an empty early poll).
282+
connection_ids_with_establishing_stats = set()
283+
for ev in session.events.filter(type='stats').exclude(connection_id=None).only(
284+
'connection_id', 'data'
285+
):
286+
if ev.data and _stats_payload_indicates_established(ev.data):
287+
connection_ids_with_establishing_stats.add(ev.connection_id)
288+
# If the peer leaves, the PC often ends in failed while rows still look "unfinished".
289+
# Once we logged connected/completed, do not treat as never-connected.
290+
connection_ids_ever_established = set(
291+
session.events.filter(
292+
Q(type='onconnectionstatechange', data='connected')
293+
| Q(type='oniceconnectionstatechange', data='connected')
294+
| Q(type='oniceconnectionstatechange', data='completed'),
295+
)
296+
.exclude(connection_id=None)
297+
.values_list('connection_id', flat=True)
298+
)
299+
established_ids = connection_ids_with_establishing_stats | connection_ids_ever_established
192300
# if the user had at least one connection
193301
if len(connections) > 0:
194-
bad_conns = [c for c in connections if c.state in connection_bad_state]
302+
bad_conns = [
303+
c for c in connections
304+
if c.state in connection_bad_state and c.id not in established_ids
305+
]
195306

196307
if len(connections) == len(bad_conns):
197308
Issue(

0 commit comments

Comments
 (0)