We have a suspicion that DTLS handshake failures are caused by Linus not handling correctly the switching of the selected ICE candidate pair.
by making the following changes
Sending selectedCandidatePairChanges with the add media success and failure metrics so that we can prove that it's always greater than 1 when DTLS handshake fails and equal to 1 when we connect without issues.
Unfortunately, selectedCandidatePairChanges is not supported by Firefox and Safari, so we'll get these metrics only on Chrome and Edge.
Implementation details:
I've renamed getCurrentConnectionType() to getCurrentConnectionInfo(), because now it returns more properties
getCurrentConnectionType() used to wait for the connection to be established, I've removed that, because we need to call it now in failure cases when connection is not established and anyway the only place it was ever called from so far was from addMedia() which already calls waitForMediaConnectionConnected() before calling getCurrentConnectionType(), so the wait inside getCurrentConnectionType() was redundant.
we're only interested in selectedCandidatePairChanges but I'm also sending numTransports, because I want to be absolutely sure we send the correct value for selectedCandidatePairChanges and we never have more than 1 transport
Change Type
[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
COMPLETES #SPARK-532530
This pull request addresses
We have a suspicion that DTLS handshake failures are caused by Linus not handling correctly the switching of the selected ICE candidate pair.
by making the following changes
Sending
selectedCandidatePairChanges
with the add media success and failure metrics so that we can prove that it's always greater than 1 when DTLS handshake fails and equal to 1 when we connect without issues.Unfortunately, selectedCandidatePairChanges is not supported by Firefox and Safari, so we'll get these metrics only on Chrome and Edge.
Implementation details:
getCurrentConnectionType()
togetCurrentConnectionInfo()
, because now it returns more propertiesgetCurrentConnectionType()
used to wait for the connection to be established, I've removed that, because we need to call it now in failure cases when connection is not established and anyway the only place it was ever called from so far was from addMedia() which already callswaitForMediaConnectionConnected()
before callinggetCurrentConnectionType()
, so the wait insidegetCurrentConnectionType()
was redundant.selectedCandidatePairChanges
but I'm also sendingnumTransports
, because I want to be absolutely sure we send the correct value forselectedCandidatePairChanges
and we never have more than 1 transportChange Type
The following scenarios where tested
manually with locally linked web app + unit tests
I certified that
[x] I have read and followed contributing guidelines
[x] I discussed changes with code owners prior to submitting this pull request
[x] I have not skipped any automated checks
[x] All existing and new tests passed
[x] I have updated the documentation accordingly
Make sure to have followed the contributing guidelines before submitting.