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(meetings): empty webrtc dumps when user closes the browser early #3830

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

marcin-bazyl
Copy link
Collaborator

@marcin-bazyl marcin-bazyl commented Sep 13, 2024

COMPLETES #SPARK-562098

This pull request addresses

When investigating join meeting failure cases where ICE connection was successful, but RTP packets seemed to be not getting through I saw a lot of webrtc dumps that were missing the stats, they only contained the log. It's because SDK uploads the dumps 5s after connection creation and WCME also calls getStats() 5s after connection creation, but it's just after the SDK timer, so the metrics that get uploaded don't have the result of the 1st getStats() call.

by making the following changes

Changed SDK to do the first upload as soon as we get a first stats-report.

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios where tested

unit tests and manual run with the web app

I certified that

  • I have read and followed contributing guidelines

  • I discussed changes with code owners prior to submitting this pull request

  • I have not skipped any automated checks

  • All existing and new tests passed

  • I have updated the documentation accordingly


Make sure to have followed the contributing guidelines before submitting.

@marcin-bazyl marcin-bazyl requested review from a team as code owners September 13, 2024 15:49
@marcin-bazyl marcin-bazyl added the validated If the pull request is validated for automation. label Sep 13, 2024
this.setNewConnectionId();
// Send the first set of metrics at 5 seconds in the case of a user leaving the call shortly after joining.
setTimeout(this.sendMetricsInQueue.bind(this), 5 * 1000);
this.resetConnection();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've renamed setNewConnectionId() to resetConnection(), because we need to reset the initialMetricsSent flag when we have a new connection, so that we upload the stats for the new connection as soon as they appear

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-3830.d3m3l2kee0btzx.amplifyapp.com

Copy link
Contributor

@adhmenon adhmenon left a comment

Choose a reason for hiding this comment

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

LGTM

@marcin-bazyl marcin-bazyl merged commit 8fa3283 into webex:next Sep 16, 2024
11 checks passed
pagour98 pushed a commit to pagour98/webex-js-sdk that referenced this pull request Sep 27, 2024
pagour98 pushed a commit to pagour98/webex-js-sdk that referenced this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validated If the pull request is validated for automation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants