Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix temporary call messages being handled without call #1834

Merged
merged 5 commits into from
Aug 13, 2021

Conversation

Palid
Copy link
Contributor

@Palid Palid commented Aug 9, 2021

In cases where a rogue client, or just a simple bug, sends a temporary call message, like CallNegotiate the messages should just be discarded if there's no actual p2p connection created.

@Palid Palid requested a review from dbkr August 9, 2021 10:43
@Palid Palid requested a review from a team as a code owner August 9, 2021 10:43
@Palid Palid enabled auto-merge August 9, 2021 10:43
src/webrtc/call.ts Outdated Show resolved Hide resolved
@Palid Palid force-pushed the palid/fix/signaling-state-errors branch from f6f3a69 to 33d876a Compare August 11, 2021 12:26
@Palid Palid changed the title Fix signalingState errors Fix temporary call messages being handled without call Aug 11, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2021

Here's what your changelog entry will look like:

🐛 Bug Fixes

  • Fix temporary call messages being handled without call (#1834). Contributed by Palid.

In cases where a rogue client, or just a simple bug, sends a temporary
call message, like CallNegotiate the messages should just be discarded
if there's no actual p2p connection created.
@Palid Palid force-pushed the palid/fix/signaling-state-errors branch from 33d876a to 9fe05e7 Compare August 11, 2021 12:28
@Palid Palid requested a review from dbkr August 11, 2021 12:28
src/webrtc/call.ts Outdated Show resolved Hide resolved
src/webrtc/callEventHandler.ts Outdated Show resolved Hide resolved
src/webrtc/callEventHandler.ts Outdated Show resolved Hide resolved
@Palid Palid added the T-Defect label Aug 11, 2021
@Palid Palid disabled auto-merge August 11, 2021 12:39
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at call.js again, the most most likely cause of this was old calls ie. inited with initWithHangup(). It is worrying that we're seeing these spurious events at all and if it happened with a non-hung-up call we'd get very strange behaviour, but ok, let's see if we start getting this log message a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants